Skip to content

Commit

Permalink
Implement resource binding type prefix mismatch diagnostic infrastruc…
Browse files Browse the repository at this point in the history
…ture (#97103)

There are currently no diagnostics being emitted for when a resource is
bound to a register with an incorrect binding type prefix. For example,
a CBuffer type resource should be bound with a a binding type prefix of
'b', but if instead the prefix is 'u', no errors will be emitted. This
PR implements such diagnostics. The focus of this PR is to implement
both the flag setting and diagnostic emisison steps specified in the
relevant spec: microsoft/hlsl-specs#230
The relevant issue is: #57886
This is a continuation / refresh of this PR:
#87578
  • Loading branch information
bob80905 authored Aug 23, 2024
1 parent fa089ef commit ebc4a66
Show file tree
Hide file tree
Showing 22 changed files with 751 additions and 60 deletions.
4 changes: 2 additions & 2 deletions clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -4538,7 +4538,7 @@ def HLSLSV_GroupIndex: HLSLAnnotationAttr {

def HLSLResourceBinding: InheritableAttr {
let Spellings = [HLSLAnnotation<"register">];
let Subjects = SubjectList<[HLSLBufferObj, ExternalGlobalVar]>;
let Subjects = SubjectList<[HLSLBufferObj, ExternalGlobalVar], ErrorDiag>;
let LangOpts = [HLSL];
let Args = [StringArgument<"Slot">, StringArgument<"Space", 1>];
let Documentation = [HLSLResourceBindingDocs];
Expand Down Expand Up @@ -4622,7 +4622,7 @@ def HLSLROV : InheritableAttr {

def HLSLResourceClass : InheritableAttr {
let Spellings = [CXX11<"hlsl", "resource_class">];
let Subjects = SubjectList<[Struct]>;
let Subjects = SubjectList<[Field]>;
let LangOpts = [HLSL];
let Args = [
EnumArgument<"ResourceClass", "llvm::hlsl::ResourceClass",
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticGroups.td
Original file line number Diff line number Diff line change
Expand Up @@ -1547,6 +1547,9 @@ def DXILValidation : DiagGroup<"dxil-validation">;
// Warning for HLSL API availability
def HLSLAvailability : DiagGroup<"hlsl-availability">;

// Warnings for legacy binding behavior
def LegacyConstantRegisterBinding : DiagGroup<"legacy-constant-register-binding">;

// Warnings and notes related to const_var_decl_type attribute checks
def ReadOnlyPlacementChecks : DiagGroup<"read-only-types">;

Expand Down
8 changes: 7 additions & 1 deletion clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -12352,7 +12352,13 @@ def err_hlsl_missing_semantic_annotation : Error<
def err_hlsl_init_priority_unsupported : Error<
"initializer priorities are not supported in HLSL">;

def err_hlsl_unsupported_register_type : Error<"invalid resource class specifier '%0' used; expected 'b', 's', 't', or 'u'">;
def warn_hlsl_user_defined_type_missing_member: Warning<"binding type '%select{t|u|b|s|c}0' only applies to types containing %select{SRV resources|UAV resources|constant buffer resources|sampler state|numeric types}0">, InGroup<LegacyConstantRegisterBinding>;
def err_hlsl_binding_type_mismatch: Error<"binding type '%select{t|u|b|s|c}0' only applies to %select{SRV resources|UAV resources|constant buffer resources|sampler state|numeric variables in the global scope}0">;
def err_hlsl_binding_type_invalid: Error<"binding type '%0' is invalid">;
def err_hlsl_duplicate_register_annotation: Error<"binding type '%select{t|u|b|s|c|i}0' cannot be applied more than once">;
def warn_hlsl_register_type_c_packoffset: Warning<"binding type 'c' ignored in buffer declaration. Did you mean 'packoffset'?">, InGroup<LegacyConstantRegisterBinding>, DefaultError;
def warn_hlsl_deprecated_register_type_b: Warning<"binding type 'b' only applies to constant buffers. The 'bool constant' binding type is no longer supported">, InGroup<LegacyConstantRegisterBinding>, DefaultError;
def warn_hlsl_deprecated_register_type_i: Warning<"binding type 'i' ignored. The 'integer constant' binding type is no longer supported">, InGroup<LegacyConstantRegisterBinding>, DefaultError;
def err_hlsl_unsupported_register_number : Error<"register number should be an integer">;
def err_hlsl_expected_space : Error<"invalid space specifier '%0' used; expected 'space' followed by an integer, like space1">;
def warn_hlsl_packoffset_mix : Warning<"cannot mix packoffset elements with nonpackoffset elements in a cbuffer">,
Expand Down
4 changes: 3 additions & 1 deletion clang/include/clang/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -3021,15 +3021,17 @@ class Parser : public CodeCompletionHandler {
SemaCodeCompletion::AttributeCompletion::None,
const IdentifierInfo *EnclosingScope = nullptr);

void MaybeParseHLSLAnnotations(Declarator &D,
bool MaybeParseHLSLAnnotations(Declarator &D,
SourceLocation *EndLoc = nullptr,
bool CouldBeBitField = false) {
assert(getLangOpts().HLSL && "MaybeParseHLSLAnnotations is for HLSL only");
if (Tok.is(tok::colon)) {
ParsedAttributes Attrs(AttrFactory);
ParseHLSLAnnotations(Attrs, EndLoc, CouldBeBitField);
D.takeAttributes(Attrs);
return true;
}
return false;
}

void MaybeParseHLSLAnnotations(ParsedAttributes &Attrs,
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2326,7 +2326,8 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
}

if (getLangOpts().HLSL)
MaybeParseHLSLAnnotations(D);
while (MaybeParseHLSLAnnotations(D))
;

if (Tok.is(tok::kw_requires))
ParseTrailingRequiresClause(D);
Expand Down
4 changes: 3 additions & 1 deletion clang/lib/Sema/HLSLExternalSemaSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -503,9 +503,11 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWBuffer")
.addSimpleTemplateParams(*SemaPtr, {"element_type"})
.Record;

onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
ResourceKind::TypedBuffer, /*IsROV=*/false)
ResourceKind::TypedBuffer,
/*IsROV=*/false)
.addArraySubscriptOperators()
.completeDefinition();
});
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Sema/SemaDeclAttr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6889,6 +6889,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
case ParsedAttr::AT_HLSLSV_GroupIndex:
handleSimpleAttribute<HLSLSV_GroupIndexAttr>(S, D, AL);
break;
case ParsedAttr::AT_HLSLGroupSharedAddressSpace:
handleSimpleAttribute<HLSLGroupSharedAddressSpaceAttr>(S, D, AL);
break;
case ParsedAttr::AT_HLSLSV_DispatchThreadID:
S.HLSL().handleSV_DispatchThreadIDAttr(D, AL);
break;
Expand Down
Loading

0 comments on commit ebc4a66

Please sign in to comment.