Skip to content

Conversation

@yxsamliu
Copy link
Collaborator

If there is a type T which can be converted to both float and double etc but itself is not specialized for __numeric_type, and it is called for math functions eg. fma, it will cause ambiguity with test function of __numeric_type.

Since test is not template, this error is not bypassed by SFINAE. This is a design flaw of __numeric_type. This patch fixes clang wrapper header to use SFINAE to avoid such ambiguity.

Fixes: SWDEV-461604

Fixes: #101239

If there is a type T which can be converted to both float and double etc but itself is not specialized for __numeric_type, and it is called for math functions eg. fma, it will cause ambiguity with test function of __numeric_type. Since test is not template, this error is not bypassed by SFINAE. This is a design flaw of __numeric_type. This patch fixes clang wrapper header to use SFINAE to avoid such ambiguity. Fixes: SWDEV-461604 Fixes: llvm#101239
@yxsamliu yxsamliu requested review from Artem-B and jhuber6 July 31, 2024 14:31
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics labels Jul 31, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 31, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-x86

Author: Yaxun (Sam) Liu (yxsamliu)

Changes

If there is a type T which can be converted to both float and double etc but itself is not specialized for __numeric_type, and it is called for math functions eg. fma, it will cause ambiguity with test function of __numeric_type.

Since test is not template, this error is not bypassed by SFINAE. This is a design flaw of __numeric_type. This patch fixes clang wrapper header to use SFINAE to avoid such ambiguity.

Fixes: SWDEV-461604

Fixes: #101239


Full diff: https://github.com/llvm/llvm-project/pull/101341.diff

2 Files Affected:

  • (modified) clang/lib/Headers/__clang_hip_cmath.h (+6-1)
  • (modified) clang/test/Headers/__clang_hip_cmath.hip (+19)
diff --git a/clang/lib/Headers/__clang_hip_cmath.h b/clang/lib/Headers/__clang_hip_cmath.h index b52d6b7816611..7d982ad9af7ee 100644 --- a/clang/lib/Headers/__clang_hip_cmath.h +++ b/clang/lib/Headers/__clang_hip_cmath.h @@ -395,7 +395,12 @@ template <class _Tp> struct __numeric_type { // No support for long double, use double instead. static double __test(long double); - typedef decltype(__test(declval<_Tp>())) type; + template <typename _U> + static auto __test_impl(int) -> decltype(__test(declval<_U>())); + + template <typename _U> static void __test_impl(...); + + typedef decltype(__test_impl<_Tp>(0)) type; static const bool value = !is_same<type, void>::value; }; diff --git a/clang/test/Headers/__clang_hip_cmath.hip b/clang/test/Headers/__clang_hip_cmath.hip index ed1030b820627..0c9ff4cdd7808 100644 --- a/clang/test/Headers/__clang_hip_cmath.hip +++ b/clang/test/Headers/__clang_hip_cmath.hip @@ -87,3 +87,22 @@ extern "C" __device__ float test_sin_f32(float x) { extern "C" __device__ float test_cos_f32(float x) { return cos(x); } + +// Check user defined type which can be converted to float and double but not +// specializes __numeric_type will not cause ambiguity diagnostics. +struct user_bfloat16 { + __host__ __device__ user_bfloat16(float); + operator float(); + operator double(); +}; + +namespace user_namespace { + __device__ user_bfloat16 fma(const user_bfloat16 a, const user_bfloat16 b, const user_bfloat16 c) { + return a; + } + + __global__ void test_fma() { + user_bfloat16 a = 1.0f, b = 2.0f; + fma(a, b, b); + } +} 
@yxsamliu yxsamliu merged commit db1d3b2 into llvm:main Aug 2, 2024
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Sep 24, 2024
If there is a type T which can be converted to both float and double etc but itself is not specialized for __numeric_type, and it is called for math functions eg. fma, it will cause ambiguity with test function of __numeric_type. Since test is not template, this error is not bypassed by SFINAE. This is a design flaw of __numeric_type. This patch fixes clang wrapper header to use SFINAE to avoid such ambiguity. Fixes: SWDEV-461604 Fixes: llvm#101239 Change-Id: I6cd22046aaf3c18871d95244a07ecddad23449a7
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Nov 22, 2024
If there is a type T which can be converted to both float and double etc but itself is not specialized for __numeric_type, and it is called for math functions eg. fma, it will cause ambiguity with test function of __numeric_type. Since test is not template, this error is not bypassed by SFINAE. This is a design flaw of __numeric_type. This patch fixes clang wrapper header to use SFINAE to avoid such ambiguity. Fixes: SWDEV-461604 Fixes: llvm#101239 Change-Id: I285adbeb7bf8fe57d084625b98fa6fd49092d641
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics clang Clang issues not falling into any other category

3 participants