Skip to content

[HLSL] Split out resource class data from resource attribute #98419

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.

Sign up for GitHub

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 8 commits into from
Jul 12, 2024
Merged

[HLSL] Split out resource class data from resource attribute #98419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b740aa9
split out resource class data from resource attr, add some prelim dia…
bob80905 Jul 11, 2024
d5d4b00
add back ResourceClass for default handle constructor for codegen tests
bob80905 Jul 11, 2024
8b12c39
add hlslresourceclassattr to attr list test
bob80905 Jul 11, 2024
65da73a
Update clang/lib/CodeGen/CGHLSLRuntime.cpp
bob80905 Jul 11, 2024
9cf74db
shorten tests, include sloc in attr construction, use existing RC val…
bob80905 Jul 11, 2024
a9a02c4
remove unneeded error, update test
bob80905 Jul 11, 2024
30c3b4c
remove unneeded check, simplify verbose var name, use extra sloc arg …
bob80905 Jul 11, 2024
2539a08
remove entry points and make tests dump ast
bob80905 Jul 12, 2024
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
19 changes: 14 additions & 5 deletions clang/include/clang/Basic/Attr.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4549,11 +4549,7 @@ def HLSLResource : InheritableAttr {
let Spellings = [];
let Subjects = SubjectList<[Struct]>;
let LangOpts = [HLSL];
let Args = [
EnumArgument<"ResourceClass", "llvm::hlsl::ResourceClass",
/*is_string=*/0, ["SRV", "UAV", "CBuffer", "Sampler"],
["SRV", "UAV", "CBuffer", "Sampler"],
/*opt=*/0, /*fake=*/0, /*isExternalType=*/1>,
let Args = [
EnumArgument<
"ResourceKind", "llvm::hlsl::ResourceKind",
/*is_string=*/0,
Expand All @@ -4577,6 +4573,19 @@ def HLSLResource : InheritableAttr {
let Documentation = [InternalOnly];
}

def HLSLResourceClass : InheritableAttr {
let Spellings = [CXX11<"hlsl", "resource_class">];
let Subjects = SubjectList<[Struct]>;
let LangOpts = [HLSL];
let Args = [
EnumArgument<"ResourceClass", "llvm::hlsl::ResourceClass",
/*is_string=*/true, ["SRV", "UAV", "CBuffer", "Sampler"],
["SRV", "UAV", "CBuffer", "Sampler"],
/*opt=*/0, /*fake=*/0, /*isExternalType=*/1>
];
let Documentation = [InternalOnly];
}

def HLSLGroupSharedAddressSpace : TypeAttr {
let Spellings = [CustomKeyword<"groupshared">];
let Subjects = SubjectList<[Var]>;
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Sema/SemaHLSL.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class SemaHLSL : public SemaBase {
void handleSV_DispatchThreadIDAttr(Decl *D, const ParsedAttr &AL);
void handlePackOffsetAttr(Decl *D, const ParsedAttr &AL);
void handleShaderAttr(Decl *D, const ParsedAttr &AL);
void handleResourceClassAttr(Decl *D, const ParsedAttr &AL);
void handleResourceBindingAttr(Decl *D, const ParsedAttr &AL);
void handleParamModifierAttr(Decl *D, const ParsedAttr &AL);

Expand Down
11 changes: 6 additions & 5 deletions clang/lib/CodeGen/CGHLSLRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,13 +280,14 @@ void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, GlobalVariable *GV) {
const auto *RD = Ty->getAsCXXRecordDecl();
if (!RD)
return;
const auto *Attr = RD->getAttr<HLSLResourceAttr>();
if (!Attr)
const auto *HLSLResAttr = RD->getAttr<HLSLResourceAttr>();
const auto *HLSLResClassAttr = RD->getAttr<HLSLResourceClassAttr>();
if (!HLSLResAttr || !HLSLResClassAttr)
return;

llvm::hlsl::ResourceClass RC = Attr->getResourceClass();
llvm::hlsl::ResourceKind RK = Attr->getResourceKind();
bool IsROV = Attr->getIsROV();
llvm::hlsl::ResourceClass RC = HLSLResClassAttr->getResourceClass();
llvm::hlsl::ResourceKind RK = HLSLResAttr->getResourceKind();
bool IsROV = HLSLResAttr->getIsROV();
llvm::hlsl::ElementType ET = calculateElementType(CGM.getContext(), Ty);

BufferResBinding Binding(D->getAttr<HLSLResourceBindingAttr>());
Expand Down
13 changes: 7 additions & 6 deletions clang/lib/Sema/HLSLExternalSemaSource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,14 @@ struct BuiltinTypeDeclBuilder {
return addMemberVariable("h", Ty, Access);
}

BuiltinTypeDeclBuilder &annotateResourceClass(ResourceClass RC,
ResourceKind RK, bool IsROV) {
BuiltinTypeDeclBuilder &annotateHLSLResource(ResourceClass RC,
ResourceKind RK, bool IsROV) {
if (Record->isCompleteDefinition())
return *this;
Record->addAttr(HLSLResourceAttr::CreateImplicit(Record->getASTContext(),
RC, RK, IsROV));
Record->addAttr(
HLSLResourceClassAttr::CreateImplicit(Record->getASTContext(), RC));
Record->addAttr(
HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RK, IsROV));
return *this;
}

Expand Down Expand Up @@ -171,7 +173,6 @@ struct BuiltinTypeDeclBuilder {

DeclRefExpr *Fn =
lookupBuiltinFunction(AST, S, "__builtin_hlsl_create_handle");

Expr *RCExpr = emitResourceClassExpr(AST, RC);
Expr *Call = CallExpr::Create(AST, Fn, {RCExpr}, AST.VoidPtrTy, VK_PRValue,
SourceLocation(), FPOptionsOverride());
Expand Down Expand Up @@ -496,7 +497,7 @@ static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
return BuiltinTypeDeclBuilder(Decl)
.addHandleMember()
.addDefaultHandleConstructor(S, RC)
.annotateResourceClass(RC, RK, IsROV);
.annotateHLSLResource(RC, RK, IsROV);
}

void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Sema/SemaDeclAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7053,6 +7053,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
case ParsedAttr::AT_HLSLResourceBinding:
S.HLSL().handleResourceBindingAttr(D, AL);
break;
case ParsedAttr::AT_HLSLResourceClass:
S.HLSL().handleResourceClassAttr(D, AL);
break;
case ParsedAttr::AT_HLSLParamModifier:
S.HLSL().handleParamModifierAttr(D, AL);
break;
Expand Down
22 changes: 22 additions & 0 deletions clang/lib/Sema/SemaHLSL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,28 @@ void SemaHLSL::handleShaderAttr(Decl *D, const ParsedAttr &AL) {
D->addAttr(NewAttr);
}

void SemaHLSL::handleResourceClassAttr(Decl *D, const ParsedAttr &AL) {
if (!AL.isArgIdent(0)) {
Diag(AL.getLoc(), diag::err_attribute_argument_type)
<< AL << AANT_ArgumentIdentifier;
return;
}

IdentifierLoc *Loc = AL.getArgAsIdent(0);
StringRef Identifier = Loc->Ident->getName();
SourceLocation ArgLoc = Loc->Loc;

// Validate.
llvm::dxil::ResourceClass RC;
if (!HLSLResourceClassAttr::ConvertStrToResourceClass(Identifier, RC)) {
Diag(ArgLoc, diag::warn_attribute_type_not_supported)
<< "ResourceClass" << Identifier;
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to use HLSLResourceClassAttr::ConvertStrToResourceClass here, rather than hand rolling this. It's also probably fine to just emit diag::warn_attribute_type_not_supported rather than create a more specific message just for this.


D->addAttr(HLSLResourceClassAttr::Create(getASTContext(), RC, ArgLoc));
}

void SemaHLSL::handleResourceBindingAttr(Decl *D, const ParsedAttr &AL) {
StringRef Space = "space0";
StringRef Slot = "";
Expand Down
6 changes: 4 additions & 2 deletions clang/test/AST/HLSL/RWBuffer-AST.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ RWBuffer<float> Buffer;
// CHECK-NEXT: CXXRecordDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> implicit class RWBuffer definition

// CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit final
// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit UAV TypedBuffer
// CHECK-NEXT: HLSLResourceClassAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit UAV
// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit TypedBuffer
// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> implicit h 'element_type *'

// CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> operator[] 'element_type &const (unsigned int) const'
Expand Down Expand Up @@ -62,5 +63,6 @@ RWBuffer<float> Buffer;
// CHECK: TemplateArgument type 'float'
// CHECK-NEXT: BuiltinType 0x{{[0-9A-Fa-f]+}} 'float'
// CHECK-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit final
// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit UAV TypedBuffer
// CHECK-NEXT: HLSLResourceClassAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit UAV
// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit TypedBuffer
// CHECK-NEXT: FieldDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> implicit referenced h 'float *'
1 change: 1 addition & 0 deletions clang/test/Misc/pragma-attribute-supported-attributes-list.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
// CHECK-NEXT: FunctionReturnThunks (SubjectMatchRule_function)
// CHECK-NEXT: GNUInline (SubjectMatchRule_function)
// CHECK-NEXT: HIPManaged (SubjectMatchRule_variable)
// CHECK-NEXT: HLSLResourceClass (SubjectMatchRule_record_not_is_union)
// CHECK-NEXT: Hot (SubjectMatchRule_function)
// CHECK-NEXT: IBAction (SubjectMatchRule_objc_method_is_instance)
// CHECK-NEXT: IFunc (SubjectMatchRule_function)
Expand Down
32 changes: 32 additions & 0 deletions clang/test/ParserHLSL/hlsl_resource_class_attr.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s


// CHECK: -HLSLResourceClassAttr 0x{{[0-9a-f]+}} <col:31> SRV
struct [[hlsl::resource_class(SRV)]] Eg1 {
int i;
};

Eg1 e1;

// CHECK: -CXXRecordDecl 0x{{[0-9a-f]+}} <line:13:1, line:15:1> line:13:38 referenced struct Eg2 definition
// CHECK: -HLSLResourceClassAttr 0x{{[0-9a-f]+}} <col:31> UAV
struct [[hlsl::resource_class(UAV)]] Eg2 {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised this has an invalid source location given that it's coming from the source. Did you use the right constructor for the attribute?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added ArgLoc to the constructor, it was an optional arg

int i;
};
Eg2 e2;

// CHECK: -CXXRecordDecl 0x{{[0-9a-f]+}} <line:20:1, line:22:1> line:20:42 referenced struct Eg3 definition
// CHECK: -HLSLResourceClassAttr 0x{{[0-9a-f]+}} <col:31> CBuffer
struct [[hlsl::resource_class(CBuffer)]] Eg3 {
int i;
};
Eg3 e3;

// CHECK: -CXXRecordDecl 0x{{[0-9a-f]+}} <line:27:1, line:29:1> line:27:42 referenced struct Eg4 definition
// CHECK: -HLSLResourceClassAttr 0x{{[0-9a-f]+}} <col:31> Sampler
struct [[hlsl::resource_class(Sampler)]] Eg4 {
int i;
};
Eg4 e4;

RWBuffer<int> In : register(u1);
15 changes: 15 additions & 0 deletions clang/test/ParserHLSL/hlsl_resource_class_attr_error.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s -verify

// expected-error@+1{{'resource_class' attribute takes one argument}}
struct [[hlsl::resource_class()]] Eg1 {
int i;
};

Eg1 e1;

// expected-warning@+1{{ResourceClass attribute argument not supported: gibberish}}
struct [[hlsl::resource_class(gibberish)]] Eg2 {
int i;
};

Eg2 e2;
Loading