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

yaxunl marked 2 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Headers/__clang_hip_cmath.h:20 #include #include #include ashi1 wrote: > we may not need to `#include` limits and/or type_traits if we replaced them > with `__hip:

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

ashi1 added inline comments. Comment at: clang/lib/Headers/__clang_hip_cmath.h:20 #include #include #include we may not need to `#include` limits and/or type_traits if we replaced them with `__hip::is_integral` and `__hip::is_arithmetic`. ===

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

This revision was automatically updated to reflect the committed changes. yaxunl marked an inline comment as done. Closed by commit rG8baba6890de7: [HIP] Support overloaded math functions for hipRTC (authored by yaxunl). Herald added a project: clang. Changed prior to commit: https://reviews.ll

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

tra accepted this revision. tra added inline comments. Comment at: clang/lib/Headers/__clang_hip_cmath.h:586-587 _GLIBCXX_BEGIN_NAMESPACE_VERSION #endif #endif Nit: I'd add `// ` here for consistency. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

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

yaxunl updated this revision to Diff 339773. yaxunl added a comment. make it NFC for non-hipRTC CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100794/new/ https://reviews.llvm.org/D100794 Files: clang/lib/Headers/__clang_hip_cmath.h clang/lib/Headers/__clang_hip_runtime_wrapper.h

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

yaxunl added inline comments. Comment at: clang/lib/Headers/__clang_hip_cmath.h:309 +}; +template <> struct is_arithmetic<_Float16> { + enum { value = 1 }; This changes the overloading behavior of math functions with _Float16 arguments since previously we use s

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

yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: clang/lib/Headers/__clang_hip_cmath.h:347 + static const bool value = !is_same::value; }; yaxunl wrote: > tra wrote: > > Nit: `}; // namespace __hip` > will do actually this is

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

yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: clang/lib/Headers/__clang_hip_cmath.h:347 + static const bool value = !is_same::value; }; tra wrote: > Nit: `}; // namespace __hip` will do CHANGES SINCE LAST ACTION https:

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

tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/lib/Headers/__clang_hip_cmath.h:347 + static const bool value = !is_same::value; }; Nit: `}; // namespace __hip` CHANGES SINCE LAST A

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

yaxunl updated this revision to Diff 339009. yaxunl marked an inline comment as done. yaxunl added a comment. fix __hip::__numeric_type CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100794/new/ https://reviews.llvm.org/D100794 Files: clang/lib/Headers/__clang_hip_cmath.h clang/lib/

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

jlebar added a comment. Other than the declval issue discussed above, this looks reasonable to me. I don't notice anything wrong with the standard library reimplementations here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100794/new/ https://reviews.llvm.org/D100794 __

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

yaxunl marked 2 inline comments as done. yaxunl added inline comments. Comment at: clang/lib/Headers/__clang_hip_cmath.h:341 - typedef decltype(__test(std::declval<_Tp>())) type; - static const bool value = !std::is_same::value; + typedef decltype(__test(_Tp{})) type; + sta

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

tra added inline comments. Comment at: clang/lib/Headers/__clang_hip_cmath.h:341 - typedef decltype(__test(std::declval<_Tp>())) type; - static const bool value = !std::is_same::value; + typedef decltype(__test(_Tp{})) type; + static const bool value = !is_same::value;

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

yaxunl updated this revision to Diff 338864. yaxunl marked an inline comment as done. yaxunl added a comment. revised by Artem's comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100794/new/ https://reviews.llvm.org/D100794 Files: clang/lib/Headers/__clang_hip_cmath.h clang/li

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

yaxunl marked an inline comment as done. yaxunl added inline comments. 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 !de

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

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::value; + typed

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

yaxunl created this revision. yaxunl added a reviewer: tra. yaxunl requested review of this revision. Remove the dependence on standard C++ header for overloaded math functions in HIP header since standard C++ header is not available for hipRTC. https://reviews.llvm.org/D100794 Files: clang/l