[PATCH] D100794: [HIP] Support overloaded math functions for hipRTC

Artem Belevich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 19 14:58:42 PDT 2021


tra added a subscriber: jlebar.
tra added a comment.

LGTM overall.

@jlebar: I could use your opinion here.



================
Comment at: clang/lib/Headers/__clang_hip_cmath.h:341
 
-  typedef decltype(__test(std::declval<_Tp>())) type;
-  static const bool value = !std::is_same<type, void>::value;
+  typedef decltype(__test(_Tp{})) type;
+  static const bool value = !is_same<type, void>::value;
----------------
@jlebar : Should we expect any observable surprises here?


================
Comment at: clang/lib/Headers/__clang_hip_runtime_wrapper.h:76-86
+#if !defined(__HIPCC_RTC__)
 #include <__clang_cuda_math_forward_declares.h>
+#endif // __HIPCC_RTC__
 #include <__clang_hip_cmath.h>
+#if !defined(__HIPCC_RTC__)
 #include <__clang_cuda_complex_builtins.h>
 
----------------
I'd add an empty line separators between `#if/#endif` blocks.

Or, perhaps, it should be restructured as 
```
#if defined(__HIPCC_RTC__)
#include <__clang_hip_cmath.h>
#else
#include <__clang_cuda_math_forward_declares.h>
#include <__clang_hip_cmath.h>
#include <__clang_cuda_complex_builtins.h>
#include <algorithm>
#include <complex>
#include <new>
#endif // __HIPCC_RTC__
```


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

https://reviews.llvm.org/D100794



More information about the cfe-commits mailing list