llvm-project
llvm-project copied to clipboard
clang crashes due to bool redeclaration
/* Visual Studio < 2013 does not have stdbool.h so here it is a replacement: */
#if defined __STDC__ && defined __STDC_VERSION__ && __STDC_VERSION__ >= 199901L
/* have a C99 compiler */
typedef _Bool bool;
#else
/* do not have a C99 compiler */
typedef unsigned char bool;
#endif
When compiling with OpenMP for an AMDGPU, it seems to automatically include stdbool.h
.
> cat test.c
/* Visual Studio < 2013 does not have stdbool.h so here it is a replacement: */
#if defined __STDC__ && defined __STDC_VERSION__ && __STDC_VERSION__ >= 199901L
/* have a C99 compiler */
typedef _Bool bool;
#else
/* do not have a C99 compiler */
typedef unsigned char bool;
> clang -c -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx908 test.c
test.c:4:15: error: cannot combine with previous 'bool' declaration specifier
typedef _Bool bool;
^
lib/clang/16.0.0/include/stdbool.h:20:14: note: expanded from macro 'bool'
#define bool _Bool
^
../test.c:4:1: warning: typedef requires a name [-Wmissing-declarations]
typedef _Bool bool;
^~~~~~~~~~~~~~~~~~
1 warning and 1 error generated.
There is no error when compiling for the CPU (leave off all openmp flags).
I also noticed that amdclang does not run into this problem (using amdclang from rocm/5.1.0). Is there special handling going on in amdclang that is not going on upstream?
amdclang shows the desired behavior. That is, amdclang runs into the problem if you modify the testcase to include stdbool.h
, that is, prepend #include stdbool.h
to the testcase.
Tagging @arsenm for visibility and additional contacts (if appropriate)
Also tested with rocm/5.2.1, same behavior for amdclang (lack of error).
@llvm/issue-subscribers-clang-driver
@llvm/issue-subscribers-openmp
Is it possible for you to verify that the patch in https://reviews.llvm.org/D131639 solves your issue?
It gets past the original error but, like @shiltian mentioned on your review, bool is used by another file:
In file included from <built-in>:1:
In file included from /ptmp/rodriiva/hpc-pe-cce-cce/build/dev/install/cce-clang/x86_64/lib/clang/15.0.0/include/openmp_wrappers/__clang_openmp_device_functions.h:51:
/ptmp/rodriiva/hpc-pe-cce-cce/build/dev/install/cce-clang/x86_64/lib/clang/15.0.0/include/__clang_hip_libdevice_declares.h:315:63: error: unknown type name 'bool'
float c, bool s);
^
1 error generated.
Yeah that's the only usage I know of, was wondering if it work work regardless. One solution is to just use _Bool
directly.
Sure. Do you want to update your patch or should I push one?
I'll update it real quick, let me know if it works because I didn't have any errors on my end.
I also got the provided test case to compile, but I ran it on our in-house test suites and a lot of stuff broke. I still haven't gotten to the bottom of it, but it seems it's causing kernels to be invalid when attempting to load them via HIP. I haven't ran any upstream test suites yet, however.
If this were a simple workaround to make someone happy I'd be fine, but since this seems to be a rather annoying work-around only required for broken user code I'm tempted to close this as wontfix.
Turns out my build environment was erroneous. The patch worked cleanly and didn't break anything on this end.
As discussed in D131639, we will not work around this. There is no guarantee that the system or toolchain will not include headers like <stdbool.h>
so we shouldn't be expected to work around these cases. The user should instead use the provided __bool_true_false_are_defined
macro to prevent potential collisions with <stdbool.h>
.