jdoerfert added a comment. Last issue I have (in addition to the check @tra suggested) is the order in which we include math.h and cstdlib. can you flip it in one of the test cases?
================ Comment at: lib/Headers/__clang_cuda_math_forward_declares.h:30-38 +#ifndef _OPENMP +__DEVICE__ long abs(long); +__DEVICE__ long long abs(long long); +#else +#ifndef __cplusplus __DEVICE__ long abs(long); __DEVICE__ long long abs(long long); ---------------- gtbercea wrote: > tra wrote: > > I'm not quite sure what's the idea here. It may be worth adding a comment. > > > > It could also be expressed somewhat simpler: > > > > ``` > > #if !(defined(_OPENMP) && defined(__cplusplus)) > > ... > > #endif > > ``` > > > When these two functions definitions are here or in the __clang_cuda_cmath.h > header then I get the following error (adapted for the __clang_cuda_cmath.h > case): > > > ``` > /usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/cstdlib:166:3: > error: declaration conflicts with target of using declaration already in > scope > abs(long __i) { return __builtin_labs(__i); } > ^ > /autofs/home/gbercea/patch-compiler/obj-release/lib/clang/9.0.0/include/__clang_cuda_cmath.h:40:17: > note: target of using declaration > __DEVICE__ long abs(long __n) { return ::labs(__n); } > ^ > /usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/cstdlib:122:11: > note: using declaration > using ::abs; > ^ > /usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/cstdlib:174:3: > error: declaration conflicts with target of using declaration already in > scope > abs(long long __x) { return __builtin_llabs (__x); } > ^ > /autofs/home/gbercea/patch-compiler/obj-release/lib/clang/9.0.0/include/__clang_cuda_cmath.h:39:22: > note: target of using declaration > __DEVICE__ long long abs(long long __n) { return ::llabs(__n); } > ^ > /usr/lib/gcc/ppc64le-redhat-linux/4.8.5/../../../../include/c++/4.8.5/cstdlib:122:11: > note: using declaration > using ::abs; > ``` > > Long story short, we currently cannot use the overload trick through `__device__` and therefore *replace* (not augment) host math headers with the cuda versions which unfortunately mix std math functions with other functions that we don't want/need. ================ Comment at: lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h:10 + +#if defined(__NVPTX__) && defined(_OPENMP) + ---------------- tra wrote: > You may want to add include guards. > > I'd also make inclusion of the file in non-openmp compilation an error, if it > makes sense for OpenMP. It does for CUDA. That is something we should be able to do, error out if _OPENMP is not defined I mean. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61765/new/ https://reviews.llvm.org/D61765 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits