Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HLSL] Shore up floating point conversions #90222

Merged

Conversation

llvm-beanz
Copy link
Collaborator

This PR fixes bugs in HLSL floating conversions. HLSL always has half, float and double types, which promote in the order:

half->float->double

and convert in the order:

double->float->half

As with other conversions in C++, promotions are preferred over conversions.

We do have floating conversions documented in the draft language specification (https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf [Conv.rank.float]) although the exact language is still in flux (microsoft/hlsl-specs#206).

Resolves #81047

This PR fixes bugs in HLSL floating conversions. HLSL always has
`half`, `float` and `double` types, which promote in the order:

`half`->`float`->`double`

and convert in the order:

`double`->`float`->`half`

As with other conversions in C++, promotions are preferred over
conversions.

We do have floating conversions documented in the draft language
specification (https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf
[Conv.rank.float]) although the exact language is still in flux
(microsoft/hlsl-specs#206).

Resolves llvm#81047
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Apr 26, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 26, 2024

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Chris B (llvm-beanz)

Changes

This PR fixes bugs in HLSL floating conversions. HLSL always has half, float and double types, which promote in the order:

half->float->double

and convert in the order:

double->float->half

As with other conversions in C++, promotions are preferred over conversions.

We do have floating conversions documented in the draft language specification (https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf [Conv.rank.float]) although the exact language is still in flux (microsoft/hlsl-specs#206).

Resolves #81047


Patch is 24.98 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90222.diff

3 Files Affected:

  • (modified) clang/lib/Sema/SemaOverload.cpp (+42-1)
  • (added) clang/test/SemaHLSL/ScalarOverloadResolution.hlsl (+229)
  • (added) clang/test/SemaHLSL/VectorElementOverloadResolution.hlsl (+228)
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 04cd9e78739d20..a416df2e97c439 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -2587,7 +2587,8 @@ bool Sema::IsIntegralPromotion(Expr *From, QualType FromType, QualType ToType) {
 
   // In HLSL an rvalue of integral type can be promoted to an rvalue of a larger
   // integral type.
-  if (Context.getLangOpts().HLSL)
+  if (Context.getLangOpts().HLSL && FromType->isIntegerType() &&
+      ToType->isIntegerType())
     return Context.getTypeSize(FromType) < Context.getTypeSize(ToType);
 
   return false;
@@ -2616,6 +2617,13 @@ bool Sema::IsFloatingPointPromotion(QualType FromType, QualType ToType) {
            ToBuiltin->getKind() == BuiltinType::Ibm128))
         return true;
 
+      // In HLSL, `half` promotes to `float` or `double`, regardless of whether
+      // or not native half types are enabled.
+      if (getLangOpts().HLSL && FromBuiltin->getKind() == BuiltinType::Half &&
+          (ToBuiltin->getKind() == BuiltinType::Float ||
+           ToBuiltin->getKind() == BuiltinType::Double))
+        return true;
+
       // Half can be promoted to float.
       if (!getLangOpts().NativeHalfType &&
            FromBuiltin->getKind() == BuiltinType::Half &&
@@ -4393,6 +4401,24 @@ getFixedEnumPromtion(Sema &S, const StandardConversionSequence &SCS) {
   return FixedEnumPromotion::ToPromotedUnderlyingType;
 }
 
+static ImplicitConversionSequence::CompareKind
+HLSLCompareFloatingRank(QualType LHS, QualType RHS) {
+  assert(LHS->isVectorType() == RHS->isVectorType() &&
+         "Either both elements should be vectors or neither should.");
+  if (const auto *VT = LHS->getAs<VectorType>())
+    LHS = VT->getElementType();
+
+  if (const auto *VT = RHS->getAs<VectorType>())
+    RHS = VT->getElementType();
+
+  const auto L = LHS->getAs<BuiltinType>()->getKind();
+  const auto R = RHS->getAs<BuiltinType>()->getKind();
+  if (L == R)
+    return ImplicitConversionSequence::Indistinguishable;
+  return L < R ? ImplicitConversionSequence::Better
+               : ImplicitConversionSequence::Worse;
+}
+
 /// CompareStandardConversionSequences - Compare two standard
 /// conversion sequences to determine whether one is better than the
 /// other or if they are indistinguishable (C++ 13.3.3.2p3).
@@ -4634,6 +4660,21 @@ CompareStandardConversionSequences(Sema &S, SourceLocation Loc,
                  : ImplicitConversionSequence::Worse;
   }
 
+  if (S.getLangOpts().HLSL) {
+    // On a promotion we prefer the lower rank to disambiguate.
+    if ((SCS1.Second == ICK_Floating_Promotion &&
+         SCS2.Second == ICK_Floating_Promotion) ||
+        (SCS1.Element == ICK_Floating_Promotion &&
+         SCS2.Element == ICK_Floating_Promotion))
+      return HLSLCompareFloatingRank(SCS1.getToType(2), SCS2.getToType(2));
+    // On a conversion we prefer the higher rank to disambiguate.
+    if ((SCS1.Second == ICK_Floating_Conversion &&
+         SCS2.Second == ICK_Floating_Conversion) ||
+        (SCS1.Element == ICK_Floating_Conversion &&
+         SCS2.Element == ICK_Floating_Conversion))
+      return HLSLCompareFloatingRank(SCS2.getToType(2), SCS1.getToType(2));
+  }
+
   return ImplicitConversionSequence::Indistinguishable;
 }
 
diff --git a/clang/test/SemaHLSL/ScalarOverloadResolution.hlsl b/clang/test/SemaHLSL/ScalarOverloadResolution.hlsl
new file mode 100644
index 00000000000000..13758995ec6a1e
--- /dev/null
+++ b/clang/test/SemaHLSL/ScalarOverloadResolution.hlsl
@@ -0,0 +1,229 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -Wconversion -verify -o - %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -ast-dump %s | FileCheck %s
+
+// This test verifies floating point type implicit conversion ranks for overload
+// resolution. In HLSL the built-in type ranks are half < float < double. This
+// applies to both scalar and vector types.
+
+// HLSL allows implicit truncation fo types, so it differentiates between
+// promotions (converting to larger types) and conversions (converting to
+// smaller types). Promotions are preferred over conversions. Promotions prefer
+// promoting to the next lowest type in the ranking order. Conversions prefer
+// converting to the next highest type in the ranking order.
+
+void HalfFloatDouble(double D);
+void HalfFloatDouble(float F);
+void HalfFloatDouble(half H);
+
+// CHECK: FunctionDecl {{.*}} used HalfFloatDouble 'void (double)'
+// CHECK: FunctionDecl {{.*}} used HalfFloatDouble 'void (float)'
+// CHECK: FunctionDecl {{.*}} used HalfFloatDouble 'void (half)'
+
+void FloatDouble(double D);
+void FloatDouble(float F);
+
+// CHECK: FunctionDecl {{.*}} used FloatDouble 'void (double)'
+// CHECK: FunctionDecl {{.*}} used FloatDouble 'void (float)'
+
+void HalfDouble(double D);
+void HalfDouble(half H);
+
+// CHECK: FunctionDecl {{.*}} used HalfDouble 'void (double)'
+// CHECK: FunctionDecl {{.*}} used HalfDouble 'void (half)'
+
+void HalfFloat(float F);
+void HalfFloat(half H);
+
+// CHECK: FunctionDecl {{.*}} used HalfFloat 'void (float)'
+// CHECK: FunctionDecl {{.*}} used HalfFloat 'void (half)'
+
+void Double(double D);
+void Float(float F);
+void Half(half H);
+
+// CHECK: FunctionDecl {{.*}} used Double 'void (double)'
+// CHECK: FunctionDecl {{.*}} used Float 'void (float)'
+// CHECK: FunctionDecl {{.*}} used Half 'void (half)'
+
+
+// Case 1: A function declared with overloads for half float and double types.
+//   (a) When called with half, it will resolve to half because half is an exact
+//   match.
+//   (b) When called with float it will resolve to float because float is an
+//   exact match.
+//   (c) When called with double it will resolve to double because it is an
+//   exact match.
+
+// CHECK: FunctionDecl {{.*}} Case1 'void (half, float, double)'
+void Case1(half H, float F, double D) {
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(half)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (half)' lvalue Function {{.*}} 'HalfFloatDouble' 'void (half)'
+  HalfFloatDouble(H);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(float)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (float)' lvalue Function {{.*}} 'HalfFloatDouble' 'void (float)'
+  HalfFloatDouble(F);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(double)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (double)' lvalue Function {{.*}} 'HalfFloatDouble' 'void (double)'
+  HalfFloatDouble(D);
+}
+
+// Case 2: A function declared with double and float overlaods.
+//   (a) When called with half, it will resolve to float because float is lower
+//   ranked than double.
+//   (b) When called with float it will resolve to float because float is an
+//   exact match.
+//   (c) When called with double it will resolve to double because it is an
+//   exact match.
+
+// CHECK: FunctionDecl {{.*}} Case2 'void (half, float, double)'
+void Case2(half H, float F, double D) {
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(float)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (float)' lvalue Function {{.*}} 'FloatDouble' 'void (float)'
+  FloatDouble(H);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(float)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (float)' lvalue Function {{.*}} 'FloatDouble' 'void (float)'
+  FloatDouble(F);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(double)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (double)' lvalue Function {{.*}} 'FloatDouble' 'void (double)'
+  FloatDouble(D);
+}
+
+// Case 3: A function declared with half and double overloads
+//   (a) When called with half, it will resolve to half because it is an exact
+//   match.
+//   (b) When called with flaot, it will resolve to double because double is a
+//   valid promotion.
+//   (c) When called with double, it will resolve to double because it is an
+//   exact match.
+
+// CHECK: FunctionDecl {{.*}} Case3 'void (half, float, double)'
+void Case3(half H, float F, double D) {
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(half)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (half)' lvalue Function {{.*}} 'HalfDouble' 'void (half)'
+  HalfDouble(H);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(double)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (double)' lvalue Function {{.*}} 'HalfDouble' 'void (double)'
+  HalfDouble(F);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(double)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (double)' lvalue Function {{.*}} 'HalfDouble' 'void (double)'
+  HalfDouble(D);
+}
+
+// Case 4: A function declared with half and float overloads.
+//   (a) When called with half, it will resolve to half because half is an exact
+//   match.
+//   (b) When called with float it will resolve to float because float is an
+//   exact match.
+//   (c) When called with double it will resolve to float because it is the
+//   float is higher rank than half.
+
+// CHECK: FunctionDecl {{.*}} Case4 'void (half, float, double)'
+void Case4(half H, float F, double D) {
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(half)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (half)' lvalue Function {{.*}} 'HalfFloat' 'void (half)'
+  HalfFloat(H);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(float)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (float)' lvalue Function {{.*}} 'HalfFloat' 'void (float)'
+  HalfFloat(F);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(float)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (float)' lvalue Function {{.*}} 'HalfFloat' 'void (float)'
+  HalfFloat(D); // expected-warning{{implicit conversion loses floating-point precision: 'double' to 'float'}}
+}
+
+// Case 5: A function declared with only a double overload.
+//   (a) When called with half, it will resolve to double because double is a
+//   valid promotion.
+//   (b) When called with float it will resolve to double because double is a
+//   valid promotion.
+//   (c) When called with double it will resolve to double because it is an
+//   exact match.
+
+// CHECK: FunctionDecl {{.*}} Case5 'void (half, float, double)'
+void Case5(half H, float F, double D) {
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(double)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (double)' lvalue Function {{.*}} 'Double' 'void (double)'
+  Double(H);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(double)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (double)' lvalue Function {{.*}} 'Double' 'void (double)'
+  Double(F);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(double)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (double)' lvalue Function {{.*}} 'Double' 'void (double)'
+  Double(D);
+}
+
+// Case 6: A function declared with only a float overload.
+//   (a) When called with half, it will resolve to float because float is a
+//   valid promotion.
+//   (b) When called with float it will resolve to float because float is an
+//   exact match.
+//   (c) When called with double it will resolve to float because it is a
+//   valid conversion.
+
+// CHECK: FunctionDecl {{.*}} Case6 'void (half, float, double)'
+void Case6(half H, float F, double D) {
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(float)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (float)' lvalue Function {{.*}} 'Float' 'void (float)'
+  Float(H);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(float)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (float)' lvalue Function {{.*}} 'Float' 'void (float)'
+  Float(F);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(float)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (float)' lvalue Function {{.*}} 'Float' 'void (float)'
+  Float(D); // expected-warning{{implicit conversion loses floating-point precision: 'double' to 'float'}}
+}
+
+// Case 7: A function declared with only a half overload.
+//   (a) When called with half, it will resolve to half because half is an
+//   exact match
+//   (b) When called with float it will resolve to half because half is a
+//   valid conversion.
+//   (c) When called with double it will resolve to float because it is a
+//   valid conversion.
+
+// CHECK: FunctionDecl {{.*}} Case7 'void (half, float, double)'
+void Case7(half H, float F, double D) {
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(half)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (half)' lvalue Function {{.*}} 'Half' 'void (half)'
+  Half(H);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(half)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (half)' lvalue Function {{.*}} 'Half' 'void (half)'
+  Half(F); // expected-warning{{implicit conversion loses floating-point precision: 'float' to 'half'}}
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(half)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (half)' lvalue Function {{.*}} 'Half' 'void (half)'
+  Half(D); // expected-warning{{implicit conversion loses floating-point precision: 'double' to 'half'}}
+}
diff --git a/clang/test/SemaHLSL/VectorElementOverloadResolution.hlsl b/clang/test/SemaHLSL/VectorElementOverloadResolution.hlsl
new file mode 100644
index 00000000000000..78bba54255309b
--- /dev/null
+++ b/clang/test/SemaHLSL/VectorElementOverloadResolution.hlsl
@@ -0,0 +1,228 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -Wconversion -verify -o - %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -fnative-half-type -finclude-default-header -Wno-conversion -ast-dump %s | FileCheck %s
+
+// This test verifies floating point type implicit conversion ranks for overload
+// resolution. In HLSL the built-in type ranks are half < float < double. This
+// applies to both scalar and vector types.
+
+// HLSL allows implicit truncation fo types, so it differentiates between
+// promotions (converting to larger types) and conversions (converting to
+// smaller types). Promotions are preferred over conversions. Promotions prefer
+// promoting to the next lowest type in the ranking order. Conversions prefer
+// converting to the next highest type in the ranking order.
+
+void HalfFloatDouble(double2 D);
+void HalfFloatDouble(float2 F);
+void HalfFloatDouble(half2 H);
+
+// CHECK: FunctionDecl {{.*}} used HalfFloatDouble 'void (double2)'
+// CHECK: FunctionDecl {{.*}} used HalfFloatDouble 'void (float2)'
+// CHECK: FunctionDecl {{.*}} used HalfFloatDouble 'void (half2)'
+
+void FloatDouble(double2 D);
+void FloatDouble(float2 F);
+
+// CHECK: FunctionDecl {{.*}} used FloatDouble 'void (double2)'
+// CHECK: FunctionDecl {{.*}} used FloatDouble 'void (float2)'
+
+void HalfDouble(double2 D);
+void HalfDouble(half2 H);
+
+// CHECK: FunctionDecl {{.*}} used HalfDouble 'void (double2)'
+// CHECK: FunctionDecl {{.*}} used HalfDouble 'void (half2)'
+
+void HalfFloat(float2 F);
+void HalfFloat(half2 H);
+
+// CHECK: FunctionDecl {{.*}} used HalfFloat 'void (float2)'
+// CHECK: FunctionDecl {{.*}} used HalfFloat 'void (half2)'
+
+void Double(double2 D);
+void Float(float2 F);
+void Half(half2 H);
+
+// CHECK: FunctionDecl {{.*}} used Double 'void (double2)'
+// CHECK: FunctionDecl {{.*}} used Float 'void (float2)'
+// CHECK: FunctionDecl {{.*}} used Half 'void (half2)'
+
+// Case 1: A function declared with overloads for half float and double types.
+//   (a) When called with half, it will resolve to half because half is an exact
+//   match.
+//   (b) When called with float it will resolve to float because float is an
+//   exact match.
+//   (c) When called with double it will resolve to double because it is an
+//   exact match.
+
+// CHECK: FunctionDecl {{.*}} Case1 'void (half2, float2, double2)'
+void Case1(half2 H, float2 F, double2 D) {
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(half2)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (half2)' lvalue Function {{.*}} 'HalfFloatDouble' 'void (half2)'
+  HalfFloatDouble(H);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(float2)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (float2)' lvalue Function {{.*}} 'HalfFloatDouble' 'void (float2)'
+  HalfFloatDouble(F);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(double2)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (double2)' lvalue Function {{.*}} 'HalfFloatDouble' 'void (double2)'
+  HalfFloatDouble(D);
+}
+
+// Case 2: A function declared with double and float overlaods.
+//   (a) When called with half, it will resolve to float because float is lower
+//   ranked than double.
+//   (b) When called with float it will resolve to float because float is an
+//   exact match.
+//   (c) When called with double it will resolve to double because it is an
+//   exact match.
+
+// CHECK: FunctionDecl {{.*}} Case2 'void (half2, float2, double2)'
+void Case2(half2 H, float2 F, double2 D) {
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(float2)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (float2)' lvalue Function {{.*}} 'FloatDouble' 'void (float2)'
+  FloatDouble(H);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(float2)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (float2)' lvalue Function {{.*}} 'FloatDouble' 'void (float2)'
+  FloatDouble(F);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(double2)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (double2)' lvalue Function {{.*}} 'FloatDouble' 'void (double2)'
+  FloatDouble(D);
+}
+
+// Case 3: A function declared with half and double overloads
+//   (a) When called with half, it will resolve to half because it is an exact
+//   match.
+//   (b) When called with flaot, it will resolve to double because double is a
+//   valid promotion.
+//   (c) When called with double, it will resolve to double because it is an
+//   exact match.
+
+// CHECK: FunctionDecl {{.*}} Case3 'void (half2, float2, double2)'
+void Case3(half2 H, float2 F, double2 D) {
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(half2)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (half2)' lvalue Function {{.*}} 'HalfDouble' 'void (half2)'
+  HalfDouble(H);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(double2)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (double2)' lvalue Function {{.*}} 'HalfDouble' 'void (double2)'
+  HalfDouble(F);
+
+  // CHECK: CallExpr {{.*}} 'void'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(double2)' <FunctionToPointerDecay>
+  // CHECK-NEXT: DeclRefExpr {{.*}} 'void (double2)' lvalue Function {{.*}} 'HalfDouble' 'void (double2)'
+  HalfDouble(D);
+}
+
+// Case 4: A function declared with half and float overloads.
+//   (a) When called with half, it will resolve to half because half is an exact
+//   match.
+//   (b) When called with float it will resolve to float because float is an
+//   exact match.
+// ...
[truncated]

void HalfFloatDouble(float F);
void HalfFloatDouble(half H);

// CHECK: FunctionDecl {{.*}} used HalfFloatDouble 'void (double)'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question: are these CHECK: FunctionDecls here as a sanity check, or are they validating something about this change or have some functional purpose to making the test work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, the ones at the top are just sanity checking and could be removed.

The checks for the Case* function decls below force FileCheck to scan up to the declaration, which can improve error reporting if anything goes wrong in the test because it can give you a "last match" marker or show if your match skipped ahead.

cbieneman/hlsl-floating-conversions
// (c) When called with double it will resolve to double because it is an
// exact match.

// CHECK: FunctionDecl {{.*}} Case1 'void (half, float, double)'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would using CHECK-LABEL be a bit better for these Case* checks? I usually find it better for this purpose, since it prevents checks from before the label from erroneously matching content after the label which can make the subsequent error reporting harder to deal with. But perhaps you can't use it due to restrictions on using regex within CHECK-LABEL, which IIRC was an issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, I wasn't relying on any match variables, which is what I usually use CHECK-LABEL for, but it could result in better output on error. I'll make that update.

Copy link
Contributor

@coopp coopp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Took me a while to go through all of the tests. Looks like everything is covered!

@@ -2616,6 +2617,13 @@ bool Sema::IsFloatingPointPromotion(QualType FromType, QualType ToType) {
ToBuiltin->getKind() == BuiltinType::Ibm128))
return true;

// In HLSL, `half` promotes to `float` or `double`, regardless of whether
// or not native half types are enabled.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that half is represented as LLVM IR type half with -enable-16bit-types option specified and is promoted to float without it being specified per this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a difference between the IR and language types. half is always half in the language, which is always a different type form float. This behavior is consistent in earlier language modes and with 202x:

https://godbolt.org/z/3bzoocdrG

When you don't pass -enable-16bit-types, half is 32-bit and conforms to the IEEE single-precision fp32 representation (see: #90694).

@farzonl
Copy link
Member

farzonl commented May 1, 2024

Are there test cases in OverloadResolutionBugs.hlsl that have been fixed by this change and would need to be removed from that file?

cbieneman/hlsl-floating-conversions
by 270 commits.
../clang/test/SemaHLSL/VectorElementOverloadResolution.hlsl
@llvm-beanz llvm-beanz merged commit aa5ff68 into llvm:main May 2, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[HLSL] Overload resolution should promote 16-bit types
7 participants