Skip to content

[HLSL] Rework implicit conversion sequences #96011

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

Merged
merged 3 commits into from
Jul 13, 2024

Conversation

llvm-beanz
Copy link
Collaborator

This PR reworks HLSL's implicit conversion sequences. Initially I was seeking to match DXC's behavior more closely, but that was leading to a pile of special case rules to tie-break ambiguous cases that should really be left as ambiguous. We've decided that we're going to break compatibility with DXC here, and we may port this new behavior over to DXC instead.

This change is a bit closer to C++'s overload resolution rules, but it does have a bit of nuance around how dimension adjustment conversions are ranked. Conversion sequence ranks for HLSL are:

  • Exact match
  • Scalar Widening (i.e. splat)
  • Promotion
  • Scalar Widening with Promotion
  • Conversion
  • Scalar Widening with Conversion
  • Dimension Reduction (i.e. truncation)
  • Dimension Reduction with Promotion
  • Dimension Reduction with Conversion

In this implementation I've folded the disambiguation into the conversion sequence ranks which does add some complexity as compared to C++, however this avoids needing to add special casing in CompareStandardConversionSequences. I believe the added conversion rank values provide a simpler approach, but feedback is appreciated.

The HLSL language spec updates are in the PR here: microsoft/hlsl-specs#261

@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 Jun 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 19, 2024

@llvm/pr-subscribers-clang

Author: Chris B (llvm-beanz)

Changes

This PR reworks HLSL's implicit conversion sequences. Initially I was seeking to match DXC's behavior more closely, but that was leading to a pile of special case rules to tie-break ambiguous cases that should really be left as ambiguous. We've decided that we're going to break compatibility with DXC here, and we may port this new behavior over to DXC instead.

This change is a bit closer to C++'s overload resolution rules, but it does have a bit of nuance around how dimension adjustment conversions are ranked. Conversion sequence ranks for HLSL are:

  • Exact match
  • Scalar Widening (i.e. splat)
  • Promotion
  • Scalar Widening with Promotion
  • Conversion
  • Scalar Widening with Conversion
  • Dimension Reduction (i.e. truncation)
  • Dimension Reduction with Promotion
  • Dimension Reduction with Conversion

In this implementation I've folded the disambiguation into the conversion sequence ranks which does add some complexity as compared to C++, however this avoids needing to add special casing in CompareStandardConversionSequences. I believe the added conversion rank values provide a simpler approach, but feedback is appreciated.

The HLSL language spec updates are in the PR here: microsoft/hlsl-specs#261


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

15 Files Affected:

  • (modified) clang/docs/HLSL/ExpectedDifferences.rst (+11)
  • (modified) clang/include/clang/Sema/Overload.h (+31-7)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+79-84)
  • (modified) clang/lib/Sema/SemaOverload.cpp (+39-46)
  • (modified) clang/test/CodeGenHLSL/BasicFeatures/standard_conversion_sequences.hlsl (+9-9)
  • (modified) clang/test/CodeGenHLSL/builtins/dot.hlsl (+1-33)
  • (modified) clang/test/CodeGenHLSL/builtins/lerp.hlsl (-24)
  • (modified) clang/test/CodeGenHLSL/builtins/mad.hlsl (-22)
  • (modified) clang/test/SemaHLSL/ScalarOverloadResolution.hlsl (+13-15)
  • (added) clang/test/SemaHLSL/SplatOverloadResolution.hlsl (+166)
  • (added) clang/test/SemaHLSL/TruncationOverloadResolution.hlsl (+68)
  • (modified) clang/test/SemaHLSL/Types/BuiltinVector/ScalarSwizzles.hlsl (+1-1)
  • (modified) clang/test/SemaHLSL/VectorElementOverloadResolution.hlsl (+16-20)
  • (modified) clang/test/SemaHLSL/VectorOverloadResolution.hlsl (+5-5)
  • (modified) clang/test/SemaHLSL/standard_conversion_sequences.hlsl (+8-8)
diff --git a/clang/docs/HLSL/ExpectedDifferences.rst b/clang/docs/HLSL/ExpectedDifferences.rst
index d1b6010f10f43..a29b6348e0b8e 100644
--- a/clang/docs/HLSL/ExpectedDifferences.rst
+++ b/clang/docs/HLSL/ExpectedDifferences.rst
@@ -67,12 +67,16 @@ behavior between Clang and DXC. Some examples include:
   void takesDoubles(double, double, double);
 
   cbuffer CB {
+    bool B;
     uint U;
     int I;
     float X, Y, Z;
     double3 A, B;
   }
 
+  void twoParams(int, int);
+  void twoParams(float, float);
+
   export void call() {
     halfOrInt16(U); // DXC: Fails with call ambiguous between int16_t and uint16_t overloads
                     // Clang: Resolves to halfOrInt16(uint16_t).
@@ -98,6 +102,13 @@ behavior between Clang and DXC. Some examples include:
                           // FXC: Expands to compute double dot product with fmul/fadd
                           // Clang: Resolves to dot(float3, float3), emits conversion warnings.
 
+  #ifndef IGNORE_ERRORS
+    tan(B); // DXC: resolves to tan(float).
+            // Clang: Fails to resolve, ambiguous between integer types.
+
+    twoParams(I, X); // DXC: resolves twoParams(int, int).
+                     // Clang: Fails to resolve ambiguous conversions.
+  #endif
   }
 
 .. note::
diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h
index 4a5c9e8ca1229..9d8b797af6663 100644
--- a/clang/include/clang/Sema/Overload.h
+++ b/clang/include/clang/Sema/Overload.h
@@ -201,6 +201,9 @@ class Sema;
     /// HLSL non-decaying array rvalue cast.
     ICK_HLSL_Array_RValue,
 
+    // HLSL vector splat from scalar or boolean type.
+    ICK_HLSL_Vector_Splat,
+
     /// The number of conversion kinds
     ICK_Num_Conversion_Kinds,
   };
@@ -213,15 +216,27 @@ class Sema;
     /// Exact Match
     ICR_Exact_Match = 0,
 
+    /// HLSL Scalar Widening
+    ICR_HLSL_Scalar_Widening,
+
     /// Promotion
     ICR_Promotion,
 
+    /// HLSL Scalar Widening with promotion
+    ICR_HLSL_Scalar_Widening_Promotion,
+
+    /// HLSL Matching Dimension Reduction
+    ICR_HLSL_Dimension_Reduction,
+
     /// Conversion
     ICR_Conversion,
 
     /// OpenCL Scalar Widening
     ICR_OCL_Scalar_Widening,
 
+    /// HLSL Scalar Widening with conversion
+    ICR_HLSL_Scalar_Widening_Conversion,
+
     /// Complex <-> Real conversion
     ICR_Complex_Real_Conversion,
 
