[PATCH] D81176: [HIP] Add default header and include path

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 4 11:37:26 PDT 2020


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





More information about the cfe-commits mailing list