jdoerfert added a comment.
Really looking forward to this! Thanks a lot!
I left some comments.
================
Comment at: clang/lib/Headers/__clang_hip_math.h:35
+#ifdef __OPENMP_AMDGCN__
+#define __RETURN_TYPE int
+#else
----------------
JonChesterfield wrote:
> I'd expect openmp to match the cplusplus/c distinction here, as openmp works
> on C source
^ Agreed. Though, we use a different trick because it's unfortunately not as
straight forward always and can be decided based on the C vs C++.
================
Comment at:
clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h:112
+
+#pragma omp end declare variant
+
----------------
Can you make the declare variant scope of nvptx and amdgpu smaller and put them
next to each other.
```
#ifdef __cplusplus
extern "C" {
#endif
#declare variant
#define ...
...
#undef
#end
#declare variant
...
#end
#ifdef __cplusplus
} // extern "C"
```
================
Comment at: clang/lib/Headers/openmp_wrappers/cmath:83
+#include <__clang_hip_cmath.h>
+#undef __OPENMP_AMDGCN__
+
----------------
No match_any needed (here and elsewhere).
Also, don't we want all but the includes to be the same for both GPUs. Maybe we
have a device(kind(gpu)) variant and inside the nvptx and amdgpu just for the
respective include?
================
Comment at: clang/lib/Headers/openmp_wrappers/math.h:59
+#pragma omp end declare variant
+
#endif
----------------
FWIW, This is what I think the begin/end regions should look like. Small and
next to each other.
================
Comment at: clang/test/Headers/Inputs/include/cstdlib:15
+#ifndef __AMDGCN__
namespace std
----------------
JonChesterfield wrote:
> I think I'd expect builtin_labs et al to work on amdgcn, are we missing
> lowering for them?
Yeah, looks weird that we cannot compile this mock-up header.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104904/new/
https://reviews.llvm.org/D104904
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