@@ -233,11 +248,21 @@ class Sema;
 
     /// Conversion not allowed by the C standard, but that we accept as an
     /// extension anyway.
-    ICR_C_Conversion_Extension
+    ICR_C_Conversion_Extension,
+
+    /// HLSL Dimension reduction with promotion
+    ICR_HLSL_Dimension_Reduction_Promotion,
+
+    /// HLSL Dimension reduction with conversion
+    ICR_HLSL_Dimension_Reduction_Conversion,
   };
 
   ImplicitConversionRank GetConversionRank(ImplicitConversionKind Kind);
 
+  ImplicitConversionRank
+  GetDimensionConversionRank(ImplicitConversionRank Base,
+                             ImplicitConversionKind Dimension);
+
   /// NarrowingKind - The kind of narrowing conversion being performed by a
   /// standard conversion sequence according to C++11 [dcl.init.list]p7.
   enum NarrowingKind {
@@ -277,11 +302,10 @@ class Sema;
     /// pointer-to-member conversion, or boolean conversion.
     ImplicitConversionKind Second : 8;
 
-    /// Element - Between the second and third conversion a vector or matrix
-    /// element conversion may occur. If this is not ICK_Identity this
-    /// conversion is applied element-wise to each element in the vector or
-    /// matrix.
-    ImplicitConversionKind Element : 8;
+    /// Dimension - Between the second and third conversion a vector or matrix
+    /// dimension conversion may occur. If this is not ICK_Identity this
+    /// conversion truncates the vector or matrix, or extends a scalar.
+    ImplicitConversionKind Dimension : 8;
 
     /// Third - The third conversion can be a qualification conversion
     /// or a function conversion.
@@ -379,7 +403,7 @@ class Sema;
     void setAsIdentityConversion();
 
     bool isIdentityConversion() const {
-      return Second == ICK_Identity && Element == ICK_Identity &&
+      return Second == ICK_Identity && Dimension == ICK_Identity &&
              Third == ICK_Identity;
     }
 
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index f3af8dee6b090..7b955cb7a8e24 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4361,6 +4361,19 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
   return From;
 }
 
+// GetIntermediateVectorType - Compute the intermediate cast type casting
+// elements of the from type to the elements of the to type without resizing the
+// vector.
+static QualType adjustVectorType(ASTContext &Context, QualType FromTy,
+                                 QualType ToType) {
+  auto *ToVec = ToType->castAs<VectorType>();
+  QualType ElType = ToVec->getElementType();
+  if (!FromTy->isVectorType())
+    return ElType;
+  auto *FromVec = FromTy->castAs<VectorType>();
+  return Context.getExtVectorType(ElType, FromVec->getNumElements());
+}
+
 /// PerformImplicitConversion - Perform an implicit conversion of the
 /// expression From to the type ToType by following the standard
 /// conversion sequence SCS. Returns the converted
@@ -4515,27 +4528,38 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
     break;
 
   case ICK_Integral_Promotion:
