Create Texture Dimension attribute in HLSL #104239
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-hlsl @llvm/pr-subscribers-clang Author: Joshua Batista (bob80905) ChangesThis PR creates a new attribute in HLSL, "TextureDimension", which specifies the dimension of a specific texture resource. Full diff: https://github.com/llvm/llvm-project/pull/104239.diff 9 Files Affected:
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 10a9d9e899e007..c61ec7e2a15305 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4642,6 +4642,14 @@ def HLSLParamModifier : TypeAttr {
let Args = [DefaultBoolArgument<"MergedSpelling", /*default*/0, /*fake*/1>];
}
+def HLSLTextureDimension : InheritableAttr {
+ let Spellings = [CXX11<"hlsl", "texture_dimension">];
+ let Subjects = SubjectList<[Struct]>;
+ let LangOpts = [HLSL];
+ let Args = [IntArgument<"Dimension">];
+ let Documentation = [InternalOnly];
+}
+
def RandomizeLayout : InheritableAttr {
let Spellings = [GCC<"randomize_layout">];
let Subjects = SubjectList<[Record]>;
diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h
index d60cb2a57d4918..d9d4c8ed6eb00c 100644
--- a/clang/include/clang/Sema/SemaHLSL.h
+++ b/clang/include/clang/Sema/SemaHLSL.h
@@ -59,6 +59,7 @@ class SemaHLSL : public SemaBase {
void handleROVAttr(Decl *D, const ParsedAttr &AL);
void handleResourceClassAttr(Decl *D, const ParsedAttr &AL);
void handleResourceBindingAttr(Decl *D, const ParsedAttr &AL);
+ void handleTextureDimensionAttr(Decl *D, const ParsedAttr &AL);
void handleParamModifierAttr(Decl *D, const ParsedAttr &AL);
bool CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 89a0e391920cc6..6ee9d9cae567da 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -107,7 +107,7 @@ struct BuiltinTypeDeclBuilder {
}
BuiltinTypeDeclBuilder &
- addHandleMember(ResourceClass RC, ResourceKind RK, bool IsROV,
+ addHandleMember(ResourceClass RC, ResourceKind RK, bool IsROV, int TD,
AccessSpecifier Access = AccessSpecifier::AS_private) {
if (Record->isCompleteDefinition())
return *this;
@@ -125,8 +125,12 @@ struct BuiltinTypeDeclBuilder {
HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RK);
Attr *ROVAttr =
IsROV ? HLSLROVAttr::CreateImplicit(Record->getASTContext()) : nullptr;
- addMemberVariable("h", Ty, {ResourceClassAttr, ResourceAttr, ROVAttr},
- Access);
+ Attr *TextureDimensionAttr =
+ HLSLTextureDimensionAttr::CreateImplicit(Record->getASTContext(), TD);
+ addMemberVariable(
+ "h", Ty,
+ {ResourceClassAttr, ResourceAttr, ROVAttr, TextureDimensionAttr},
+ Access);
return *this;
}
@@ -492,9 +496,9 @@ void HLSLExternalSemaSource::defineTrivialHLSLTypes() {
/// Set up common members and attributes for buffer types
static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
ResourceClass RC, ResourceKind RK,
- bool IsROV) {
+ bool IsROV, int TD) {
return BuiltinTypeDeclBuilder(Decl)
- .addHandleMember(RC, RK, IsROV)
+ .addHandleMember(RC, RK, IsROV, TD)
.addDefaultHandleConstructor(S, RC);
}
@@ -505,7 +509,8 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
.Record;
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
- ResourceKind::TypedBuffer, /*IsROV=*/false)
+ ResourceKind::TypedBuffer, /*IsROV=*/false,
+ /*TextureDimension*/ 1)
.addArraySubscriptOperators()
.completeDefinition();
});
@@ -516,7 +521,8 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
.Record;
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
- ResourceKind::TypedBuffer, /*IsROV=*/true)
+ ResourceKind::TypedBuffer, /*IsROV=*/true,
+ /*TextureDimension*/ 1)
.addArraySubscriptOperators()
.completeDefinition();
});
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 3b5e984f4ee773..9e90e16040129e 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6910,6 +6910,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
case ParsedAttr::AT_HLSLParamModifier:
S.HLSL().handleParamModifierAttr(D, AL);
break;
+ case ParsedAttr::AT_HLSLTextureDimension:
+ S.HLSL().handleTextureDimensionAttr(D, AL);
+ break;
case ParsedAttr::AT_AbiTag:
handleAbiTagAttr(S, D, AL);
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index e3e926465e799e..17f56d07f3d188 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -538,6 +538,50 @@ void SemaHLSL::handleParamModifierAttr(Decl *D, const ParsedAttr &AL) {
D->addAttr(NewAttr);
}
+int ConvertStrToTextureDimension(StringRef Str) {
+ // Str should be an integer between 1 and 3
+ unsigned Num;
+ if (Str.getAsInteger(10, Num))
+ return 0;
+ if (Num < 1 || Num > 3)
+ return 0;
+ return Num;
+}
+
+void SemaHLSL::handleTextureDimensionAttr(Decl *D, const ParsedAttr &AL) {
+ Expr *E = AL.getArgAsExpr(0);
+ if (!E) {
+ Diag(AL.getLoc(), diag::err_attribute_argument_type)
+ << AL << AANT_ArgumentConstantExpr;
+ return;
+ }
+
+ std::optional<llvm::APSInt> I = llvm::APSInt(64);
+ I = E->getIntegerConstantExpr(getASTContext());
+
+ int arg0;
+ if (I.has_value())
+ arg0 = I->getZExtValue();
+ else {
+
+ Diag(E->getExprLoc(), diag::err_attribute_argument_type)
+ << AL << AANT_ArgumentIntegerConstant;
+ return;
+ }
+
+ if (arg0 < 1 || arg0 > 3) {
+ Diag(E->getExprLoc(), diag::warn_attribute_type_not_supported)
+ << "TextureDimension" << arg0;
+ return;
+ }
+
+ HLSLTextureDimensionAttr *NewAttr =
+ HLSLTextureDimensionAttr::Create(getASTContext(), arg0, E->getExprLoc());
+
+ if (NewAttr)
+ D->addAttr(NewAttr);
+}
+
namespace {
/// This class implements HLSL availability diagnostics for default
diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
index 1a71556213bb16..a50b4ebbfc3ea7 100644
--- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -83,6 +83,7 @@
// CHECK-NEXT: HIPManaged (SubjectMatchRule_variable)
// CHECK-NEXT: HLSLROV (SubjectMatchRule_record_not_is_union)
// CHECK-NEXT: HLSLResourceClass (SubjectMatchRule_record_not_is_union)
+// CHECK-NEXT: HLSLTextureDimension (SubjectMatchRule_record_not_is_union)
// CHECK-NEXT: Hot (SubjectMatchRule_function)
// CHECK-NEXT: HybridPatchable (SubjectMatchRule_function)
// CHECK-NEXT: IBAction (SubjectMatchRule_objc_method_is_instance)
diff --git a/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl b/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
index 320d1160e761dd..bc9fd656fb205a 100644
--- a/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
+++ b/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
@@ -4,6 +4,7 @@
// 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
+// CHECK: -HLSLTextureDimensionAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit 1
RWBuffer<float> Buffer1;
// CHECK: -ClassTemplateDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> implicit RasterizerOrderedBuffer
@@ -12,4 +13,5 @@ RWBuffer<float> Buffer1;
// CHECK: -HLSLResourceClassAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit UAV
// CHECK: -HLSLResourceAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit TypedBuffer
// CHECK: -HLSLROVAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit
+// CHECK: -HLSLTextureDimensionAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit 1
RasterizerOrderedBuffer<vector<float, 4> > BufferArray3[4] : register(u4, space1);
diff --git a/clang/test/ParserHLSL/hlsl_texture_dimension_attr.hlsl b/clang/test/ParserHLSL/hlsl_texture_dimension_attr.hlsl
new file mode 100644
index 00000000000000..1d6b644e943d76
--- /dev/null
+++ b/clang/test/ParserHLSL/hlsl_texture_dimension_attr.hlsl
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s
+
+
+// CHECK: -HLSLTextureDimensionAttr 0x{{[0-9a-f]+}} <col:34> 1
+struct [[hlsl::texture_dimension(1)]] Eg1 {
+ int i;
+};
+
+Eg1 e1;
diff --git a/clang/test/ParserHLSL/hlsl_texture_dimension_attr_error.hlsl b/clang/test/ParserHLSL/hlsl_texture_dimension_attr_error.hlsl
new file mode 100644
index 00000000000000..1a285ab8d0354d
--- /dev/null
+++ b/clang/test/ParserHLSL/hlsl_texture_dimension_attr_error.hlsl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s -verify
+
+// expected-error@+1{{'texture_dimension' attribute takes one argument}}
+struct [[hlsl::texture_dimension(3, 2)]] Eg1 {
+ int i;
+};
+
+Eg1 e1;
+
+// expected-error@+1{{'texture_dimension' attribute takes one argument}}
+struct [[hlsl::texture_dimension]] Eg2 {
+ int i;
+};
+
+Eg2 e2;
+
+// expected-error@+1{{use of undeclared identifier 'gibberish'}}
+struct [[hlsl::texture_dimension(gibberish)]] Eg3 {
+ int i;
+};
+
+Eg2 e3;
|
|
What is this attribute supposed to mean? I think we need some design here before we create the attribute, as it isn't clear to me that a single integer covers our needs. The texture kinds that need to be representable by this and whatever other attributes we design are 1D, 2D, 3D, Cube, and arrays of 1D, 2D, and Cube. How does a single integer map to these? Is the idea that there will be another attribute for arrays and yet another for the Cube case? I'm not convinced this change makes sense on its own without a plan for how it maps to DXIL and probably also SPIR-V. |
You're right, a single integer won't be able to match to all those dimension kinds. |
|
D3D at least tends to view "buffer" as another type of resource dimension. Wonder if this should be included, rather than having to assume that omission of the attribute means it must be a buffer. |
If we're just going to use an enum we should probably just stick with |
|
|
||
|
|
||
| // CHECK: -HLSLTextureDimensionAttr 0x{{[0-9a-f]+}} <col:34> 1 | ||
| struct [[hlsl::texture_dimension(1)]] Eg1 { |
There was a problem hiding this comment.
Should we limit this attribute for Texture only?
There was a problem hiding this comment.
Yes, buffer resources won't have this attribute.
|
We should get the proposal approved before implementing it. |
This PR creates a new attribute in HLSL, "TextureDimension", which specifies the dimension of a specific texture resource.
This PR is another necessary part of completing #98192
Tests were added that check this attribute gets properly placed in the AST, and for all currently defined resources (which have dimension equal to 1), the attribute was added to these resources with dimension equal to 1.
The attribute is further described here:
llvm/wg-hlsl#42
Addresses this issue:
llvm/wg-hlsl#39