[Openmp-commits] [PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert marked 3 inline comments as done.
jdoerfert added inline comments.


================
Comment at: clang/lib/AST/ASTContext.cpp:1621
   case BuiltinType::LongDouble:
-    if (getLangOpts().OpenMP && getLangOpts().OpenMPIsDevice)
       return AuxTarget->getLongDoubleFormat();
----------------
These AuxTarget changes can be split off. I observed a segfault while testing stuff.


================
Comment at: clang/lib/Headers/CMakeLists.txt:146
   openmp_wrappers/new
 )
 
----------------
the math_declares above will provide all `C` math functions so we do not need `math.h` anymore here.


================
Comment at: clang/lib/Headers/__clang_cuda_cmath.h:306
 
+#ifndef _OPENMP
 // We need to define these overloads in exactly the namespace our standard
----------------
when this is included with OpenMP it is actually following the system `<cmath>` and these using declarations are neither needed nor helpful (afaict)


================
Comment at: clang/lib/Headers/__clang_cuda_math_forward_declares.h:42-43
 __DEVICE__ long long abs(long long);
-__DEVICE__ double abs(double);
-__DEVICE__ float abs(float);
-#endif
----------------
tra wrote:
> Shouldn't float and double abs variants be preserved?
yes, thx! I make so many errors modifying these files, it is ridicules...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75788/new/

https://reviews.llvm.org/D75788