Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrectly warns about mixing code and declarations. #1229

Open
stolk opened this issue Sep 9, 2023 · 9 comments
Open

Incorrectly warns about mixing code and declarations. #1229

stolk opened this issue Sep 9, 2023 · 9 comments

Comments

@stolk
Copy link

stolk commented Sep 9, 2023

So, OpenCL is based on C99, so mixing code and variable declarations is perfectly acceptable.

Yet, clsvp will complain with:

warning: mixing declarations and code is incompatible with standards before C99

Is there a flag I need to provide to clspv to stop this warning?

@rjodinchr
Copy link
Collaborator

Could you provide an example showing this message please?

@stolk
Copy link
Author

stolk commented Sep 10, 2023

Here is an example kernel:

#define uint32_t        uint

__kernel
void foo
(
        __global uint32_t* __restrict__ a,
        __global uint32_t* __restrict__ b,
        __global uint32_t* __restrict__ c
)
{
        const uint32_t pindex = get_global_id(0);
        uint32_t aa = a[pindex] * a[pindex];
        if (aa < 1)
                aa = 1;
        uint32_t bb = b[pindex] * b[pindex];
        if (bb < 1)
                bb = 1;
        c[pindex] = aa + bb;
}

Which will give the compiler warning:

$ ~/src/clspv/build/bin/clspv foo.cl 
foo.cl:15:11: warning: mixing declarations and code is incompatible with standards before C99
   15 |         uint32_t bb = b[pindex] * b[pindex];
      |    

... which makes no sense, as OpenCL follows C99.

@rjodinchr
Copy link
Collaborator

clspv is build to enable all warnings, which is equivalent to compiling with -Weverything.

Compiling a simple C code can produce the message even with C11:

$ clang -std=c11 test.c  -Weverything
test.c:7:9: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
    int toto = 0;

For more information: llvm/llvm-project#53438

@alan-baker should we add no-declaration-after-statement in the warning list of clspv?

@rjodinchr
Copy link
Collaborator

Note that the behavior is the same with gcc, expect that -Weverything does not exist, thus one need to use -Wdeclaration-after-statement to trigger the warning message (even with C11 or newer).

@stolk
Copy link
Author

stolk commented Sep 11, 2023

I think clspv needs to add a flag internally, because it will not accept it on its own command line:

~/src/clspv/build/bin/clspv -Wno-declaration-after-statement -o foo.spirv foo.cl 
clspv: Unknown command line argument '-Wno-declaration-after-statement'.  Try: '/home/bram/src/clspv/build/bin/clspv --help'
clspv: Did you mean '--enable-ext-tsp-block-placement'?

@rjodinchr
Copy link
Collaborator

I don't think we want to make it configurable. Either we want to remove this warning from clspv, or we want to keep it like it is today.
Is this blocking something on your side for some reason?

@stolk
Copy link
Author

stolk commented Sep 11, 2023

No, it is not blocking. It is just unnecessary noise.

You can close it if you don't think it needs addressing.

@rjodinchr
Copy link
Collaborator

I think this is worth discussing. Let's wait for @alan-baker opinion before closing it.
Thank you for reporting it.

@alan-baker
Copy link
Collaborator

I generally prefer to have as few deviations from clang as possible. If anything I'd rather expose an option that can be forwarded to clang to let the user control which warnings they want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants