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] Fix casting asserts #82827

Merged
merged 3 commits into from
Feb 26, 2024
Merged

Conversation

farzonl
Copy link
Member

@farzonl farzonl commented Feb 23, 2024

There are two issues here. first ICK_Floating_Integral were always defaulting to CK_FloatingToIntegral for vectors regardless of direction of cast. Check was scalar only so added a vec float check to the conditional.

Second issue was float to int casts were resolving to ICK_Integral_Promotion when they need to be resolving to CK_FloatingToIntegral. This was fixed by changing the ordering of conversion checks.

This fixes #82826

@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 Feb 23, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 23, 2024

@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Farzon Lotfi (farzonl)

Changes

There are two issues here. first ICK_Floating_Integral were always defaulting to CK_FloatingToIntegral for vectors regardless of direction of cast. Check was scalar only so added a vec float check to the conditional.

Second issue was float to int casts were resolving to ICK_Integral_Promotion when they need to be resolving to CK_FloatingToIntegral. This was fixed by changing the ordering of conversion checks.

This fixes #82826


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

3 Files Affected:

  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+1-1)
  • (modified) clang/lib/Sema/SemaOverload.cpp (+8-8)
  • (modified) clang/test/SemaHLSL/VectorOverloadResolution.hlsl (+44)
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 322bd1c87b1d71..99f8abf77c0cb6 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4843,7 +4843,7 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
                  .get();
       break;
     case ICK_Floating_Integral:
-      if (ToType->isRealFloatingType())
+      if (ToType->isRealFloatingType() || ToType->hasFloatingRepresentation())
         From =
             ImpCastExprToType(From, ToType, CK_IntegralToFloating, VK_PRValue,
                               /*BasePath=*/nullptr, CCK)
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index f7645422348b65..324a7b20113fcd 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1883,9 +1883,16 @@ static bool IsVectorElementConversion(Sema &S, QualType FromType,
     ICK = ICK_Boolean_Conversion;
     return true;
   }
+  
+  if ((FromType->isRealFloatingType() && ToType->isIntegralType(S.Context)) ||
+      (FromType->isIntegralOrUnscopedEnumerationType() &&
+       ToType->isRealFloatingType())) {
+    ICK = ICK_Floating_Integral;
+    return true;
+  }
 
   if (S.IsIntegralPromotion(From, FromType, ToType)) {
-    ICK = ICK_Integral_Promotion;
+    ICK =  ICK_Integral_Promotion;
     return true;
   }
 
@@ -1895,13 +1902,6 @@ static bool IsVectorElementConversion(Sema &S, QualType FromType,
     return true;
   }
 
-  if ((FromType->isRealFloatingType() && ToType->isIntegralType(S.Context)) ||
-      (FromType->isIntegralOrUnscopedEnumerationType() &&
-       ToType->isRealFloatingType())) {
-    ICK = ICK_Floating_Integral;
-    return true;
-  }
-
   return false;
 }
 
diff --git a/clang/test/SemaHLSL/VectorOverloadResolution.hlsl b/clang/test/SemaHLSL/VectorOverloadResolution.hlsl
index e07391f803f899..d8794ef675768b 100644
--- a/clang/test/SemaHLSL/VectorOverloadResolution.hlsl
+++ b/clang/test/SemaHLSL/VectorOverloadResolution.hlsl
@@ -1,4 +1,8 @@
 // RUN: %clang_cc1 -triple dxil-unknown-shadermodel6.6-library -S -fnative-half-type -finclude-default-header -o - -ast-dump %s | FileCheck %s
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.3-library %s -fnative-half-type \
+// RUN:   -emit-llvm -disable-llvm-passes -O3 -o - | FileCheck %s \ 
+// RUN:   --check-prefixes=CHECKIR
 void Fn(double2 D);
 void Fn(half2 H);
 
@@ -28,3 +32,43 @@ void Fn2(int16_t2 S);
 void Call2(int2 I) {
   Fn2(I);
 }
