ashi1 added a comment. A few small comments, otherwise LGTM on the HIP header side.
================ Comment at: clang/lib/Headers/__clang_hip_cmath.h:30 +#ifdef __OPENMP_AMDGCN__ +#define __DEVICE__ static __attribute__((always_inline, nothrow)) +#define __CONSTEXPR__ constexpr ---------------- Does OpenMP not require `__device__` attribute here? I know constexpr defines `__device__` on HIP, does OMP do the same? ================ Comment at: clang/lib/Headers/__clang_hip_cmath.h:32 +#define __CONSTEXPR__ constexpr +#define __constant__ __attribute__((constant)) +#else ---------------- I don't think this is the right place to define `__constant__`? It's unused in this header, and may get forgotten. Would it be better to define it in the openmp wrapper or does cmath define it in OpenMP? ================ Comment at: clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h:47 +#define __OPENMP_AMDGCN__ +#define __device__ +/// Include declarations for libdevice functions. ---------------- Would it be better to push and pop these macros, in case it was defined outside of here? 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
