Skip to content

[Sema] Add and test new Subobject Attribute#7258

Merged
bob80905 merged 9 commits intomicrosoft:mainfrom
bob80905:AddSubobjectAttribute
Mar 27, 2025
Merged

[Sema] Add and test new Subobject Attribute#7258
bob80905 merged 9 commits intomicrosoft:mainfrom
bob80905:AddSubobjectAttribute

Conversation

@bob80905
Copy link
Copy Markdown
Collaborator

This PR adds and tests a new subobject attribute. It will be useful for checking if a given decl is a subobject decl. This functionality will be used in #7239
We need an attribute in order to determine whether to check its initializer for availability attributes or not.

Fixes #7257

RecordDecl *RD = RT->getDecl();
if (!RD->hasAttr<HLSLSubObjectAttr>()) {
return false;
}
Copy link
Copy Markdown
Collaborator

@pow2clk pow2clk Mar 26, 2025

Choose a reason for hiding this comment

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

Given that you've added the attribute now, it would be pretty easy to avoid the string compares by adding a parameter to it containing an integer representation of the Subobjectkind similar to how HLSLResourceAttr does. The callers of CreateSubobjectStateObjectConfig and friends that call StartSubObjectDecl have the AR_OBJECT enums, which indicate which DXIL:SubobjectKind they need. It would be pretty easy to plumb it down and apply it to the attr.

Comment thread tools/clang/lib/Sema/SemaHLSL.cpp Outdated
static CXXRecordDecl *StartSubobjectDecl(ASTContext &context,
const char *name) {
static CXXRecordDecl *StartSubobjectDecl(ASTContext &context, const char *name,
ArBasicKind kind) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm afraid this isn't quite what I had in mind. If you pass in the correct DXIL::SubobjectKind type into StartsSubObjectDecl even if you have to cast it to a uint from:

  • CreateSubobjectStateObjectConfig
  • CreateSubobjectRootSignature
  • CreateSubobjectSubobjectToExportsSassoc
  • CreateSubobjectRaytracingShaderConfig
  • CreateSubobjectRaytracingShaderConfig1
  • CreateSubobjectTriangleHitGroup
  • CreateSubobjectProceuralPrimitiveHitGroup
    I think each of these know what SubobjectKind they're going to require, so there's no need to let the AR_OBJECT types escape SemaHLSL.cpp.

Comment thread tools/clang/lib/AST/HlslTypes.cpp Outdated
return false;
HLSLSubObjectAttr *Attr = RD->getAttr<HLSLSubObjectAttr>();

switch (Attr->getSubObjKindUint()) {
Copy link
Copy Markdown
Collaborator

@pow2clk pow2clk Mar 26, 2025

Choose a reason for hiding this comment

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

Just to make sure of clarity here. I expected the argument to be the DXIL::SubobjectKind enum rather than the AR_OBJECTs which wouldn't require any conversion here apart from casting it back to the enum type.

Copy link
Copy Markdown
Collaborator

@pow2clk pow2clk left a comment

Choose a reason for hiding this comment

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

Thanks for indulging me!

Comment thread tools/clang/lib/Sema/SemaHLSL.cpp Outdated
Comment thread tools/clang/lib/Sema/SemaHLSL.cpp Outdated
Comment thread tools/clang/lib/Sema/SemaHLSL.cpp Outdated
Comment thread tools/clang/include/clang/Basic/Attr.td Outdated
Comment thread tools/clang/test/HLSLFileCheck/shader_targets/raytracing/subobjects_attr.hlsl Outdated
Copy link
Copy Markdown
Contributor

@tex3d tex3d left a comment

Choose a reason for hiding this comment

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

LGTM!

@bob80905 bob80905 merged commit 5ff9cbc into microsoft:main Mar 27, 2025
12 checks passed
@github-project-automation github-project-automation Bot moved this from New to Done in HLSL Roadmap Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[Feature Request] Add SubObject attribute

3 participants