-  case ICK_Integral_Conversion:
-    if (ToType->isBooleanType()) {
+  case ICK_Integral_Conversion: {
+    QualType ElTy = ToType;
+    QualType StepTy = ToType;
+    if (ToType->isVectorType()) {
+      StepTy = adjustVectorType(Context, FromType, ToType);
+      ElTy = StepTy->castAs<VectorType>()->getElementType();
+    }
+    if (ElTy->isBooleanType()) {
       assert(FromType->castAs<EnumType>()->getDecl()->isFixed() &&
              SCS.Second == ICK_Integral_Promotion &&
              "only enums with fixed underlying type can promote to bool");
-      From = ImpCastExprToType(From, ToType, CK_IntegralToBoolean, VK_PRValue,
+      From = ImpCastExprToType(From, StepTy, CK_IntegralToBoolean, VK_PRValue,
                                /*BasePath=*/nullptr, CCK)
                  .get();
     } else {
-      From = ImpCastExprToType(From, ToType, CK_IntegralCast, VK_PRValue,
+      From = ImpCastExprToType(From, StepTy, CK_IntegralCast, VK_PRValue,
                                /*BasePath=*/nullptr, CCK)
                  .get();
     }
     break;
+  }
 
   case ICK_Floating_Promotion:
-  case ICK_Floating_Conversion:
-    From = ImpCastExprToType(From, ToType, CK_FloatingCast, VK_PRValue,
+  case ICK_Floating_Conversion: {
+    QualType StepTy = ToType;
+    if (ToType->isVectorType())
+      StepTy = adjustVectorType(Context, FromType, ToType);
+    From = ImpCastExprToType(From, StepTy, CK_FloatingCast, VK_PRValue,
                              /*BasePath=*/nullptr, CCK)
                .get();
     break;
+  }
 
   case ICK_Complex_Promotion:
   case ICK_Complex_Conversion: {
@@ -4558,16 +4582,23 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
     break;
   }
 
-  case ICK_Floating_Integral:
-    if (ToType->isRealFloatingType())
-      From = ImpCastExprToType(From, ToType, CK_IntegralToFloating, VK_PRValue,
+  case ICK_Floating_Integral: {
+    QualType ElTy = ToType;
+    QualType StepTy = ToType;
+    if (FromType->isVectorType()) {
+      StepTy = adjustVectorType(Context, FromType, ToType);
+      ElTy = StepTy->castAs<VectorType>()->getElementType();
+    }
+    if (ElTy->isRealFloatingType())
+      From = ImpCastExprToType(From, StepTy, CK_IntegralToFloating, VK_PRValue,
                                /*BasePath=*/nullptr, CCK)
                  .get();
     else
-      From = ImpCastExprToType(From, ToType, CK_FloatingToIntegral, VK_PRValue,
+      From = ImpCastExprToType(From, StepTy, CK_FloatingToIntegral, VK_PRValue,
                                /*BasePath=*/nullptr, CCK)
                  .get();
     break;
+  }
 
   case ICK_Fixed_Point_Conversion:
     assert((FromType->isFixedPointType() || ToType->isFixedPointType()) &&
@@ -4689,18 +4720,26 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
     break;
   }
 
-  case ICK_Boolean_Conversion:
+  case ICK_Boolean_Conversion: {
     // Perform half-to-boolean conversion via float.
     if (From->getType()->isHalfType()) {
       From = ImpCastExprToType(From, Context.FloatTy, CK_FloatingCast).get();
       FromType = Context.FloatTy;
     }
+    QualType ElTy = FromType;
+    QualType StepTy = ToType;
+    if (FromType->isVectorType()) {
+      if (getLangOpts().HLSL)
+        StepTy = adjustVectorType(Context, FromType, ToType);
+      ElTy = FromType->castAs<VectorType>()->getElementType();
+    }
 
-    From = ImpCastExprToType(From, Context.BoolTy,
-                             ScalarTypeToBooleanCastKind(FromType), VK_PRValue,
+    From = ImpCastExprToType(From, StepTy,
+                             ScalarTypeToBooleanCastKind(ElTy), VK_PRValue,
                              /*BasePath=*/nullptr, CCK)
                .get();
     break;
+  }
 
   case ICK_Derived_To_Base: {
     CXXCastPath BasePath;
@@ -4826,22 +4865,6 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
                              CK_ZeroToOCLOpaqueType,
                              From->getValueKind()).get();
     break;
-  case ICK_HLSL_Vector_Truncation: {
-    // Note: HLSL built-in vectors are ExtVectors. Since this truncates a vector
-    // to a smaller vector, this can only operate on arguments where the source
-    // and destination types are ExtVectors.
-    assert(From->getType()->isExtVectorType() && ToType->isExtVectorType() &&
-           "HLSL vector truncation should only apply to ExtVectors");
-    auto *FromVec = From->getType()->castAs<VectorType>();
-    auto *ToVec = ToType->castAs<VectorType>();
-    QualType ElType = FromVec->getElementType();
-    QualType TruncTy =
-        Context.getExtVectorType(ElType, ToVec->getNumElements());
-    From = ImpCastExprToType(From, TruncTy, CK_HLSLVectorTruncation,
-                             From->getValueKind())
-               .get();
-    break;
-  }
 
   case ICK_Lvalue_To_Rvalue:
   case ICK_Array_To_Pointer:
@@ -4852,73 +4875,45 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
   case ICK_C_Only_Conversion:
   case ICK_Incompatible_Pointer_Conversion:
   case ICK_HLSL_Array_RValue:
+  case ICK_HLSL_Vector_Truncation:
+  case ICK_HLSL_Vector_Splat:
     llvm_unreachable("Improper second standard conversion");
   }
 
-  if (SCS.Element != ICK_Identity) {
+  if (SCS.Dimension != ICK_Identity) {
     // If SCS.Element is not ICK_Identity the To and From types must be HLSL
     // vectors or matrices.
 
     // TODO: Support HLSL matrices.
     assert((!From->getType()->isMatrixType() && !ToType->isMatrixType()) &&
-           "Element conversion for matrix types is not implemented yet.");
-    assert(From->getType()->isVectorType() && ToType->isVectorType() &&
-           "Element conversion is only supported for vector types.");
-    assert(From->getType()->getAs<VectorType>()->getNumElements() ==
-               ToType->getAs<VectorType>()->getNumElements() &&
-           "Element conversion is only supported for vectors with the same "
-           "element counts.");
-    QualType FromElTy = From->getType()->getAs<VectorType>()->getElementType();
-    unsigned NumElts = ToType->getAs<VectorType>()->getNumElements();
-    switch (SCS.Element) {
-    case ICK_Boolean_Conversion:
-      // Perform half-to-boolean conversion via float.
-      if (FromElTy->isHalfType()) {
-        QualType FPExtType = Context.getExtVectorType(FromElTy, NumElts);
-        From = ImpCastExprToType(From, FPExtType, CK_FloatingCast).get();
-        FromType = FPExtType;
-      }
-
-      From =
-          ImpCastExprToType(From, ToType, ScalarTypeToBooleanCastKind(FromElTy),
-                            VK_PRValue,
-                            /*BasePath=*/nullptr, CCK)
-              .get();
-      break;
-    case ICK_Integral_Promotion:
-    case ICK_Integral_Conversion:
-      if (ToType->isBooleanType()) {
-        assert(FromType->castAs<EnumType>()->getDecl()->isFixed() &&
-               SCS.Second == ICK_Integral_Promotion &&
-               "only enums with fixed underlying type can promote to bool");
-        From = ImpCastExprToType(From, ToType, CK_IntegralToBoolean, VK_PRValue,
-                                 /*BasePath=*/nullptr, CCK)
-                   .get();
-      } else {
-        From = ImpCastExprToType(From, ToType, CK_IntegralCast, VK_PRValue,
-                                 /*BasePath=*/nullptr, CCK)
-                   .get();
-      }
-      break;
-
-    case ICK_Floating_Promotion:
-    case ICK_Floating_Conversion:
-      From = ImpCastExprToType(From, ToType, CK_FloatingCast, VK_PRValue,
+           "Dimension conversion for matrix types is not implemented yet.");
+    assert(ToType->isVectorType() &&
+           "Dimension conversion is only supported for vector types.");
+    switch (SCS.Dimension) {
+    case ICK_HLSL_Vector_Splat: {
+      // Vector splat from any arithmetic type to a vector.
+      Expr *Elem = prepareVectorSplat(ToType, From).get();
+      From = ImpCastExprToType(Elem, ToType, CK_VectorSplat, VK_PRValue,
                                /*BasePath=*/nullptr, CCK)
                  .get();
       break;
-    case ICK_Floating_Integral:
-      if (ToType->hasFloatingRepresentation())
-        From =
-            ImpCastExprToType(From, ToType, CK_IntegralToFloating, VK_PRValue,
-                              /*BasePath=*/nullptr, CCK)
-                .get();
-      else
-        From =
-            ImpCastExprToType(From, ToType, CK_FloatingToIntegral, VK_PRValue,
-                              /*BasePath=*/nullptr, CCK)
-                .get();
+    }
+    case ICK_HLSL_Vector_Truncation: {
+      // Note: HLSL built-in vectors are ExtVectors. Since this truncates a
+      // vector to a smaller vector, this can only operate on arguments where
+      // the source and destination types are ExtVectors.
+      assert(From->getType()->isExtVectorType() && ToType->isExtVectorType() &&
+             "HLSL vector truncation should only apply to ExtVectors");
+      auto *FromVec = From->getType()->castAs<VectorType>();
+      auto *ToVec = ToType->castAs<VectorType>();
+      QualType ElType = FromVec->getElementType();
+      QualType TruncTy =
+          Context.getExtVectorType(ElType, ToVec->getNumElements());
+      From = ImpCastExprToType(From, TruncTy, CK_HLSLVectorTruncation,
+                               From->getValueKind())
+                 .get();
       break;
+    }
     case ICK_Identity:
     default:
       llvm_unreachable("Improper element standard conversion");
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index fb4ff72e42eb5..e17b62f32626a 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -163,13 +163,33 @@ ImplicitConversionRank clang::GetConversionRank(ImplicitConversionKind Kind) {
       ICR_C_Conversion,
       ICR_C_Conversion_Extension,
       ICR_Conversion,
+      ICR_HLSL_Dimension_Reduction,
       ICR_Conversion,
-      ICR_Conversion,
+      ICR_HLSL_Scalar_Widening,
   };
   static_assert(std::size(Rank) == (int)ICK_Num_Conversion_Kinds);
   return Rank[(int)Kind];
 }
 
+ImplicitConversionRank
+clang::GetDimensionConversionRank(ImplicitConversionRank Base,
+                                  ImplicitConversionKind Dimension) {
+  ImplicitConversionRank Rank = GetConversionRank(Dimension);
+  if (Rank == ICR_HLSL_Scalar_Widening) {
+    if (Base == ICR_Promotion)
+      return ICR_HLSL_Scalar_Widening_Promotion;
+    if (Base == ICR_Conversion)
+      return ICR_HLSL_Scalar_Widening_Conversion;
+  }
+  if (Rank == ICR_HLSL_Dimension_Reduction) {
+    if(Base == ICR_Promotion)
+      return ICR_HLSL_Dimension_Reduction_Promotion;
+    if (Base == ICR_Conversion)
+      return ICR_HLSL_Dimension_Reduction_Conversion;
+  }
+  return Rank;
+}
+
 /// GetImplicitConversionName - Return the name of this kind of
 /// implicit conversion.
 static const char *GetImplicitConversionName(ImplicitConversionKind Kind) {
@@ -207,6 +227,7 @@ static const char *GetImplicitConversionName(ImplicitConversionKind Kind) {
       "Fixed point conversion",
       "HLSL vector truncation",
       "Non-decaying array conversion",
+      "HLSL vector splat",
   };
   static_assert(std::size(Name) == (int)ICK_Num_Conversion_Kinds);
   return Name[Kind];
@@ -217,7 +238,7 @@ static const char *GetImplicitConversionName(ImplicitConversionKind Kind) {
 void StandardConversionSequence::setAsIdentityConversion() {
   First = ICK_Identity;
   Second = ICK_Identity;
-  Element = ICK_Identity;
+  Dimension = ICK_Identity;
   Third = ICK_Identity;
   DeprecatedStringLiteralToCharPtr = false;
   QualificationIncludesObjCLifetime = false;
@@ -240,8 +261,8 @@ ImplicitConversionRank StandardConversionSequence::getRank() const {
     Rank = GetConversionRank(First);
   if (GetConversionRank(Second) > Rank)
     Rank = GetConversionRank(Second);
-  if (GetConversionRank(Element) > Rank)
-    Rank = GetConversionRank(Element);
+  if (GetDimensionConversionRank(Rank, Dimension) > Rank)
+    Rank = GetDimensionConversionRank(Rank, Dimension);
   if (GetConversionRank(Third) > Rank)
     Rank = GetConversionRank(Third);
   return Rank;
@@ -1989,15 +2010,15 @@ static bool IsVectorConversion(Sema &S, QualType FromType, QualType ToType,
         if (FromElts < ToElts)
           return false;
         if (FromElts == ToElts)
-          ICK = ICK_Identity;
+          ElConv = ICK_Identity;
         else
-          ICK = ICK_HLSL_Vector_Truncation;
+          ElConv = ICK_HLSL_Vector_Truncation;
 
         QualType FromElTy = FromExtType->getElementType();
         QualType ToElTy = ToExtType->getElementType();
         if (S.Context.hasSameUnqualifiedType(FromElTy, ToElTy))
           return true;
-        return IsVectorElementConversion(S, FromElTy, ToElTy, ElConv, From);
+        return IsVectorElementConversion(S, FromElTy, ToElTy, ICK, From);
       }
       // There are no conversions between extended vector types other than the
       // identity conversion.
@@ -2006,6 +2027,11 @@ static bool IsVectorConversion(Sema &S, QualType FromType, QualType ToType,
 
     // Vector splat from any arithmetic type to a vector.
     if (FromType->isArithmeticType()) {
+      if (S.getLangOpts().HLSL) {
+        ElConv = ICK_HLSL_Vector_Splat;
+        QualType ToElTy = ToExtType->getElementType();
+        return IsVectorElementConversion(S, FromType, ToElTy, ICK, From);
+      }
       ICK = ICK_Vector_Splat;
       return true;
     }
@@ -2220,7 +2246,7 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
   // conversion.
   bool IncompatibleObjC = false;
   ImplicitConversionKind SecondICK = ICK_Identity;
-  ImplicitConversionKind ElementICK = ICK_Identity;
+  ImplicitConversionKind DimensionICK = ICK_Identity;
   if (S.Context.hasSameUnqualifiedType(FromType, ToType)) {
     // The unqualified versions of the types are the same: there's no
     // conversion to do.
@@ -2286,10 +2312,10 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
                                          InOverloadResolution, FromType)) {
     // Pointer to member conversions (4.11).
     SCS.Second = ICK_Pointer_Member;
-  } else if (IsVectorConversion(S, FromType, ToType, SecondICK, ElementICK,
+  } else if (IsVectorConversion(S, FromType, ToType, SecondICK, DimensionICK,
                                 From, InOverloadResolution, CStyle)) {
     SCS.Second = SecondICK;
-    SCS.Element = ElementICK;
+    SCS.Dimension = DimensionICK;
     FromType = ToType.getUnqualifiedType();
   } else if (!S.getLangOpts().CPlusPlus &&
              S.Context.typesAreCompatible(ToType, FromType)) {
@@ -4341,24 +4367,6 @@ 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 n...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Jun 19, 2024

@llvm/pr-subscribers-hlsl

Author: Chris B (llvm-beanz)

Changes

This PR reworks HLSL's implicit conversion sequences. Initially I was seeking to match DXC's behavior more closely, but that was leading to a pile of special case rules to tie-break ambiguous cases that should really be left as ambiguous. We've decided that we're going to break compatibility with DXC here, and we may port this new behavior over to DXC instead.

This change is a bit closer to C++'s overload resolution rules, but it does have a bit of nuance around how dimension adjustment conversions are ranked. Conversion sequence ranks for HLSL are:

  • Exact match
  • Scalar Widening (i.e. splat)
  • Promotion
  • Scalar Widening with Promotion
  • Conversion
  • Scalar Widening with Conversion
  • Dimension Reduction (i.e. truncation)
  • Dimension Reduction with Promotion
  • Dimension Reduction with Conversion

In this implementation I've folded the disambiguation into the conversion sequence ranks which does add some complexity as compared to C++, however this avoids needing to add special casing in CompareStandardConversionSequences. I believe the added conversion rank values provide a simpler approach, but feedback is appreciated.

The HLSL language spec updates are in the PR here: microsoft/hlsl-specs#261


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

15 Files Affected:

  • (modified) clang/docs/HLSL/ExpectedDifferences.rst (+11)
  • (modified) clang/include/clang/Sema/Overload.h (+31-7)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+79-84)
  • (modified) clang/lib/Sema/SemaOverload.cpp (+39-46)
  • (modified) clang/test/CodeGenHLSL/BasicFeatures/standard_conversion_sequences.hlsl (+9-9)
  • (modified) clang/test/CodeGenHLSL/builtins/dot.hlsl (+1-33)
  • (modified) clang/test/CodeGenHLSL/builtins/lerp.hlsl (-24)
  • (modified) clang/test/CodeGenHLSL/builtins/mad.hlsl (-22)
  • (modified) clang/test/SemaHLSL/ScalarOverloadResolution.hlsl (+13-15)
  • (added) clang/test/SemaHLSL/SplatOverloadResolution.hlsl (+166)
  • (added) clang/test/SemaHLSL/TruncationOverloadResolution.hlsl (+68)
  • (modified) clang/test/SemaHLSL/Types/BuiltinVector/ScalarSwizzles.hlsl (+1-1)
  • (modified) clang/test/SemaHLSL/VectorElementOverloadResolution.hlsl (+16-20)
  • (modified) clang/test/SemaHLSL/VectorOverloadResolution.hlsl (+5-5)
  • (modified) clang/test/SemaHLSL/standard_conversion_sequences.hlsl (+8-8)
diff --git a/clang/docs/HLSL/ExpectedDifferences.rst b/clang/docs/HLSL/ExpectedDifferences.rst
index d1b6010f10f43..a29b6348e0b8e 100644
--- a/clang/docs/HLSL/ExpectedDifferences.rst
+++ b/clang/docs/HLSL/ExpectedDifferences.rst
@@ -67,12 +67,16 @@ behavior between Clang and DXC. Some examples include:
   void takesDoubles(double, double, double);
 
   cbuffer CB {
+    bool B;
     uint U;
     int I;
     float X, Y, Z;
     double3 A, B;
   }
 
+  void twoParams(int, int);
+  void twoParams(float, float);
+
   export void call() {
     halfOrInt16(U); // DXC: Fails with call ambiguous between int16_t and uint16_t overloads
                     // Clang: Resolves to halfOrInt16(uint16_t).
@@ -98,6 +102,13 @@ behavior between Clang and DXC. Some examples include:
                           // FXC: Expands to compute double dot product with fmul/fadd
                           // Clang: Resolves to dot(float3, float3), emits conversion warnings.
 
+  #ifndef IGNORE_ERRORS
+    tan(B); // DXC: resolves to tan(float).
+            // Clang: Fails to resolve, ambiguous between integer types.
+
+    twoParams(I, X); // DXC: resolves twoParams(int, int).
+                     // Clang: Fails to resolve ambiguous conversions.
+  #endif
   }
 
 .. note::
diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h
index 4a5c9e8ca1229..9d8b797af6663 100644
--- a/clang/include/clang/Sema/Overload.h
+++ b/clang/include/clang/Sema/Overload.h
@@ -201,6 +201,9 @@ class Sema;
     /// HLSL non-decaying array rvalue cast.
     ICK_HLSL_Array_RValue,
 
+    // HLSL vector splat from scalar or boolean type.
+    ICK_HLSL_Vector_Splat,
+
     /// The number of conversion kinds
     ICK_Num_Conversion_Kinds,
   };
@@ -213,15 +216,27 @@ class Sema;
     /// Exact Match
     ICR_Exact_Match = 0,
 
+    /// HLSL Scalar Widening
+    ICR_HLSL_Scalar_Widening,
+
     /// Promotion
     ICR_Promotion,
 
+    /// HLSL Scalar Widening with promotion
+    ICR_HLSL_Scalar_Widening_Promotion,
+
+    /// HLSL Matching Dimension Reduction
+    ICR_HLSL_Dimension_Reduction,
+
     /// Conversion
     ICR_Conversion,
 
     /// OpenCL Scalar Widening
     ICR_OCL_Scalar_Widening,
 
+    /// HLSL Scalar Widening with conversion
+    ICR_HLSL_Scalar_Widening_Conversion,
+
     /// Complex <-> Real conversion
     ICR_Complex_Real_Conversion,
 
@@ -233,11 +248,21 @@ class Sema;
 
     /// Conversion not allowed by the C standard, but that we accept as an
     /// extension anyway.
-    ICR_C_Conversion_Extension
+    ICR_C_Conversion_Extension,
+
+    /// HLSL Dimension reduction with promotion
+    ICR_HLSL_Dimension_Reduction_Promotion,
+
+    /// HLSL Dimension reduction with conversion
+    ICR_HLSL_Dimension_Reduction_Conversion,
   };
 
   ImplicitConversionRank GetConversionRank(ImplicitConversionKind Kind);
 
+  ImplicitConversionRank
+  GetDimensionConversionRank(ImplicitConversionRank Base,
+                             ImplicitConversionKind Dimension);
+
   /// NarrowingKind - The kind of narrowing conversion being performed by a
   /// standard conversion sequence according to C++11 [dcl.init.list]p7.
   enum NarrowingKind {
@@ -277,11 +302,10 @@ class Sema;
     /// pointer-to-member conversion, or boolean conversion.
     ImplicitConversionKind Second : 8;
 
-    /// Element - Between the second and third conversion a vector or matrix
-    /// element conversion may occur. If this is not ICK_Identity this
-    /// conversion is applied element-wise to each element in the vector or
-    /// matrix.
-    ImplicitConversionKind Element : 8;
+    /// Dimension - Between the second and third conversion a vector or matrix
+    /// dimension conversion may occur. If this is not ICK_Identity this
+    /// conversion truncates the vector or matrix, or extends a scalar.
+    ImplicitConversionKind Dimension : 8;
 
     /// Third - The third conversion can be a qualification conversion
     /// or a function conversion.
@@ -379,7 +403,7 @@ class Sema;
     void setAsIdentityConversion();
 
     bool isIdentityConversion() const {
-      return Second == ICK_Identity && Element == ICK_Identity &&
+      return Second == ICK_Identity && Dimension == ICK_Identity &&
              Third == ICK_Identity;
     }
 
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index f3af8dee6b090..7b955cb7a8e24 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4361,6 +4361,19 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
   return From;
 }
 
+// GetIntermediateVectorType - Compute the intermediate cast type casting
+// elements of the from type to the elements of the to type without resizing the
+// vector.
+static QualType adjustVectorType(ASTContext &Context, QualType FromTy,
+                                 QualType ToType) {
+  auto *ToVec = ToType->castAs<VectorType>();
+  QualType ElType = ToVec->getElementType();
+  if (!FromTy->isVectorType())
+    return ElType;
+  auto *FromVec = FromTy->castAs<VectorType>();
+  return Context.getExtVectorType(ElType, FromVec->getNumElements());
+}
+
 /// PerformImplicitConversion - Perform an implicit conversion of the
 /// expression From to the type ToType by following the standard
 /// conversion sequence SCS. Returns the converted
@@ -4515,27 +4528,38 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
     break;
 
   case ICK_Integral_Promotion:
-  case ICK_Integral_Conversion:
-    if (ToType->isBooleanType()) {
+  case ICK_Integral_Conversion: {
+    QualType ElTy = ToType;
+    QualType StepTy = ToType;
+    if (ToType->isVectorType()) {
+      StepTy = adjustVectorType(Context, FromType, ToType);
+      ElTy = StepTy->castAs<VectorType>()->getElementType();
+    }
+    if (ElTy->isBooleanType()) {
       assert(FromType->castAs<EnumType>()->getDecl()->isFixed() &&
              SCS.Second == ICK_Integral_Promotion &&
              "only enums with fixed underlying type can promote to bool");
-      From = ImpCastExprToType(From, ToType, CK_IntegralToBoolean, VK_PRValue,
+      From = ImpCastExprToType(From, StepTy, CK_IntegralToBoolean, VK_PRValue,
                                /*BasePath=*/nullptr, CCK)
                  .get();
     } else {
-      From = ImpCastExprToType(From, ToType, CK_IntegralCast, VK_PRValue,
+      From = ImpCastExprToType(From, StepTy, CK_IntegralCast, VK_PRValue,
                                /*BasePath=*/nullptr, CCK)
                  .get();
     }
     break;
+  }
 
   case ICK_Floating_Promotion:
-  case ICK_Floating_Conversion:
-    From = ImpCastExprToType(From, ToType, CK_FloatingCast, VK_PRValue,
+  case ICK_Floating_Conversion: {
+    QualType StepTy = ToType;
+    if (ToType->isVectorType())
+      StepTy = adjustVectorType(Context, FromType, ToType);
+    From = ImpCastExprToType(From, StepTy, CK_FloatingCast, VK_PRValue,
                              /*BasePath=*/nullptr, CCK)
                .get();
     break;
+  }
 
   case ICK_Complex_Promotion:
   case ICK_Complex_Conversion: {
@@ -4558,16 +4582,23 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
     break;
   }
 
-  case ICK_Floating_Integral:
-    if (ToType->isRealFloatingType())
-      From = ImpCastExprToType(From, ToType, CK_IntegralToFloating, VK_PRValue,
+  case ICK_Floating_Integral: {
+    QualType ElTy = ToType;
+    QualType StepTy = ToType;
+    if (FromType->isVectorType()) {
+      StepTy = adjustVectorType(Context, FromType, ToType);
+      ElTy = StepTy->castAs<VectorType>()->getElementType();
+    }
+    if (ElTy->isRealFloatingType())
+      From = ImpCastExprToType(From, StepTy, CK_IntegralToFloating, VK_PRValue,
                                /*BasePath=*/nullptr, CCK)
                  .get();
     else
-      From = ImpCastExprToType(From, ToType, CK_FloatingToIntegral, VK_PRValue,
+      From = ImpCastExprToType(From, StepTy, CK_FloatingToIntegral, VK_PRValue,
                                /*BasePath=*/nullptr, CCK)
                  .get();
     break;
+  }
 
   case ICK_Fixed_Point_Conversion:
     assert((FromType->isFixedPointType() || ToType->isFixedPointType()) &&
@@ -4689,18 +4720,26 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
     break;
   }
 
-  case ICK_Boolean_Conversion:
+  case ICK_Boolean_Conversion: {
     // Perform half-to-boolean conversion via float.
     if (From->getType()->isHalfType()) {
       From = ImpCastExprToType(From, Context.FloatTy, CK_FloatingCast).get();
       FromType = Context.FloatTy;
     }
+    QualType ElTy = FromType;
+    QualType StepTy = ToType;
+    if (FromType->isVectorType()) {
+      if (getLangOpts().HLSL)
+        StepTy = adjustVectorType(Context, FromType, ToType);
+      ElTy = FromType->castAs<VectorType>()->getElementType();
+    }
 
-    From = ImpCastExprToType(From, Context.BoolTy,
-                             ScalarTypeToBooleanCastKind(FromType), VK_PRValue,
+    From = ImpCastExprToType(From, StepTy,
+                             ScalarTypeToBooleanCastKind(ElTy), VK_PRValue,
                              /*BasePath=*/nullptr, CCK)
                .get();
     break;
+  }
 
   case ICK_Derived_To_Base: {
     CXXCastPath BasePath;
@@ -4826,22 +4865,6 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
                              CK_ZeroToOCLOpaqueType,
                              From->getValueKind()).get();
     break;
-  case ICK_HLSL_Vector_Truncation: {
-    // Note: HLSL built-in vectors are ExtVectors. Since this truncates a vector
-    // to a smaller vector, this can only operate on arguments where the source
-    // and destination types are ExtVectors.
-    assert(From->getType()->isExtVectorType() && ToType->isExtVectorType() &&
-           "HLSL vector truncation should only apply to ExtVectors");
-    auto *FromVec = From->getType()->castAs<VectorType>();
-    auto *ToVec = ToType->castAs<VectorType>();
-    QualType ElType = FromVec->getElementType();
-    QualType TruncTy =
-        Context.getExtVectorType(ElType, ToVec->getNumElements());
-    From = ImpCastExprToType(From, TruncTy, CK_HLSLVectorTruncation,
-                             From->getValueKind())
-               .get();
-    break;
-  }
 
   case ICK_Lvalue_To_Rvalue:
   case ICK_Array_To_Pointer:
@@ -4852,73 +4875,45 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
   case ICK_C_Only_Conversion:
   case ICK_Incompatible_Pointer_Conversion:
   case ICK_HLSL_Array_RValue:
+  case ICK_HLSL_Vector_Truncation:
+  case ICK_HLSL_Vector_Splat:
     llvm_unreachable("Improper second standard conversion");
   }
 
-  if (SCS.Element != ICK_Identity) {
+  if (SCS.Dimension != ICK_Identity) {
     // If SCS.Element is not ICK_Identity the To and From types must be HLSL
     // vectors or matrices.
 
     // TODO: Support HLSL matrices.
     assert((!From->getType()->isMatrixType() && !ToType->isMatrixType()) &&
-           "Element conversion for matrix types is not implemented yet.");
-    assert(From->getType()->isVectorType() && ToType->isVectorType() &&
-           "Element conversion is only supported for vector types.");
-    assert(From->getType()->getAs<VectorType>()->getNumElements() ==
-               ToType->getAs<VectorType>()->getNumElements() &&
-           "Element conversion is only supported for vectors with the same "
-           "element counts.");
-    QualType FromElTy = From->getType()->getAs<VectorType>()->getElementType();
-    unsigned NumElts = ToType->getAs<VectorType>()->getNumElements();
-    switch (SCS.Element) {
-    case ICK_Boolean_Conversion:
-      // Perform half-to-boolean conversion via float.
-      if (FromElTy->isHalfType()) {
-        QualType FPExtType = Context.getExtVectorType(FromElTy, NumElts);
-        From = ImpCastExprToType(From, FPExtType, CK_FloatingCast).get();
-        FromType = FPExtType;
-      }
-
-      From =
-          ImpCastExprToType(From, ToType, ScalarTypeToBooleanCastKind(FromElTy),
-                            VK_PRValue,
-                            /*BasePath=*/nullptr, CCK)
-              .get();
-      break;
-    case ICK_Integral_Promotion:
-    case ICK_Integral_Conversion:
-      if (ToType->isBooleanType()) {
-        assert(FromType->castAs<EnumType>()->getDecl()->isFixed() &&
-               SCS.Second == ICK_Integral_Promotion &&
-               "only enums with fixed underlying type can promote to bool");
-        From = ImpCastExprToType(From, ToType, CK_IntegralToBoolean, VK_PRValue,
-                                 /*BasePath=*/nullptr, CCK)
-                   .get();
-      } else {
-        From = ImpCastExprToType(From, ToType, CK_IntegralCast, VK_PRValue,
-                                 /*BasePath=*/nullptr, CCK)
-                   .get();
-      }
-      break;
-
-    case ICK_Floating_Promotion:
-    case ICK_Floating_Conversion:
-      From = ImpCastExprToType(From, ToType, CK_FloatingCast, VK_PRValue,
+           "Dimension conversion for matrix types is not implemented yet.");
+    assert(ToType->isVectorType() &&
+           "Dimension conversion is only supported for vector types.");
+    switch (SCS.Dimension) {
+    case ICK_HLSL_Vector_Splat: {
+      // Vector splat from any arithmetic type to a vector.
+      Expr *Elem = prepareVectorSplat(ToType, From).get();
+      From = ImpCastExprToType(Elem, ToType, CK_VectorSplat, VK_PRValue,
                                /*BasePath=*/nullptr, CCK)
                  .get();
       break;
-    case ICK_Floating_Integral:
-      if (ToType->hasFloatingRepresentation())
-        From =
-            ImpCastExprToType(From, ToType, CK_IntegralToFloating, VK_PRValue,
-                              /*BasePath=*/nullptr, CCK)
-                .get();
-      else
-        From =
-            ImpCastExprToType(From, ToType, CK_FloatingToIntegral, VK_PRValue,
-                              /*BasePath=*/nullptr, CCK)
-                .get();
+    }
+    case ICK_HLSL_Vector_Truncation: {
+      // Note: HLSL built-in vectors are ExtVectors. Since this truncates a
+      // vector to a smaller vector, this can only operate on arguments where
+      // the source and destination types are ExtVectors.
+      assert(From->getType()->isExtVectorType() && ToType->isExtVectorType() &&
+             "HLSL vector truncation should only apply to ExtVectors");
+      auto *FromVec = From->getType()->castAs<VectorType>();
+      auto *ToVec = ToType->castAs<VectorType>();
+      QualType ElType = FromVec->getElementType();
+      QualType TruncTy =
+          Context.getExtVectorType(ElType, ToVec->getNumElements());
+      From = ImpCastExprToType(From, TruncTy, CK_HLSLVectorTruncation,
+                               From->getValueKind())
+                 .get();
       break;
+    }
     case ICK_Identity:
     default:
       llvm_unreachable("Improper element standard conversion");
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index fb4ff72e42eb5..e17b62f32626a 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -163,13 +163,33 @@ ImplicitConversionRank clang::GetConversionRank(ImplicitConversionKind Kind) {
       ICR_C_Conversion,
       ICR_C_Conversion_Extension,
       ICR_Conversion,
+      ICR_HLSL_Dimension_Reduction,
       ICR_Conversion,
-      ICR_Conversion,
+      ICR_HLSL_Scalar_Widening,
   };
   static_assert(std::size(Rank) == (int)ICK_Num_Conversion_Kinds);
   return Rank[(int)Kind];
 }
 
+ImplicitConversionRank
+clang::GetDimensionConversionRank(ImplicitConversionRank Base,
+                                  ImplicitConversionKind Dimension) {
+  ImplicitConversionRank Rank = GetConversionRank(Dimension);
+  if (Rank == ICR_HLSL_Scalar_Widening) {
+    if (Base == ICR_Promotion)
+      return ICR_HLSL_Scalar_Widening_Promotion;
+    if (Base == ICR_Conversion)
+      return ICR_HLSL_Scalar_Widening_Conversion;
+  }
+  if (Rank == ICR_HLSL_Dimension_Reduction) {
+    if(Base == ICR_Promotion)
+      return ICR_HLSL_Dimension_Reduction_Promotion;
+    if (Base == ICR_Conversion)
+      return ICR_HLSL_Dimension_Reduction_Conversion;
+  }
+  return Rank;
+}
+
 /// GetImplicitConversionName - Return the name of this kind of
 /// implicit conversion.
 static const char *GetImplicitConversionName(ImplicitConversionKind Kind) {
@@ -207,6 +227,7 @@ static const char *GetImplicitConversionName(ImplicitConversionKind Kind) {
       "Fixed point conversion",
       "HLSL vector truncation",
       "Non-decaying array conversion",
+      "HLSL vector splat",
   };
   static_assert(std::size(Name) == (int)ICK_Num_Conversion_Kinds);
   return Name[Kind];
@@ -217,7 +238,7 @@ static const char *GetImplicitConversionName(ImplicitConversionKind Kind) {
 void StandardConversionSequence::setAsIdentityConversion() {
   First = ICK_Identity;
   Second = ICK_Identity;
-  Element = ICK_Identity;
+  Dimension = ICK_Identity;
   Third = ICK_Identity;
   DeprecatedStringLiteralToCharPtr = false;
   QualificationIncludesObjCLifetime = false;
@@ -240,8 +261,8 @@ ImplicitConversionRank StandardConversionSequence::getRank() const {
     Rank = GetConversionRank(First);
   if (GetConversionRank(Second) > Rank)
     Rank = GetConversionRank(Second);
-  if (GetConversionRank(Element) > Rank)
-    Rank = GetConversionRank(Element);
+  if (GetDimensionConversionRank(Rank, Dimension) > Rank)
+    Rank = GetDimensionConversionRank(Rank, Dimension);
   if (GetConversionRank(Third) > Rank)
     Rank = GetConversionRank(Third);
   return Rank;
@@ -1989,15 +2010,15 @@ static bool IsVectorConversion(Sema &S, QualType FromType, QualType ToType,
         if (FromElts < ToElts)
           return false;
         if (FromElts == ToElts)
-          ICK = ICK_Identity;
+          ElConv = ICK_Identity;
         else
-          ICK = ICK_HLSL_Vector_Truncation;
+          ElConv = ICK_HLSL_Vector_Truncation;
 
         QualType FromElTy = FromExtType->getElementType();
         QualType ToElTy = ToExtType->getElementType();
         if (S.Context.hasSameUnqualifiedType(FromElTy, ToElTy))
           return true;
-        return IsVectorElementConversion(S, FromElTy, ToElTy, ElConv, From);
+        return IsVectorElementConversion(S, FromElTy, ToElTy, ICK, From);
       }
       // There are no conversions between extended vector types other than the
       // identity conversion.
@@ -2006,6 +2027,11 @@ static bool IsVectorConversion(Sema &S, QualType FromType, QualType ToType,
 
     // Vector splat from any arithmetic type to a vector.
     if (FromType->isArithmeticType()) {
+      if (S.getLangOpts().HLSL) {
+        ElConv = ICK_HLSL_Vector_Splat;
+        QualType ToElTy = ToExtType->getElementType();
+        return IsVectorElementConversion(S, FromType, ToElTy, ICK, From);
+      }
       ICK = ICK_Vector_Splat;
       return true;
     }
@@ -2220,7 +2246,7 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
   // conversion.
   bool IncompatibleObjC = false;
   ImplicitConversionKind SecondICK = ICK_Identity;
-  ImplicitConversionKind ElementICK = ICK_Identity;
+  ImplicitConversionKind DimensionICK = ICK_Identity;
   if (S.Context.hasSameUnqualifiedType(FromType, ToType)) {
     // The unqualified versions of the types are the same: there's no
     // conversion to do.
@@ -2286,10 +2312,10 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
                                          InOverloadResolution, FromType)) {
     // Pointer to member conversions (4.11).
     SCS.Second = ICK_Pointer_Member;
-  } else if (IsVectorConversion(S, FromType, ToType, SecondICK, ElementICK,
+  } else if (IsVectorConversion(S, FromType, ToType, SecondICK, DimensionICK,
                                 From, InOverloadResolution, CStyle)) {
     SCS.Second = SecondICK;
-    SCS.Element = ElementICK;
+    SCS.Dimension = DimensionICK;
     FromType = ToType.getUnqualifiedType();
   } else if (!S.getLangOpts().CPlusPlus &&
              S.Context.typesAreCompatible(ToType, FromType)) {
@@ -4341,24 +4367,6 @@ 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 n...
[truncated]

Copy link
Contributor

@bogner bogner left a comment

Choose a reason for hiding this comment

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

It's a bit unfortunate that we need so many HLSL specific ImplicitConversionKinds, but I do think that the logic is easier to follow this way so it's probably the right thing to do.

// GetIntermediateVectorType - Compute the intermediate cast type casting
// elements of the from type to the elements of the to type without resizing the
// vector.
static QualType adjustVectorType(ASTContext &Context, QualType FromTy,
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you renamed this function after writing the doc comment.

In any case, we generally don't repeat the function name in the doc comment any more, and these should be /// comments for them to show up on the doxygen page

This PR reworks HLSL's implicit conversion sequences. Initially I was
seeking to match DXC's behavior more closely, but that was leading to a
pile of special case rules to tie-break ambiguous cases that should
really be left as ambiguous.

This change is a bit closer to C++'s overload resolution rules, but it
does have a bit of nuance around how dimension adjustment conversions
are ranked. Conversion sequence ranks for HLSL are:

* Exact match
* Scalar Widening (i.e. splat)
* Promotion
* Scalar Widening with Promotion
* Conversion
* Scalar Widening with Conversion
* Dimension Reduction (i.e. truncation)
* Dimension Reduction with Promotion
* Dimension Reduction with Conversion

In this implementation I've folded the disambiguation into the
conversion sequence ranks which does add some complexity as compared to
C++, however this avoids needing to add special casing in
`CompareStandardConversionSequences`. I belive the added conversion
rank values provide a simpler approach, but welcome feedback.

../clang/test/CodeGenHLSL/BasicFeatures/standard_conversion_sequences.hl
sl
../clang/test/SemaHLSL/TruncationOverloadResolution.hlsl
../clang/test/SemaHLSL/Types/BuiltinVector/ScalarSwizzles.hlsl
../clang/test/SemaHLSL/VectorElementOverloadResolution.hlsl
../clang/test/SemaHLSL/standard_conversion_sequences.hlsl
@llvm-beanz llvm-beanz force-pushed the cbieneman/rework-overload-resolution branch from 84874c8 to 84c6a2c Compare July 12, 2024 22:25
Copy link

github-actions bot commented Jul 12, 2024

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

@llvm-beanz llvm-beanz merged commit d91ff3f into llvm:main Jul 13, 2024
8 checks passed
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
This PR reworks HLSL's implicit conversion sequences. Initially I was
seeking to match DXC's behavior more closely, but that was leading to a
pile of special case rules to tie-break ambiguous cases that should
really be left as ambiguous. We've decided that we're going to break
compatibility with DXC here, and we may port this new behavior over to
DXC instead.

This change is a bit closer to C++'s overload resolution rules, but it
does have a bit of nuance around how dimension adjustment conversions
are ranked. Conversion sequence ranks for HLSL are:

* Exact match
* Scalar Widening (i.e. splat)
* Promotion
* Scalar Widening with Promotion
* Conversion
* Scalar Widening with Conversion
* Dimension Reduction (i.e. truncation)
* Dimension Reduction with Promotion
* Dimension Reduction with Conversion

In this implementation I've folded the disambiguation into the
conversion sequence ranks which does add some complexity as compared to
C++, however this avoids needing to add special casing in
`CompareStandardConversionSequences`. I believe the added conversion
rank values provide a simpler approach, but feedback is appreciated.

The HLSL language spec updates are in the PR here:
microsoft/hlsl-specs#261
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
Archived in project
Development

Successfully merging this pull request may close these issues.

[HLSL] Integer conversion overload scoring [HLSL] Scalar to vector implicit conversion
4 participants