Skip to content

Commit f9c2d5d

Browse files
simollsimoll
authored
[SER] Diagnose HitObject in unsupported declaration contexts (#7376)
- Generalize long vector diagnostics code to HitObjects. - Diagnose unsupported use of HitObject in globals, entry params/returns and various other shader-kind-specific contexts. - Create HitObject variants from the invalid-longvec-decls*.hlsl tests to make sure all cases are covered. Specification: https://github.com/microsoft/hlsl-specs/blob/main/proposals/0027-shader-execution-reordering.md Closes #7234 [SER] Diagnose and validate illegal use of HitObject in unsupported contexts (discussed offline)
1 parent 053e7ac commit f9c2d5d

17 files changed

+964
-162
lines changed

‎tools/clang/include/clang/AST/DeclCXX.h

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -465,10 +465,6 @@ class CXXRecordDecl : public RecordDecl {
465465
/// \brief Whether we are currently parsing base specifiers.
466466
bool IsParsingBaseSpecifiers : 1;
467467

468-
/// \brief Whether this class contains at least one member or base
469-
/// class containing an HLSL vector longer than 4 elements.
470-
bool HasHLSLLongVector : 1;
471-
472468
/// \brief The number of base class specifiers in Bases.
473469
unsigned NumBases;
474470

@@ -1022,13 +1018,6 @@ class CXXRecordDecl : public RecordDecl {
10221018
return data().NeedOverloadResolutionForDestructor;
10231019
}
10241020

1025-
// HLSL Change add HLSL Long vector bit.
1026-
/// \brief Determine whether this class contains an HLSL long vector
1027-
/// of over 4 elements.
1028-
bool hasHLSLLongVector() { return data().HasHLSLLongVector; }
1029-
/// \brief Set that this class contains an HLSL long vector of over 4 elements
1030-
bool setHasHLSLLongVector() { return data().HasHLSLLongVector = true; }
1031-
10321021
/// \brief Determine whether this class describes a lambda function object.
10331022
bool isLambda() const {
10341023
// An update record can't turn a non-lambda into a lambda.

‎tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7558,8 +7558,6 @@ def err_hlsl_missing_type_specifier : Error< // Patterened after err_missing_typ
75587558
"HLSL requires a type specifier for all declarations">;
75597559
def err_hlsl_multiple_concrete_bases : Error<
75607560
"multiple concrete base types specified">;
7561-
def err_hlsl_objectintemplateargument : Error<
7562-
"%0 is an object and cannot be used as a type parameter">;
75637561
def err_hlsl_packoffset_requires_cbuffer : Error<
75647562
"packoffset is only allowed in a constant buffer">;
75657563
def warn_hlsl_packoffset_mix : Warning<
@@ -7886,6 +7884,15 @@ def err_hlsl_unsupported_long_vector
78867884
"entry function parameters|entry function return type|"
78877885
"patch constant function parameters|patch constant function return type|"
78887886
"payload parameters|attributes}0 are not supported">;
7887+
// First %select options must match err_hlsl_unsupported_long_vector (same index used)
7888+
def err_hlsl_unsupported_object_context
7889+
: Error<"object %0 is not allowed in "
7890+
"%select{ConstantBuffers or TextureBuffers|"
7891+
"tessellation patches|geometry streams|node records|"
7892+
"cbuffers or tbuffers|user-defined struct parameter|"
7893+
"entry function parameters|entry function return type|"
7894+
"patch constant function parameters|patch constant function return type|"
7895+
"payload parameters|attributes|builtin template parameters|structured buffers|global variables|groupshared variables}1">;
78897896
def err_hlsl_logical_binop_scalar : Error<
78907897
"operands for short-circuiting logical binary operator must be scalar, for non-scalar types use '%select{and|or}0'">;
78917898
def err_hlsl_ternary_scalar : Error<
@@ -7970,8 +7977,6 @@ def err_hlsl_too_many_node_inputs : Error<
79707977
"Node shader '%0' may not have more than one input record">;
79717978
def err_hlsl_node_record_type : Error<
79727979
"%0 is not valid as a node record type - struct/class required">;
7973-
def err_hlsl_node_record_object : Error<
7974-
"object %0 may not appear in a node record">;
79757980
def err_hlsl_array_disallowed : Error<
79767981
"%select{entry parameter|declaration}1 of type %0 may not be an array">;
79777982
def err_hlsl_inputpatch_size: Error<

‎tools/clang/include/clang/Sema/SemaHLSL.h

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,38 @@ bool DiagnoseNodeStructArgument(clang::Sema *self,
5959
clang::QualType ArgTy, bool &Empty,
6060
const clang::FieldDecl *FD = nullptr);
6161

62+
// Keep this in sync with err_hlsl_unsupported_object in DiagnosticSemaKinds.td
63+
enum class TypeDiagContext {
64+
// Indices that the type context is valid and no diagnostics should be emitted
65+
// for this type category.
66+
Valid = -1,
67+
// Supported indices for both `err_hlsl_unsupported_object_context` and
68+
// `err_hlsl_unsupported_long_vector`
69+
ConstantBuffersOrTextureBuffers = 0,
70+
TessellationPatches = 1,
71+
GeometryStreams = 2,
72+
NodeRecords = 3,
73+
CBuffersOrTBuffers = 4,
74+
UserDefinedStructParameter = 5,
75+
EntryFunctionParameters = 6,
76+
EntryFunctionReturnType = 7,
77+
PatchConstantFunctionParameters = 8,
78+
PatchConstantFunctionReturnType = 9,
79+
PayloadParameters = 10,
80+
Attributes = 11,
81+
TypeParameter = 12,
82+
LongVecDiagMaxSelectIndex = TypeParameter,
83+
// Below only supported for `err_hlsl_diag_unsupported_object_context`
84+
StructuredBuffers = 13,
85+
GlobalVariables = 14,
86+
GroupShared = 15,
87+
DiagMaxSelectIndex = 15,
88+
};
89+
bool DiagnoseTypeElements(clang::Sema &S, clang::SourceLocation Loc,
90+
clang::QualType Ty, TypeDiagContext ObjDiagContext,
91+
TypeDiagContext LongVecDiagContext,
92+
const clang::FieldDecl *FD = nullptr);
93+
6294
void DiagnoseControlFlowConditionForHLSL(clang::Sema *self,
6395
clang::Expr *condExpr,
6496
llvm::StringRef StmtName);

‎tools/clang/lib/AST/DeclCXX.cpp

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D)
7272
ImplicitCopyAssignmentHasConstParam(true),
7373
HasDeclaredCopyConstructorWithConstParam(false),
7474
HasDeclaredCopyAssignmentWithConstParam(false), IsLambda(false),
75-
IsParsingBaseSpecifiers(false), HasHLSLLongVector(false), NumBases(0),
76-
NumVBases(0), Bases(), VBases(), Definition(D), FirstFriend() {}
75+
IsParsingBaseSpecifiers(false), NumBases(0), NumVBases(0), Bases(),
76+
VBases(), Definition(D), FirstFriend() {}
7777
// HLSL Change End - Add HasLongVector and clang-format
7878

7979
CXXBaseSpecifier *CXXRecordDecl::DefinitionData::getBasesSlowCase() const {
@@ -203,11 +203,6 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases,
203203
if (!BaseClassDecl->isStandardLayout())
204204
data().IsStandardLayout = false;
205205

206-
// HLSL Change Begin - Propagate presence of long vector to child classes.
207-
if (BaseClassDecl->hasHLSLLongVector())
208-
data().HasHLSLLongVector = true;
209-
// HLSL Change End
210-
211206
// Record if this base is the first non-literal field or base.
212207
if (!hasNonLiteralTypeFieldsOrBases() && !BaseType->isLiteralType(C))
213208
data().HasNonLiteralTypeFieldsOrBases = true;
@@ -389,11 +384,6 @@ void CXXRecordDecl::addedClassSubobject(CXXRecordDecl *Subobj) {
389384
data().NeedOverloadResolutionForMoveConstructor = true;
390385
data().NeedOverloadResolutionForDestructor = true;
391386
}
392-
393-
// HLSL Change Begin - Propagate presence of long vector to child classes.
394-
if (Subobj->hasHLSLLongVector())
395-
data().HasHLSLLongVector = true;
396-
// HLSL Change End
397387
}
398388

399389
/// Callback function for CXXRecordDecl::forallBases that acknowledges

‎tools/clang/lib/AST/HlslTypes.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,13 @@ bool IsHLSLCopyableAnnotatableRecord(clang::QualType QT) {
120120
if (!IsHLSLNumericOrAggregateOfNumericType(Member->getType()))
121121
return false;
122122
}
123+
if (auto *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
124+
// Walk up the inheritance chain and check base class fields
125+
for (const auto &Base : CXXRD->bases()) {
126+
if (!IsHLSLCopyableAnnotatableRecord(Base.getType()))
127+
return false;
128+
}
129+
}
123130
return true;
124131
}
125132
return false;

‎tools/clang/lib/Sema/SemaDXR.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -827,19 +827,16 @@ void DiagnoseBuiltinCallWithPayload(Sema &S, const VarDecl *Payload,
827827
}
828828

829829
// Verify that the payload type is legal
830-
if (!hlsl::IsHLSLCopyableAnnotatableRecord(Payload->getType())) {
830+
if (!hlsl::IsHLSLCopyableAnnotatableRecord(Payload->getType()))
831831
S.Diag(Payload->getLocation(), diag::err_payload_attrs_must_be_udt)
832832
<< /*payload|attributes|callable*/ 0 << /*parameter %2|type*/ 0
833833
<< Payload;
834-
return;
835-
}
836834

837-
if (ContainsLongVector(Payload->getType())) {
838-
const unsigned PayloadParametersIdx = 10;
839-
S.Diag(Payload->getLocation(), diag::err_hlsl_unsupported_long_vector)
840-
<< PayloadParametersIdx;
835+
// This will produce more details, but also catch disallowed long vectors
836+
const TypeDiagContext DiagContext = TypeDiagContext::PayloadParameters;
837+
if (DiagnoseTypeElements(S, Payload->getLocation(), Payload->getType(),
838+
DiagContext, DiagContext))
841839
return;
842-
}
843840

844841
CollectNonAccessableFields(PayloadType, CallerStage, {}, {},
845842
NonWriteableFields, NonReadableFields);

0 commit comments

Comments
 (0)