Skip to content
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

Attach resource attributes to handle within record, instead of record #101433

Merged
merged 7 commits into from
Aug 3, 2024
Merged

Attach resource attributes to handle within record, instead of record #101433

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b17ddcc
add attrs to handle in record decl, add ast dump test to verify
bob80905 Aug 1, 2024
df4db59
clang format
bob80905 Aug 1, 2024
5885434
update ast test
bob80905 Aug 1, 2024
16fc4c7
pass attrs to add to addmembervar fxn
bob80905 Aug 1, 2024
6fa2c42
use const ref
bob80905 Aug 2, 2024
2987266
Update clang/lib/Sema/HLSLExternalSemaSource.cpp
bob80905 Aug 2, 2024
f303bd1
use array ref
bob80905 Aug 2, 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
24 changes: 14 additions & 10 deletions clang/lib/CodeGen/CGHLSLRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,18 +280,22 @@ void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, GlobalVariable *GV) {
const auto *RD = Ty->getAsCXXRecordDecl();
if (!RD)
return;
const auto *HLSLResAttr = RD->getAttr<HLSLResourceAttr>();
const auto *HLSLResClassAttr = RD->getAttr<HLSLResourceClassAttr>();
if (!HLSLResAttr || !HLSLResClassAttr)
return;
// the resource related attributes are on the handle member
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 not sure if this is too remedial, but for someone with my limited experience a comment along the lines of "If this variable's type is actually an HLSL resource then annotate it appropriately" would have helped me greatly in understanding it.

// inside the record decl
for (auto *FD : RD->fields()) {
const auto *HLSLResAttr = FD->getAttr<HLSLResourceAttr>();
const auto *HLSLResClassAttr = FD->getAttr<HLSLResourceClassAttr>();
if (!HLSLResAttr || !HLSLResClassAttr)
continue;

llvm::hlsl::ResourceClass RC = HLSLResClassAttr->getResourceClass();
llvm::hlsl::ResourceKind RK = HLSLResAttr->getResourceKind();
bool IsROV = HLSLResAttr->getIsROV();
llvm::hlsl::ElementType ET = calculateElementType(CGM.getContext(), Ty);
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>());
addBufferResourceAnnotation(GV, RC, RK, IsROV, ET, Binding);
BufferResBinding Binding(D->getAttr<HLSLResourceBindingAttr>());
addBufferResourceAnnotation(GV, RC, RK, IsROV, ET, Binding);
damyanp marked this conversation as resolved.
Show resolved Hide resolved
}
}

CGHLSLRuntime::BufferResBinding::BufferResBinding(
Expand Down
25 changes: 12 additions & 13 deletions clang/lib/Sema/HLSLExternalSemaSource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ struct BuiltinTypeDeclBuilder {

BuiltinTypeDeclBuilder &
addMemberVariable(StringRef Name, QualType Type,
const llvm::SmallVector<Attr *, 2> &Attrs,
bob80905 marked this conversation as resolved.
Show resolved Hide resolved
AccessSpecifier Access = AccessSpecifier::AS_private) {
if (Record->isCompleteDefinition())
return *this;
Expand All @@ -96,13 +97,16 @@ struct BuiltinTypeDeclBuilder {
nullptr, false, InClassInitStyle::ICIS_NoInit);
Field->setAccess(Access);
Field->setImplicit(true);
for (Attr *A : Attrs)
Field->addAttr(A);
Record->addDecl(Field);
Fields[Name] = Field;
return *this;
}

BuiltinTypeDeclBuilder &
addHandleMember(AccessSpecifier Access = AccessSpecifier::AS_private) {
addHandleMember(ResourceClass RC, ResourceKind RK, bool IsROV,
AccessSpecifier Access = AccessSpecifier::AS_private) {
if (Record->isCompleteDefinition())
return *this;
QualType Ty = Record->getASTContext().VoidPtrTy;
Expand All @@ -112,17 +116,13 @@ struct BuiltinTypeDeclBuilder {
Ty = Record->getASTContext().getPointerType(
QualType(TTD->getTypeForDecl(), 0));
}
return addMemberVariable("h", Ty, Access);
}

BuiltinTypeDeclBuilder &annotateHLSLResource(ResourceClass RC,
ResourceKind RK, bool IsROV) {
if (Record->isCompleteDefinition())
return *this;
Record->addAttr(
// add handle member
llvm::SmallVector<Attr *, 2> Attrs;
Attrs.push_back(
HLSLResourceClassAttr::CreateImplicit(Record->getASTContext(), RC));
Record->addAttr(
Attrs.push_back(
HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RK, IsROV));
addMemberVariable("h", Ty, Attrs, Access);
Copy link
Contributor

Choose a reason for hiding this comment

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

With the change to use ArrayRef above, I find constructing the array as part of the call slightly more readable:

    Attr *ResourceClassAttr =
        HLSLResourceClassAttr::CreateImplicit(Record->getASTContext(), RC);
    Attr *ResourceAttr =
        HLSLResourceClassAttr::CreateImplicit(Record->getASTContext(), RC);
    addMemberVariable("h", Ty, {ResourceClassAttr, ResourceAttr}, Access);

return *this;
}

Expand Down Expand Up @@ -489,9 +489,8 @@ static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
ResourceClass RC, ResourceKind RK,
bool IsROV) {
return BuiltinTypeDeclBuilder(Decl)
.addHandleMember()
.addDefaultHandleConstructor(S, RC)
.annotateHLSLResource(RC, RK, IsROV);
.addHandleMember(RC, RK, IsROV)
.addDefaultHandleConstructor(S, RC);
}

void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
Expand Down
4 changes: 2 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 @@ -30,9 +30,9 @@ 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: FieldDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> implicit h 'element_type *'
// 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'
// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> Idx 'unsigned int'
Expand All @@ -59,6 +59,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: FieldDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> implicit referenced h 'float *'
// 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 *'
14 changes: 14 additions & 0 deletions clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -ast-dump -o - %s | FileCheck %s

// CHECK: -ClassTemplateDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> implicit RWBuffer
// CHECK: -CXXRecordDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> implicit class RWBuffer definition
// CHECK: -FieldDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> implicit h 'element_type *'
// CHECK: -HLSLResourceClassAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit UAV
// CHECK: -HLSLResourceAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit TypedBuffer
RasterizerOrderedBuffer<vector<float, 4> > BufferArray3[4] : register(u4, space1);

// CHECK: -ClassTemplateSpecializationDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> class RWBuffer definition implicit_instantiation
// CHECK: -FieldDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> implicit referenced h 'float *'
// CHECK: -HLSLResourceClassAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit UAV
// CHECK: -HLSLResourceAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit TypedBuffer
RWBuffer<float> Buffer1;
Loading