[HLSL] Vector standard conversions #71098
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvm-beanz
requested review from
fhahn,
cor3ntin,
SaurabhJha,
AaronBallman and
AnastasiaStulova
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-hlsl Author: Chris B (llvm-beanz) ChangesHLSL supports vector truncation and element conversions as part of standard conversion sequences. The vector truncation conversion is a C++ second conversion in the conversion sequence. If a vector truncation is in a conversion sequence an element conversion may occur after it before the standard C++ third conversion. Vector element conversions can be boolean conversions, floating point or integral conversions or promotions. Patch is 41.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71098.diff 19 Files Affected:
diff --git a/clang/include/clang/AST/OperationKinds.def b/clang/include/clang/AST/OperationKinds.def
index 8dd98730dff7426..e497fe4d1f93ff4 100644
--- a/clang/include/clang/AST/OperationKinds.def
+++ b/clang/include/clang/AST/OperationKinds.def
@@ -361,6 +361,9 @@ CAST_OPERATION(AddressSpaceConversion)
// Convert an integer initializer to an OpenCL sampler.
CAST_OPERATION(IntToOCLSampler)
+// Truncate a vector type (HLSL only).
+CAST_OPERATION(VectorTruncation)
+
//===- Binary Operations -------------------------------------------------===//
// Operators listed in order of precedence.
// Note that additions to this should also update the StmtVisitor class,
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 224c0df7f1fb71f..01ba3a24e0018ba 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12005,6 +12005,9 @@ def err_hlsl_pointers_unsupported : Error<
def err_hlsl_operator_unsupported : Error<
"the '%select{&|*|->}0' operator is unsupported in HLSL">;
+def warn_hlsl_impcast_vector_truncation : Warning<
+ "implicit conversion truncates vector: %0 to %1">, InGroup<Conversion>;
+
// Layout randomization diagnostics.
def err_non_designated_init_used : Error<
"a randomized struct can only be initialized with a designated initializer">;
diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h
index a97968dc7b20967..0d4aaa5ff7c766b 100644
--- a/clang/include/clang/Sema/Overload.h
+++ b/clang/include/clang/Sema/Overload.h
@@ -192,6 +192,9 @@ class Sema;
/// C-only conversion between pointers with incompatible types
ICK_Incompatible_Pointer_Conversion,
+ /// HLSL vector truncation.
+ ICK_HLSL_Vector_Truncation,
+
/// The number of conversion kinds
ICK_Num_Conversion_Kinds,
};
@@ -271,6 +274,12 @@ 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;
+
/// Third - The third conversion can be a qualification conversion
/// or a function conversion.
ImplicitConversionKind Third : 8;
@@ -357,7 +366,8 @@ class Sema;
void setAsIdentityConversion();
bool isIdentityConversion() const {
- return Second == ICK_Identity && Third == ICK_Identity;
+ return Second == ICK_Identity && Element == ICK_Identity &&
+ Third == ICK_Identity;
}
ImplicitConversionRank getRank() const;
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 5d3b510df1ef9b3..d18514949b1eb37 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1882,6 +1882,7 @@ bool CastExpr::CastConsistency() const {
case CK_FixedPointToIntegral:
case CK_IntegralToFixedPoint:
case CK_MatrixCast:
+ case CK_VectorTruncation:
assert(!getType()->isBooleanType() && "unheralded conversion to bool");
goto CheckNoBasePath;
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index b6b1e6617dffaa9..f0e44ab0c85a3ea 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -13888,6 +13888,7 @@ bool IntExprEvaluator::VisitCastExpr(const CastExpr *E) {
case CK_FixedPointCast:
case CK_IntegralToFixedPoint:
case CK_MatrixCast:
+ case CK_VectorTruncation:
llvm_unreachable("invalid cast kind for integral value");
case CK_BitCast:
@@ -14726,6 +14727,7 @@ bool ComplexExprEvaluator::VisitCastExpr(const CastExpr *E) {
case CK_FixedPointToIntegral:
case CK_IntegralToFixedPoint:
case CK_MatrixCast:
+ case CK_VectorTruncation:
llvm_unreachable("invalid cast kind for complex value");
case CK_LValueToRValue:
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 54a1d300a9ac738..0e0f7f205eae9f6 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -4858,6 +4858,7 @@ LValue CodeGenFunction::EmitCastLValue(const CastExpr *E) {
case CK_FixedPointToIntegral:
case CK_IntegralToFixedPoint:
case CK_MatrixCast:
+ case CK_VectorTruncation:
return EmitUnsupportedLValue(E, "unexpected cast lvalue");
case CK_Dependent:
@@ -4985,6 +4986,7 @@ LValue CodeGenFunction::EmitCastLValue(const CastExpr *E) {
return MakeAddrLValue(V, E->getType(), LV.getBaseInfo(),
CGM.getTBAAInfoForSubobject(LV, E->getType()));
}
+
case CK_ZeroToOCLOpaqueType:
llvm_unreachable("NULL to OpenCL opaque type lvalue cast is not valid");
}
diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 810b28f25fa18bf..f9278133e5ddc6b 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -930,6 +930,7 @@ void AggExprEmitter::VisitCastExpr(CastExpr *E) {
case CK_BuiltinFnToFnPtr:
case CK_ZeroToOCLOpaqueType:
case CK_MatrixCast:
+ case CK_VectorTruncation:
case CK_IntToOCLSampler:
case CK_FloatingToFixedPoint:
@@ -1454,6 +1455,7 @@ static bool castPreservesZero(const CastExpr *CE) {
case CK_MatrixCast:
case CK_NonAtomicToAtomic:
case CK_AtomicToNonAtomic:
+ case CK_VectorTruncation:
return true;
case CK_BaseToDerivedMemberPointer:
diff --git a/clang/lib/CodeGen/CGExprComplex.cpp b/clang/lib/CodeGen/CGExprComplex.cpp
index f3cbd1d0451ebe4..45862701d59bb4a 100644
--- a/clang/lib/CodeGen/CGExprComplex.cpp
+++ b/clang/lib/CodeGen/CGExprComplex.cpp
@@ -556,6 +556,7 @@ ComplexPairTy ComplexExprEmitter::EmitCast(CastKind CK, Expr *Op,
case CK_FixedPointToIntegral:
case CK_IntegralToFixedPoint:
case CK_MatrixCast:
+ case CK_VectorTruncation:
llvm_unreachable("invalid cast kind for complex value");
case CK_FloatingRealToComplex:
diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp
index cd91a698a9336f8..b43eb1891481237 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -1225,6 +1225,7 @@ class ConstExprEmitter :
case CK_IntegralToFixedPoint:
case CK_ZeroToOCLOpaqueType:
case CK_MatrixCast:
+ case CK_VectorTruncation:
return nullptr;
}
llvm_unreachable("Invalid CastKind");
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 1a7a3f97bb779a0..61641c1131c39dd 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -610,6 +610,8 @@ class ScalarExprEmitter
llvm::Value *EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
bool isInc, bool isPre);
+ llvm::Value *EmitVectorElementConversion(QualType SrcType, QualType DstType,
+ llvm::Value *Src);
Value *VisitUnaryAddrOf(const UnaryOperator *E) {
if (isa<MemberPointerType>(E->getType())) // never sugared
@@ -1422,6 +1424,9 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType,
return Builder.CreateVectorSplat(NumElements, Src, "splat");
}
+ if (SrcType->isExtVectorType() && DstType->isExtVectorType())
+ return EmitVectorElementConversion(SrcType, DstType, Src);
+
if (SrcType->isMatrixType() && DstType->isMatrixType())
return EmitScalarCast(Src, SrcType, DstType, SrcTy, DstTy, Opts);
@@ -1701,10 +1706,14 @@ Value *ScalarExprEmitter::VisitShuffleVectorExpr(ShuffleVectorExpr *E) {
}
Value *ScalarExprEmitter::VisitConvertVectorExpr(ConvertVectorExpr *E) {
- QualType SrcType = E->getSrcExpr()->getType(),
- DstType = E->getType();
+ QualType SrcType = E->getSrcExpr()->getType(), DstType = E->getType();
+ Value *Src = CGF.EmitScalarExpr(E->getSrcExpr());
+ return EmitVectorElementConversion(SrcType, DstType, Src);
+}
- Value *Src = CGF.EmitScalarExpr(E->getSrcExpr());
+llvm::Value *ScalarExprEmitter::EmitVectorElementConversion(QualType SrcType,
+ QualType DstType,
+ Value *Src) {
SrcType = CGF.getContext().getCanonicalType(SrcType);
DstType = CGF.getContext().getCanonicalType(DstType);
@@ -2475,6 +2484,14 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
case CK_IntToOCLSampler:
return CGF.CGM.createOpenCLIntToSamplerConversion(E, CGF);
+
+ case CK_VectorTruncation: {
+ 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->getAs<VectorType>()->getNumElements(), 0);
+ return Builder.CreateShuffleVector(Vec, Mask, "trunc");
+ }
} // end of switch
diff --git a/clang/lib/Edit/RewriteObjCFoundationAPI.cpp b/clang/lib/Edit/RewriteObjCFoundationAPI.cpp
index 736e450574d9f80..5ff9e3ee39d2a35 100644
--- a/clang/lib/Edit/RewriteObjCFoundationAPI.cpp
+++ b/clang/lib/Edit/RewriteObjCFoundationAPI.cpp
@@ -1086,6 +1086,10 @@ static bool rewriteToNumericBoxedExpression(const ObjCMessageExpr *Msg,
case CK_BooleanToSignedIntegral:
llvm_unreachable("OpenCL-specific cast in Objective-C?");
+
+ case CK_VectorTruncation:
+ llvm_unreachable("HLSL-specific cast in Objective-C?");
+ break;
case CK_FloatingToFixedPoint:
case CK_FixedPointToFloating:
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 1842a783dc29aaa..0729d92d642aa57 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -15416,11 +15416,18 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
if (S.SourceMgr.isInSystemMacro(CC))
return;
return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_vector_scalar);
+ } else if (S.getLangOpts().HLSL &&
+ Target->getAs<VectorType>()->getNumElements() <
+ Source->getAs<VectorType>()->getNumElements()) {
+ // Diagnose vector truncation but don't return. We may also want to
+ // diagnose an element conversion.
+ DiagnoseImpCast(S, E, T, CC, diag::warn_hlsl_impcast_vector_truncation);
}
// If the vector cast is cast between two vectors of the same size, it is
- // a bitcast, not a conversion.
- if (S.Context.getTypeSize(Source) == S.Context.getTypeSize(Target))
+ // a bitcast, not a conversion, except under HLSL where it is a conversion.
+ if (!S.getLangOpts().HLSL &&
+ S.Context.getTypeSize(Source) == S.Context.getTypeSize(Target))
return;
Source = cast<VectorType>(Source)->getElementType().getTypePtr();
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index ea286c9709c13ff..83dcb615ccc03f1 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4709,6 +4709,19 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
CK_ZeroToOCLOpaqueType,
From->getValueKind()).get();
break;
+ case ICK_HLSL_Vector_Truncation: {
+ // Note: HLSL 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.
+ auto *FromVec = From->getType()->castAs<ExtVectorType>();
+ auto *ToVec = ToType->castAs<ExtVectorType>();
+ QualType ElType = FromVec->getElementType();
+ QualType TruncTy = Context.getExtVectorType(ElType, ToVec->getNumElements());
+ From = ImpCastExprToType(From, TruncTy, CK_VectorTruncation,
+ From->getValueKind())
+ .get();
+ break;
+ }
case ICK_Lvalue_To_Rvalue:
case ICK_Array_To_Pointer:
@@ -4721,6 +4734,76 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
llvm_unreachable("Improper second standard conversion");
}
+ if (SCS.Element != ICK_Identity) {
+ // if SCS.Element is not ICK_Identity the To and From types must be HLSL
+ // vectors or matrices. HLSL matrices aren't yet supported so this code only
+ // handles vectors for now.
+
+ 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_Identity:
+ // Nothing to do.
+ break;
+ 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,
+ /*BasePath=*/nullptr, CCK)
+ .get();
+ break;
+ case ICK_Floating_Integral:
+ if (ToType->isRealFloatingType())
+ 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();
+ break;
+ default:
+ llvm_unreachable("Improper element standard conversion");
+ }
+ }
+
switch (SCS.Third) {
case ICK_Identity:
// Nothing to do.
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index ed02d3580f34f9a..e7d3348d9983793 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -6427,7 +6427,7 @@ void InitializationSequence::InitializeFrom(Sema &S,
// For HLSL ext vector types we allow list initialization behavior for C++
// constructor syntax. This is accomplished by converting initialization
// arguments an InitListExpr late.
- if (S.getLangOpts().HLSL && DestType->isExtVectorType() &&
+ if (S.getLangOpts().HLSL && Args.size() > 1 && DestType->isExtVectorType() &&
(SourceType.isNull() ||
!Context.hasSameUnqualifiedType(SourceType, DestType))) {
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index d3d2dfed2ce0cc2..98ddc49bfe2c912 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -158,7 +158,8 @@ ImplicitConversionRank clang::GetConversionRank(ImplicitConversionKind Kind) {
// it was omitted by the patch that added
// ICK_Zero_Queue_Conversion
ICR_C_Conversion,
- ICR_C_Conversion_Extension
+ ICR_C_Conversion_Extension,
+ ICR_C_Conversion
};
static_assert(std::size(Rank) == (int)ICK_Num_Conversion_Kinds);
return Rank[(int)Kind];
@@ -197,7 +198,8 @@ static const char* GetImplicitConversionName(ImplicitConversionKind Kind) {
"OpenCL Zero Event Conversion",
"OpenCL Zero Queue Conversion",
"C specific type conversion",
- "Incompatible pointer conversion"
+ "Incompatible pointer conversion",
+ "HLSL vector truncation"
};
static_assert(std::size(Name) == (int)ICK_Num_Conversion_Kinds);
return Name[Kind];
@@ -208,6 +210,7 @@ static const char* GetImplicitConversionName(ImplicitConversionKind Kind) {
void StandardConversionSequence::setAsIdentityConversion() {
First = ICK_Identity;
Second = ICK_Identity;
+ Element = ICK_Identity;
Third = ICK_Identity;
DeprecatedStringLiteralToCharPtr = false;
QualificationIncludesObjCLifetime = false;
@@ -1841,13 +1844,86 @@ bool Sema::IsFunctionConversion(QualType FromType, QualType ToType,
return true;
}
+/// Determine whether the conversion from FromType to ToType is a valid
+/// floating point conversion.
+///
+static bool IsFloatingPointConversion(Sema &S, QualType FromType,
+ QualType ToType) {
+ if (!FromType->isRealFloatingType() || !ToType->isRealFloatingType())
+ return false;
+ // FIXME: disable conversions between long double, __ibm128 and __float128
+ // if their representation is different until there is back end support
+ // We of course allow this conversion if long double is really double.
+
+ // Conversions between bfloat16 and float16 are currently not supported.
+ if ((FromType->isBFloat16Type() &&
+ (ToType->isFloat16Type() || ToType->isHalfType())) ||
+ (ToType->isBFloat16Type() &&
+ (FromType->isFloat16Type() || FromType->isHalfType())))
+ return false;
+
+ // Conversions between IEEE-quad and IBM-extended semantics are not
+ // permitted.
+ const llvm::fltSemantics &FromSem = S.Context.getFloatTypeSemantics(FromType);
+ const llvm::fltSemantics &ToSem = S.Context.getFloatTypeSemantics(ToType);
+ if ((&FromSem == &llvm::APFloat::PPCDoubleDouble() &&
+ &ToSem == &llvm::APFloat::IEEEquad()) ||
+ (&FromSem == &llvm::APFloat::IEEEquad() &&
+ &ToSem == &llvm::APFloat::PPCDoubleDouble()))
+ return false;
+ return true;
+}
+
+static bool IsVectorElementConversion(Sema &S, QualType FromType,
+ QualType ToType,
+ ImplicitConversionKind &ICK, Expr *From) {
+ if (S.Context.hasSameUnqualifiedType(FromType, ToType))
+ return true;
+
+ if (IsFloatingPointConversion(S, FromType, ToType)) {
+ ICK = ICK_Floating_Conversion;
+ return true;
+ }
+
+ if (S.IsFloatingPointPromotion(FromType, ToType)) {
+ ICK = ICK_Floating_Promotion;
+ return true;
+ }
+
+ if (ToType->isBooleanType() && FromType->isArithmeticType()) {
+ ICK = ICK_Boolean_Conversion;
+ return true;
+ }
+
+ if (FromType->isIntegralOrUnscopedEnumerationType() &&
+ ToType->isIntegralType(S.Context)) {
+ ICK = ICK_Integral_Conversion;
+ return true;
+ }
+
+ if (S.IsIntegralPromotion(From, FromType, ToType)) {
+ ICK = ICK_Integral_Promotion;
+ return true;
+ }
+
+ if ((FromType->isRealFloatingType() && ToType->isIntegralType(S.Context)) ||
+ (FromType->isIntegralOrUnscopedEnumerationType() &&
+ ToType->isRealFloatingType())) {
+ ICK = ICK_Floating_Integral;
+ return true;
+ }
+
+ return false;
+}
+
/// Determine whether the conversion from FromType to ToType is a valid
/// vector conversion.
///
/// \param ICK Will be set to the vector conversion kind, if this is a vector
/// conversion.
static bool IsVectorConversion(Sema &S, QualType FromType, QualType ToType,
- ImplicitConversionKind &ICK, Expr *From,
+ ImplicitConversionKind &ICK,
+ ImplicitConversionKind &ElConv, Expr *From,
bool InOverloadResolution, bool CStyle) {
// We need at...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
llvm-beanz
force-pushed
the
cbieneman/hlsl-standard-conversions-trunc
branch
from
73c0b9f
to
6bf40be
Compare
llvm-beanz
changed the title
|
@AaronBallman, gentle ping 😄 |
|
Adding some additional reviewers to look over codegen changes and also for another set of eyes on the conversion changes in Sema (the changes look correct to me, but conversion logic is dense enough that getting more eyes on it is a good idea). |
HLSL supports vector truncation and element conversions as part of standard conversion sequences. The vector truncation conversion is a C++ second conversion in the conversion sequence. If a vector truncation is in a conversion sequence an element conversion may occur after it before the standard C++ third conversion. Vector element conversions can be boolean conversions, floating point or integral conversions or promotions. [HLSL Draft Specification](https://microsoft.github.io/hlsl-specs/specs/hlsl.pdf) ../clang/test/CodeGenHLSL/BasicFeatures/standard_conversion_sequences.hl sl ../clang/test/SemaHLSL/BuiltIns/vector-constructors-erros.hlsl ../clang/test/SemaHLSL/standard_conversion_sequences.hlsl
../clang/test/SemaHLSL/standard_conversion_sequences.hlsl ../clang/test/CodeGenHLSL/builtins/ScalarSwizzles.hlsl ../clang/test/SemaHLSL/Types/BuiltinVector/ScalarSwizzles.hlsl
../clang/test/CodeGenHLSL/builtins/ScalarSwizzles.hlsl ../clang/test/SemaHLSL/Types/BuiltinVector/ScalarSwizzles.hlsl
The new "correct" implicit conversion sequences change how some of these ASTs are formed and the initial code generation. ../clang/test/CodeGenHLSL/builtins/ScalarSwizzles.hlsl ../clang/test/SemaHLSL/Types/BuiltinVector/ScalarSwizzles.hlsl
llvm-beanz
force-pushed
the
cbieneman/hlsl-standard-conversions-trunc
branch
from
6bf40be
to
899d529
Compare
Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
|
@rjmccall & @efriedma-quic, do the CodeGen changes here look okay to you? |
clang/lib/CodeGen/CGExprScalar.cpp
Outdated
| @@ -1422,6 +1424,9 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType, | |||
| return Builder.CreateVectorSplat(NumElements, Src, "splat"); | |||
| } | |||
|
|
|||
| if (SrcType->isExtVectorType() && DstType->isExtVectorType()) | |||
| return EmitVectorElementConversion(SrcType, DstType, Src); | |||
There was a problem hiding this comment.
EmitScalarConversion is a mess; we have cast kinds, but we never taught this part of CodeGen to use them, so it just guesses the cast kind based on the source and destination types. This is particularly bad for vectors, since it's often possible to cast them in multiple different ways; I'm not confident this change won't have unintended side-effects. Please fix the code for the relevant cast kinds to avoid going through EmitScalarConversion.
There was a problem hiding this comment.
Yeah, I believe the reason we still have this whole analysis in EmitScalarConversion is, sadly enough, just to deal with the LHS and final result of a compound assignment expression, which we don't have CastKinds for. The more we can get away from relying on this, the better.
cbieneman/hlsl-standard-conversions-trunc
'beanz/cbieneman/hlsl-standard-conversions-trunc' by 1664 commits.
|
@rjmccall, do my latest updates here cover all your concerns? |
|
@rjmccall, ping |
|
@rjmccall, friendly ping 😄 |
rjmccall
reviewed
Feb 5, 2024
| @@ -361,6 +361,9 @@ CAST_OPERATION(AddressSpaceConversion) | |||
| // Convert an integer initializer to an OpenCL sampler. | |||
| CAST_OPERATION(IntToOCLSampler) | |||
|
|
|||
| // Truncate a vector type (HLSL only). | |||
| CAST_OPERATION(HLSLVectorTruncation) | |||
There was a problem hiding this comment.
Unfortunately, we already use "truncate" a lot to talk about narrowing integer conversions. Ideally, the term here wouldn't be confusable with that; I don't have a great alternative to suggest, though. Even if we can't find a better name, though, please briefly describe the conversion here, since it's not obvious from context.
As an aside: uh, wow, that sure seems like an unfortunate conversion to have in the language. I guess it's not that bad for something like dropping the last element from a 4-vector, where often it's really a 3-vector in disguise. Still, oof, what a footgun.
There was a problem hiding this comment.
Yea... HLSL has a lot of unfortunate implicit language behaviors. Part of our goal with the Clang implementation is to capture those behaviors in a way we can diagnose them more accurately, and slowly tighten up the language as we evolve it.
There was a problem hiding this comment.
Okay. I think this patch overall looks fine to land, except please do expand on the comment here to explain what truncating a vector type means (dropping elements from the end).
| CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, CE); | ||
| return EmitScalarConversion(Visit(E), E->getType(), DestTy, | ||
| CE->getExprLoc()); | ||
| } |
There was a problem hiding this comment.
Interesting. I guess we would've had a miscompile here except that Sema currently never emits these cast kinds for vector types?
Given that, I feel like this code shouldn't check for ExtVectorType specifically. If Sema in its wisdom tells us to emit one of these conversions, we should do it the right way for any vector type.
I guess HLSL probably doesn't care about any of the contextual things from the FP options. Do the constrained FP intrinsics even support vectors? Please at least leave a TODO about handling these properly.
There was a problem hiding this comment.
Will do. HLSL doesn't support constrained FP intrinsics (yet), but may eventually.
| Mask.insert(Mask.begin(), DestTy->castAs<VectorType>()->getNumElements(), | ||
| 0); | ||
| return Builder.CreateShuffleVector(Vec, Mask, "trunc"); | ||
| } |
There was a problem hiding this comment.
Okay, please tell me this is wrong. The result of truncating <A,B,C,D> to a 3-vector is <A,A,A>? I'm not misunderstanding something fundamental about shufflevector here, right?
| if (S.IsFloatingPointPromotion(FromType, ToType)) { | ||
| ICK = ICK_Floating_Promotion; | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This case is completely shadowed by the case above it, I think. I don't know whether it matters. Maybe the list-initialization logic won't allow your vector conversions in relatively late C++ modes because only promotions are allowed?
There was a problem hiding this comment.
Good catch! We do need this distinction to for overload resolution because we prefer promotions over conversions:
void Fn(vector<double,2> D);
void Fn(vector<half,2> H);
void Call(vector<float,2> F) {
Fn(F); // Should call the double version.
}That showed that I also missed a place where I need to factor in the element conversion to the conversion sequence rank. I'll get the fixed up.
|
Argh, sorry for the slow response. |
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.
cbieneman/hlsl-standard-conversions-trunc
'beanz/cbieneman/hlsl-standard-conversions-trunc'.
'beanz/cbieneman/hlsl-standard-conversions-trunc'.
bogner
approved these changes
Feb 14, 2024
clang/lib/Sema/SemaExprCXX.cpp
Outdated
| if (SCS.Element != ICK_Identity) { | ||
| // If SCS.Element is not ICK_Identity the To and From types must be HLSL | ||
| // vectors or matrices. HLSL matrices aren't yet supported so this code only | ||
| // handles vectors for now. |
There was a problem hiding this comment.
The comment about matrices should probably be a TODO:, and it might even be worth a separate assert that says "not implemented yet" for clarity
clang/lib/Sema/SemaExprCXX.cpp
Outdated
| switch (SCS.Element) { | ||
| case ICK_Identity: | ||
| // Nothing to do. | ||
| break; |
There was a problem hiding this comment.
SCS.Element == ICK_Identity is unreachable - we checked above
| // CHECK: [[vec1:%.*]] = load <1 x double>, ptr [[Tmp]], align 8 | ||
| // CHECK: [[vec3:%.*]] = shufflevector <1 x double> [[vec1]], <1 x double> poison, <3 x i32> zeroinitializer | ||
| // CHECK: [[vec3f:%.*]] = fptrunc <3 x double> [[vec3]] to <3 x float> | ||
| // ret <3 x float> [[vec3f]] | ||
|
|
||
| // ret <3 x float> [[Vec3F3]] |
There was a problem hiding this comment.
Why isn't the ret line a check? Also there seem to be two of them now.
| @@ -6432,7 +6432,7 @@ void InitializationSequence::InitializeFrom(Sema &S, | |||
| // For HLSL ext vector types we allow list initialization behavior for C++ | |||
| // constructor syntax. This is accomplished by converting initialization | |||
| // arguments an InitListExpr late. | |||
| if (S.getLangOpts().HLSL && DestType->isExtVectorType() && | |||
| if (S.getLangOpts().HLSL && Args.size() > 1 && DestType->isExtVectorType() && | |||
There was a problem hiding this comment.
This sort of looks like a separate bug fix. In any case I don't see a test for it (though maybe I missed it)
There was a problem hiding this comment.
This is subtle but there are tests that cover this. The tests introduced here cover the Args.size() == 1 case. Take this example:
void Fn(double2 D);
void Call(float2 F) {
Fn(F);
}In the call to Fn the argument becomes an implicit initialization list expression with one argument (fun right). In that case we rely on HLSL's standard conversion sequences to convert the first argument to the target type.
In other cases that we cover in the vector-constructors.hlsl test, we initialize vectors with initializer lists that are more than one argument like:
float2 f = float2(1, 2);| ICR_C_Conversion, | ||
| ICR_C_Conversion_Extension, | ||
| ICR_Conversion, | ||
| ICR_Conversion, |
There was a problem hiding this comment.
Might be nice to commit the clang-format changes in this file separately.
cbieneman/hlsl-standard-conversions-trunc
cbieneman/hlsl-standard-conversions-trunc 'beanz/cbieneman/hlsl-standard-conversions-trunc' by 798 commits. ../clang/test/Analysis/Checkers/WebKit/uncounted-obj-arg.cpp ../compiler-rt/test/profile/instrprof-block-coverage.c ../compiler-rt/test/profile/instrprof-entry-coverage.c ../libc/test/src/__support/integer_to_string_test.cpp ../libc/test/src/__support/str_to_long_double_test.cpp ../libcxx/include/__random/linear_congruential_engine.h ../libcxx/test/std/containers/container.adaptors/container.adaptors.form at/format.functions.tests.h ../libcxx/test/std/containers/sequences/vector.bool/vector.bool.fmt/form at.functions.tests.h ../libcxx/test/std/numerics/rand/rand.eng/rand.eng.lcong/alg.pass.cpp ../libcxx/test/std/numerics/rand/rand.eng/rand.eng.lcong/assign.pass.cpp ../libcxx/test/std/numerics/rand/rand.eng/rand.eng.lcong/copy.pass.cpp ../libcxx/test/std/numerics/rand/rand.eng/rand.eng.lcong/default.pass.cp p ../libcxx/test/std/numerics/rand/rand.eng/rand.eng.lcong/values.pass.cpp ../libcxx/test/std/thread/thread.threads/thread.thread.class/thread.thre ad.id/format.functions.tests.h ../libcxx/test/std/utilities/format/format.range/format.range.fmtmap/for mat.functions.tests.h ../libcxx/test/std/utilities/format/format.range/format.range.fmtset/for mat.functions.tests.h ../libcxx/test/std/utilities/format/format.range/format.range.fmtstr/for mat.functions.tests.h ../libcxx/test/std/utilities/format/format.range/format.range.formatter/ format.functions.tests.h ../libcxx/test/std/utilities/format/format.tuple/format.functions.tests. h ../llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp ../llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-opts-uzp1.ll ../llvm/test/tools/llvm-objdump/wasm/linked-symbol-table-namesec.yaml ../mlir/include/mlir/Dialect/AMDGPU/Transforms/Transforms.h ../mlir/lib/Dialect/AMDGPU/Transforms/OptimizeSharedMemory.cpp ../mlir/test/Dialect/AMDGPU/transform_optimize_shmem_reads_writes.mlir ../utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
'beanz/cbieneman/hlsl-standard-conversions-trunc'.
|
Is this causing these crashes? http://45.33.8.238/linux/130945/step_7.txt |
I can confirm that this commit is causing these crashes. @llvm-beanz can you take a look or revert? |
|
Seeing the crash in Linux CI toolchain builders. |
Prabhuk
added a commit
to Prabhuk/llvm-project
that referenced
this pull request
Feb 16, 2024
This reverts commit 5c57fd7. This commit causes crashes in Clang codegen tests.
llvm-beanz
added a commit
that referenced
this pull request
Feb 16, 2024
This should effectively preserve the old behavior for non-HLSL code.
|
I pushed a speculative fix in 0065161. I think the assert I added was too aggressive. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
HLSL supports vector truncation and element conversions as part of standard conversion sequences. The vector truncation conversion is a C++ second conversion in the conversion sequence. If a vector truncation is in a conversion sequence an element conversion may occur after it before the standard C++ third conversion.
Vector element conversions can be boolean conversions, floating point or integral conversions or promotions.
HLSL Draft Specification