+
+void Fn3( int64_t2 p0);
+
+// CHECK: FunctionDecl {{.*}} Call3 'void (half2)'
+// CHECK: CallExpr {{.*}} 'void'
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(int64_t2)' <FunctionToPointerDecay>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'void (int64_t2)' lvalue Function {{.*}} 'Fn3' 'void (int64_t2)'
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'int64_t2':'long __attribute__((ext_vector_type(2)))' <FloatingToIntegral>
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'half2':'half __attribute__((ext_vector_type(2)))' <LValueToRValue>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'half2':'half __attribute__((ext_vector_type(2)))' lvalue ParmVar {{.*}} 'p0' 'half2':'half __attribute__((ext_vector_type(2)))'
+// CHECKIR: %conv = fptosi <2 x half> %0 to <2 x i64>
+void Call3(half2 p0) {
+  Fn3(p0);
+}
+
+// CHECK: FunctionDecl {{.*}} Call4 'void (float2)'
+// CHECK: CallExpr {{.*}} 'void'
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(int64_t2)' <FunctionToPointerDecay>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'void (int64_t2)' lvalue Function {{.*}} 'Fn3' 'void (int64_t2)'
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'int64_t2':'long __attribute__((ext_vector_type(2)))' <FloatingToIntegral>
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'float2':'float __attribute__((ext_vector_type(2)))' <LValueToRValue>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'float2':'float __attribute__((ext_vector_type(2)))' lvalue ParmVar {{.*}} 'p0' 'float2':'float __attribute__((ext_vector_type(2)))'
+// CHECKIR: %conv = fptosi <2 x float> %0 to <2 x i64>
+void Call4(float2 p0) {
+  Fn3(p0);
+}
+
+void Fn4( float2 p0);
+
+// CHECK: FunctionDecl {{.*}} Call5 'void (int64_t2)'
+// CHECK: CallExpr {{.*}} 'void'
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(float2)' <FunctionToPointerDecay>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'void (float2)' lvalue Function {{.*}} 'Fn4' 'void (float2)'
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'float2':'float __attribute__((ext_vector_type(2)))' <IntegralToFloating>
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'int64_t2':'long __attribute__((ext_vector_type(2)))' <LValueToRValue>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'int64_t2':'long __attribute__((ext_vector_type(2)))' lvalue ParmVar {{.*}} 'p0' 'int64_t2':'long __attribute__((ext_vector_type(2)))'
+// CHECKIR: %conv = sitofp <2 x i64> %0 to <2 x float>
+void Call5(int64_t2 p0) {
+  Fn4(p0);
+}

Copy link

github-actions bot commented Feb 23, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

There are two issues here. first ICK_Floating_Integral were always
defaulting to CK_FloatingToIntegral for vectors regardless of  direction
of cast. Check was scalar only so added a vec float check to the
conditional.
Second issue was float to int  casts were resolving to ICK_Integral_Promotion
when they need to be resolving to CK_FloatingToIntegral. This was fixed
by changing the ordering of conversion checks.

This fixes llvm#82826
clang/test/SemaHLSL/VectorOverloadResolution.hlsl Outdated Show resolved Hide resolved
clang/lib/Sema/SemaExprCXX.cpp Outdated Show resolved Hide resolved
There are two issues here. first ICK_Floating_Integral were always
defaulting to CK_FloatingToIntegral for vectors regardless of  direction
of cast. Check was scalar only so added a vec float check to the
conditional.
Second issue was float to int  casts were resolving to ICK_Integral_Promotion
when they need to be resolving to CK_FloatingToIntegral. This was fixed
by changing the ordering of conversion checks.

This fixes llvm#82826
@llvm-beanz llvm-beanz merged commit a870a48 into llvm:main Feb 26, 2024
3 of 4 checks passed
@farzonl farzonl deleted the bugfix/hlsl-vec-overload branch February 29, 2024 18:44
@farzonl farzonl self-assigned this Mar 31, 2024
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: Done
Development

Successfully merging this pull request may close these issues.

[HLSL] vector float to int and vec int to float are asserting
5 participants