tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.
Thank you for the patch. This will make my life a lot easier.
There are a few nits, but it's LGTM in general.
Do I understand it correctly that for detection of HIP installation all we need
is to find the bitcode libraries?
================
Comment at: clang/lib/Driver/ToolChains/ROCm.h:68
+
+ // CUDA architectures for which we have raised an error in
+ // CheckRocmVersionSupportsArch.
----------------
CUDA -> GPU?
Looks like there are number of other mentions of CUDA that should be fixed.
================
Comment at: clang/lib/Headers/__clang_hip_math.h:26
+__DEVICE__
+inline uint64_t __make_mantissa_base8(const char *tagp) {
+ uint64_t r = 0;
----------------
Arguments for compiler-internal declarations and local symbols should all be
prefixed with `__`.
================
Comment at: clang/lib/Headers/__clang_hip_math.h:931-935
+#pragma push_macro("__DEF_FLOAT_FUN")
+#pragma push_macro("__DEF_FLOAT_FUN2")
+#pragma push_macro("__DEF_FLOAT_FUN2I")
+#pragma push_macro("__HIP_OVERLOAD")
+#pragma push_macro("__HIP_OVERLOAD2")
----------------
The macros pushed here do not match the set of macros defined below.
E.g. `__HIP_OVERLOAD` vs `__HIP_OVERLOAD1`, `__DEF_FLOAT_FUN` vs. `__DEF_FUN1`,
etc.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81176/new/
https://reviews.llvm.org/D81176
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
