Skip to content

[HLSL] Vector standard conversions #71098

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/include/clang/AST/OperationKinds.def
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,9 @@ CAST_OPERATION(AddressSpaceConversion)
// Convert an integer initializer to an OpenCL sampler.
CAST_OPERATION(IntToOCLSampler)

// Truncate a vector type by dropping elements from the end (HLSL only).
CAST_OPERATION(HLSLVectorTruncation)
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea... 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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).


//===- Binary Operations -------------------------------------------------===//
// Operators listed in order of precedence.
// Note that additions to this should also update the StmtVisitor class,
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -12112,6 +12112,9 @@ def err_hlsl_operator_unsupported : Error<
def err_hlsl_param_qualifier_mismatch :
Error<"conflicting parameter qualifier %0 on parameter %1">;

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">;
Expand Down
12 changes: 11 additions & 1 deletion clang/include/clang/Sema/Overload.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,9 @@ class Sema;
/// Fixed point type conversions according to N1169.
ICK_Fixed_Point_Conversion,

/// HLSL vector truncation.
ICK_HLSL_Vector_Truncation,

/// The number of conversion kinds
ICK_Num_Conversion_Kinds,
};
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -367,7 +376,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;
Expand Down
1 change: 1 addition & 0 deletions clang/lib/AST/Expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1897,6 +1897,7 @@ bool CastExpr::CastConsistency() const {
case CK_FixedPointToIntegral:
case CK_IntegralToFixedPoint:
case CK_MatrixCast:
case CK_HLSLVectorTruncation:
assert(!getType()->isBooleanType() && "unheralded conversion to bool");
goto CheckNoBasePath;

Expand Down
2 changes: 2 additions & 0 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13980,6 +13980,7 @@ bool IntExprEvaluator::VisitCastExpr(const CastExpr *E) {
case CK_FixedPointCast:
case CK_IntegralToFixedPoint:
case CK_MatrixCast:
case CK_HLSLVectorTruncation:
llvm_unreachable("invalid cast kind for integral value");

case CK_BitCast:
Expand Down Expand Up @@ -14818,6 +14819,7 @@ bool ComplexExprEvaluator::VisitCastExpr(const CastExpr *E) {
case CK_FixedPointToIntegral:
case CK_IntegralToFixedPoint:
case CK_MatrixCast:
case CK_HLSLVectorTruncation:
llvm_unreachable("invalid cast kind for complex value");

case CK_LValueToRValue:
Expand Down
1 change: 1 addition & 0 deletions clang/lib/CodeGen/CGExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5180,6 +5180,7 @@ LValue CodeGenFunction::EmitCastLValue(const CastExpr *E) {
case CK_FixedPointToIntegral:
case CK_IntegralToFixedPoint:
case CK_MatrixCast:
case CK_HLSLVectorTruncation:
return EmitUnsupportedLValue(E, "unexpected cast lvalue");

case CK_Dependent:
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/CodeGen/CGExprAgg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,7 @@ void AggExprEmitter::VisitCastExpr(CastExpr *E) {
case CK_BuiltinFnToFnPtr:
case CK_ZeroToOCLOpaqueType:
case CK_MatrixCast:
case CK_HLSLVectorTruncation:

case CK_IntToOCLSampler:
case CK_FloatingToFixedPoint:
Expand Down Expand Up @@ -1457,6 +1458,7 @@ static bool castPreservesZero(const CastExpr *CE) {
case CK_MatrixCast:
case CK_NonAtomicToAtomic:
case CK_AtomicToNonAtomic:
case CK_HLSLVectorTruncation:
return true;

case CK_BaseToDerivedMemberPointer:
Expand Down
1 change: 1 addition & 0 deletions clang/lib/CodeGen/CGExprComplex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,7 @@ ComplexPairTy ComplexExprEmitter::EmitCast(CastKind CK, Expr *Op,
case CK_FixedPointToIntegral:
case CK_IntegralToFixedPoint:
case CK_MatrixCast:
case CK_HLSLVectorTruncation:
llvm_unreachable("invalid cast kind for complex value");

case CK_FloatingRealToComplex:
Expand Down
1 change: 1 addition & 0 deletions clang/lib/CodeGen/CGExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,7 @@ class ConstExprEmitter :
case CK_IntegralToFixedPoint:
case CK_ZeroToOCLOpaqueType:
case CK_MatrixCast:
case CK_HLSLVectorTruncation:
return nullptr;
}
llvm_unreachable("Invalid CastKind");
Expand Down
64 changes: 61 additions & 3 deletions clang/lib/CodeGen/CGExprScalar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2408,6 +2408,12 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
CE->getExprLoc());

case CK_IntegralCast: {
if (E->getType()->isExtVectorType() && DestTy->isExtVectorType()) {
QualType SrcElTy = E->getType()->castAs<VectorType>()->getElementType();
return Builder.CreateIntCast(Visit(E), ConvertType(DestTy),
SrcElTy->isSignedIntegerOrEnumerationType(),
"conv");
}
ScalarConversionOpts Opts;
if (auto *ICE = dyn_cast<ImplicitCastExpr>(CE)) {
if (!ICE->isPartOfExplicitCast())
Expand All @@ -2416,9 +2422,50 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
return EmitScalarConversion(Visit(E), E->getType(), DestTy,
CE->getExprLoc(), Opts);
}
case CK_IntegralToFloating:
case CK_FloatingToIntegral:
case CK_FloatingCast:
case CK_IntegralToFloating: {
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");
return Builder.CreateUIToFP(Visit(E), ConvertType(DestTy), "conv");
}
CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, CE);
return EmitScalarConversion(Visit(E), E->getType(), DestTy,
CE->getExprLoc());
}
case CK_FloatingToIntegral: {
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");
return Builder.CreateFPToUI(Visit(E), ConvertType(DestTy), "conv");
}
CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, CE);
return EmitScalarConversion(Visit(E), E->getType(), DestTy,
CE->getExprLoc());
}
case CK_FloatingCast: {
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() <
SrcElTy->castAs<BuiltinType>()->getKind())
return Builder.CreateFPTrunc(Visit(E), ConvertType(DestTy), "conv");
return Builder.CreateFPExt(Visit(E), ConvertType(DestTy), "conv");
}
CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, CE);
return EmitScalarConversion(Visit(E), E->getType(), DestTy,
CE->getExprLoc());
}
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do. HLSL doesn't support constrained FP intrinsics (yet), but may eventually.

