Updates based on review feedback

llvm-beanz · llvm-beanz · commit c5430b391d40 · 2024-02-07T16:47:33.000-06:00
This addresses feedback from @rjmcall. The main updates are: * Added asserts and todos about handling constrained intrinsics. * Fixed sufflevector code generation (and updated tests) * Fixed shadowed promotion casts. Writing a test for this revealed another issue. * Added Element conversion to conversion rank checking, and added test to verify promotion is preferred over conversion. * Added HLSL integral promotion check. There are still at least two outstanding bugs that are exposed by this change which I've added an XFAIL'd test for (llvm#81047 & llvm#81049). HLSL's promotion rules are not well written down, but specifically in overload resolution integer and floating point values can implicitly cast to larger types and the smallest of the larger types is the best match.
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -2421,7 +2421,10 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
                                 CE->getExprLoc(), Opts);
   }
   case CK_IntegralToFloating: {
-    if (E->getType()->isExtVectorType() && DestTy->isExtVectorType()) {
+    if (E->getType()->isVectorType() && DestTy->isVectorType()) {
+      // TODO: Support constrained FP intrinsics.
+      assert(!Builder.getIsFPConstrained() &&
+             "FP Constrained vector casts not supported yet.");
       QualType SrcElTy = E->getType()->castAs<VectorType>()->getElementType();
       if (SrcElTy->isSignedIntegerOrEnumerationType())
         return Builder.CreateSIToFP(Visit(E), ConvertType(DestTy), "conv");
@@ -2432,7 +2435,10 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
                                 CE->getExprLoc());
   }
   case CK_FloatingToIntegral: {
-    if (E->getType()->isExtVectorType() && DestTy->isExtVectorType()) {
+    if (E->getType()->isVectorType() && DestTy->isVectorType()) {
+      // TODO: Support constrained FP intrinsics.
+      assert(!Builder.getIsFPConstrained() &&
+             "FP Constrained vector casts not supported yet.");
       QualType DstElTy = DestTy->castAs<VectorType>()->getElementType();
       if (DstElTy->isSignedIntegerOrEnumerationType())
         return Builder.CreateFPToSI(Visit(E), ConvertType(DestTy), "conv");
@@ -2443,7 +2449,10 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
                                 CE->getExprLoc());
   }
   case CK_FloatingCast: {
-    if (E->getType()->isExtVectorType() && DestTy->isExtVectorType()) {
+    if (E->getType()->isVectorType() && DestTy->isVectorType()) {
+      // TODO: Support constrained FP intrinsics.
+      assert(!Builder.getIsFPConstrained() &&
+             "FP Constrained vector casts not supported yet.");
       QualType SrcElTy = E->getType()->castAs<VectorType>()->getElementType();
       QualType DstElTy = DestTy->castAs<VectorType>()->getElementType();
       if (DstElTy->castAs<BuiltinType>()->getKind() <
@@ -2508,8 +2517,10 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
     assert(DestTy->isVectorType() && "Expected dest type to be vector type");
     Value *Vec = Visit(const_cast<Expr *>(E));
     SmallVector<int, 16> Mask;
-    Mask.insert(Mask.begin(), DestTy->castAs<VectorType>()->getNumElements(),
-                0);
+    unsigned NumElts = DestTy->castAs<VectorType>()->getNumElements();
+    for (unsigned I = 0; I != NumElts; ++I)
+      Mask.push_back(I);
+    
     return Builder.CreateShuffleVector(Vec, Mask, "trunc");
   }
 
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
@@ -232,6 +232,8 @@ ImplicitConversionRank StandardConversionSequence::getRank() const {
     Rank = GetConversionRank(First);
   if  (GetConversionRank(Second) > Rank)
     Rank = GetConversionRank(Second);
+  if  (GetConversionRank(Element) > Rank)
+    Rank = GetConversionRank(Element);
   if  (GetConversionRank(Third) > Rank)
     Rank = GetConversionRank(Third);
   return Rank;
@@ -1879,13 +1881,13 @@ static bool IsVectorElementConversion(Sema &S, QualType FromType,
   if (S.Context.hasSameUnqualifiedType(FromType, ToType))
     return true;
 
-  if (IsFloatingPointConversion(S, FromType, ToType)) {
-    ICK = ICK_Floating_Conversion;
+  if (S.IsFloatingPointPromotion(FromType, ToType)) {
+    ICK = ICK_Floating_Promotion;
     return true;
   }
 
-  if (S.IsFloatingPointPromotion(FromType, ToType)) {
-    ICK = ICK_Floating_Promotion;
+  if (IsFloatingPointConversion(S, FromType, ToType)) {
+    ICK = ICK_Floating_Conversion;
     return true;
   }
 
@@ -1894,14 +1896,14 @@ static bool IsVectorElementConversion(Sema &S, QualType FromType,
     return true;
   }
 
-  if (FromType->isIntegralOrUnscopedEnumerationType() &&
-      ToType->isIntegralType(S.Context)) {
-    ICK = ICK_Integral_Conversion;
+  if (S.IsIntegralPromotion(From, FromType, ToType)) {
+    ICK = ICK_Integral_Promotion;
     return true;
   }
 
-  if (S.IsIntegralPromotion(From, FromType, ToType)) {
-    ICK = ICK_Integral_Promotion;
+  if (FromType->isIntegralOrUnscopedEnumerationType() &&
+      ToType->isIntegralType(S.Context)) {
+    ICK = ICK_Integral_Conversion;
     return true;
   }
 
@@ -2531,6 +2533,12 @@ bool Sema::IsIntegralPromotion(Expr *From, QualType FromType, QualType ToType) {
     return true;
   }
 
+  // In HLSL an rvalue of integral type can be promoted to an rvalue of a larger
+  // integral type.
+  if (Context.getLangOpts().HLSL)
+    return Context.getTypeSize(FromType) < Context.getTypeSize(ToType);
+
+
   return false;
 }
 
diff --git a/clang/test/CodeGenHLSL/BasicFeatures/standard_conversion_sequences.hlsl b/clang/test/CodeGenHLSL/BasicFeatures/standard_conversion_sequences.hlsl
@@ -18,7 +18,7 @@ void f3_to_d4() {
 // CHECK: [[f2:%.*]] = alloca <2 x float>
 // CHECK: store <3 x float> <float 2.000000e+00, float 2.000000e+00, float 2.000000e+00>, ptr [[f3]]
 // CHECK: [[vecf3:%.*]] = load <3 x float>, ptr [[f3]]
-// CHECK: [[vecf2:%.*]] = shufflevector <3 x float> [[vecf3]], <3 x float> poison, <2 x i32> zeroinitializer
+// CHECK: [[vecf2:%.*]] = shufflevector <3 x float> [[vecf3]], <3 x float> poison, <2 x i32> <i32 0, i32 1>
 // CHECK: store <2 x float> [[vecf2]], ptr [[f2]]
 void f3_to_f2() {
   vector<float,3> f3 = 2.0;
@@ -30,7 +30,7 @@ void f3_to_f2() {
 // CHECK: [[f2:%.*]] = alloca <2 x float>
 // CHECK: store <4 x double> <double 3.000000e+00, double 3.000000e+00, double 3.000000e+00, double 3.000000e+00>, ptr [[d4]]
 // CHECK: [[vecd4:%.*]] = load <4 x double>, ptr [[d4]]
-// CHECK: [[vecd2:%.*]] = shufflevector <4 x double> [[vecd4]], <4 x double> poison, <2 x i32> zeroinitializer
+// CHECK: [[vecd2:%.*]] = shufflevector <4 x double> [[vecd4]], <4 x double> poison, <2 x i32> <i32 0, i32 1>
 // CHECK: [[vecf2:%.*]] = fptrunc <2 x double> [[vecd2]] to <2 x float>
 // CHECK: store <2 x float> [[vecf2]], ptr [[f2]]
 void d4_to_f2() {
@@ -55,7 +55,7 @@ void f2_to_i2() {
 // CHECK: [[i2:%.*]] = alloca <2 x i32>
 // CHECK: store <4 x double> <double 5.000000e+00, double 5.000000e+00, double 5.000000e+00, double 5.000000e+00>, ptr [[d4]]
 // CHECK: [[vecd4:%.*]] = load <4 x double>, ptr [[d4]]
-// CHECK: [[vecd2:%.*]] = shufflevector <4 x double> [[vecd4]], <4 x double> poison, <2 x i32> zeroinitializer
+// CHECK: [[vecd2:%.*]] = shufflevector <4 x double> [[vecd4]], <4 x double> poison, <2 x i32> <i32 0, i32 1>
 // CHECK: [[veci2]] = fptosi <2 x double> [[vecd2]] to <2 x i32>
 // CHECK: store <2 x i32> [[veci2]], ptr [[i2]]
 void d4_to_i2() {
@@ -81,7 +81,7 @@ void d4_to_l4() {
 // CHECK: [[i2:%.*]] = alloca <2 x i32>
 // CHECK: store <4 x i64> <i64 7, i64 7, i64 7, i64 7>, ptr [[l4]]
 // CHECK: [[vecl4:%.*]] = load <4 x i64>, ptr [[l4]]
-// CHECK: [[vecl2:%.*]] = shufflevector <4 x i64> [[vecl4]], <4 x i64> poison, <2 x i32> zeroinitializer
+// CHECK: [[vecl2:%.*]] = shufflevector <4 x i64> [[vecl4]], <4 x i64> poison, <2 x i32> <i32 0, i32 1>
 // CHECK: [[veci2:%.*]] = trunc <2 x i64> [[vecl2]] to <2 x i32>
 // CHECK: store <2 x i32> [[veci2]], ptr [[i2]]
 void l4_to_i2() {
@@ -108,7 +108,7 @@ void i2_to_b2() {
 // CHECK: [[b2:%.*]] = alloca i8
 // CHECK: store <4 x double> <double 9.000000e+00, double 9.000000e+00, double 9.000000e+00, double 9.000000e+00>, ptr [[d4]]
 // CHECK: [[vecd4:%.*]] = load <4 x double>, ptr [[d4]]
-// CHECK: [[vecd2:%.*]] = shufflevector <4 x double> [[vecd4]], <4 x double> poison, <2 x i32> zeroinitializer
+// CHECK: [[vecd2:%.*]] = shufflevector <4 x double> [[vecd4]], <4 x double> poison, <2 x i32> <i32 0, i32 1>
 // CHECK: [[vecb2:%.*]] = fcmp une <2 x double> [[vecd2]], zeroinitializer
 // CHECK: [[vecb8:%.*]] = shufflevector <2 x i1> [[vecb2]], <2 x i1> poison, <8 x i32> <i32 0, i32 1, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
 // CHECK: [[i8:%.*]] = bitcast <8 x i1> [[vecb8]] to i8
diff --git a/clang/test/CodeGenHLSL/builtins/sqrt.hlsl b/clang/test/CodeGenHLSL/builtins/sqrt.hlsl
@@ -1,9 +1,6 @@
 // RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
 // RUN:   dxil-pc-shadermodel6.2-library %s -fnative-half-type \
 // RUN:   -emit-llvm -disable-llvm-passes -o - | FileCheck %s
-// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
-// RUN:   dxil-pc-shadermodel6.2-library %s -emit-llvm -disable-llvm-passes \
-// RUN:   -o - | FileCheck %s --check-prefix=NO_HALF
 
 using hlsl::sqrt;
 
@@ -30,5 +27,3 @@ half sqrt_h(half x)
 
 // CHECK: define noundef half @"?sqrt_h@@YA$f16@$f16@@Z"(
 // CHECK: call half @llvm.sqrt.f16(half %0)
-// NO_HALF: define noundef float @"?sqrt_h@@YA$halff@$halff@@Z"(
-// NO_HALF: call float @llvm.sqrt.f32(float %0)
diff --git a/clang/test/SemaHLSL/OverloadResolutionBugs.hlsl b/clang/test/SemaHLSL/OverloadResolutionBugs.hlsl
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -o - -fsyntax-only %s -verify
+// XFAIL: *
+
+// https://github.com/llvm/llvm-project/issues/81047
+
+// expected-no-diagnostics
+void Fn3(double2 D);
+void Fn3(float2 F);
+
+void Call3(half2 H) {
+  Fn3(H);
+}
+
+void Fn4(int64_t2 L);
+void Fn4(int2 I);
+
+void Call4(int16_t H) {
+  Fn4(H);
+}
+
+// https://github.com/llvm/llvm-project/issues/81049
+
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple \
+// RUN:   dxil-pc-shadermodel6.2-library %s -emit-llvm -disable-llvm-passes \
+// RUN:   -o - | FileCheck %s --check-prefix=NO_HALF
+
+half sqrt_h(half x)
+{
+  return sqrt(x);
+}
+
+// NO_HALF: define noundef float @"?sqrt_h@@YA$halff@$halff@@Z"(
+// NO_HALF: call float @llvm.sqrt.f32(float %0)
diff --git a/clang/test/SemaHLSL/VectorOverloadResolution.hlsl b/clang/test/SemaHLSL/VectorOverloadResolution.hlsl
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple dxil-unknown-shadermodel6.6-library -S -fnative-half-type -finclude-default-header -o - -ast-dump %s | FileCheck %s
+void Fn(double2 D);
+void Fn(half2 H);
+
+// CHECK: FunctionDecl {{.*}} Call 'void (float2)'
+// CHECK: CallExpr {{.*}}'void'
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(double2)' <FunctionToPointerDecay>
+// CHECK-NEXT: DeclRefExpr {{.*}}'void (double2)' lvalue Function {{.*}} 'Fn' 'void (double2)'
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'double2':'double __attribute__((ext_vector_type(2)))' <FloatingCast>
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'float2':'float __attribute__((ext_vector_type(2)))' <LValueToRValue>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'float2':'float __attribute__((ext_vector_type(2)))' lvalue ParmVar {{.*}} 'F' 'float2':'float __attribute__((ext_vector_type(2)))'
+
+void Call(float2 F) {
+  Fn(F);
+}
+
+void Fn2(int64_t2 L);
+void Fn2(int16_t2 S);
+
+// CHECK: FunctionDecl {{.*}} Call2 'void (int2)'
+// CHECK: CallExpr {{.*}} 'void'
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void (*)(int64_t2)' <FunctionToPointerDecay>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'void (int64_t2)' lvalue Function {{.*}} 'Fn2' 'void (int64_t2)'
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'int64_t2':'long __attribute__((ext_vector_type(2)))' <IntegralCast>
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'int2':'int __attribute__((ext_vector_type(2)))' <LValueToRValue>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'int2':'int __attribute__((ext_vector_type(2)))' lvalue ParmVar {{.*}} 'I' 'int2':'int __attribute__((ext_vector_type(2)))'
+
+void Call2(int2 I) {
+  Fn2(I);
+}