case CK_FixedPointToFloating:
case CK_FloatingToFixedPoint: {
CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, CE);
Expand Down Expand Up @@ -2468,6 +2515,17 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
case CK_IntToOCLSampler:
return CGF.CGM.createOpenCLIntToSamplerConversion(E, CGF);

case CK_HLSLVectorTruncation: {
assert(DestTy->isVectorType() && "Expected dest type to be vector type");
Value *Vec = Visit(const_cast<Expr *>(E));
SmallVector<int, 16> Mask;
unsigned NumElts = DestTy->castAs<VectorType>()->getNumElements();
for (unsigned I = 0; I != NumElts; ++I)
Mask.push_back(I);

return Builder.CreateShuffleVector(Vec, Mask, "trunc");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Doh!


} // end of switch

llvm_unreachable("unknown scalar cast");
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/Edit/RewriteObjCFoundationAPI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,10 @@ static bool rewriteToNumericBoxedExpression(const ObjCMessageExpr *Msg,
case CK_BooleanToSignedIntegral:
llvm_unreachable("OpenCL-specific cast in Objective-C?");

case CK_HLSLVectorTruncation:
llvm_unreachable("HLSL-specific cast in Objective-C?");
break;

case CK_FloatingToFixedPoint:
case CK_FixedPointToFloating:
case CK_FixedPointCast:
Expand Down
11 changes: 9 additions & 2 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15676,11 +15676,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->castAs<VectorType>()->getNumElements() <
Source->castAs<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();
Expand Down
86 changes: 86 additions & 0 deletions clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4762,6 +4762,22 @@ 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:
Expand All @@ -4774,6 +4790,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.

// 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,
/*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;
case ICK_Identity:
default:
llvm_unreachable("Improper element standard conversion");
}
}

switch (SCS.Third) {
case ICK_Identity:
// Nothing to do.
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() &&
Copy link
Contributor

Choose a reason for hiding this comment

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

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)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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);

(SourceType.isNull() ||
!Context.hasSameUnqualifiedType(SourceType, DestType))) {

Expand Down
Loading