[clang] [HLSL] Split out the ROV attribute from the resource attribute, make it a new spellable attribute. (PR #102414)

via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 7 19:43:26 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang-codegen

Author: Joshua Batista (bob80905)

<details>
<summary>Changes</summary>

Much like #<!-- -->98193, this PR takes some more data out of the resource attribute, specifically the ROV data.
This PR introduces a new attribute called HLSLROVAttr, which contains data on whether or not the decl the attribute applies to is an ROV. Tests were added to ensure the attribute is found on the AST.
This attribute may take any boolean condition as an argument. If the condition is true, then the object the attribute applies to "is" an ROV.
Fixes #https://github.com/llvm/llvm-project/issues/102392

---
Full diff: https://github.com/llvm/llvm-project/pull/102414.diff


11 Files Affected:

- (modified) clang/include/clang/Basic/Attr.td (+11-2) 
- (modified) clang/include/clang/Sema/SemaHLSL.h (+1) 
- (modified) clang/lib/CodeGen/CGHLSLRuntime.cpp (+3-2) 
- (modified) clang/lib/Sema/HLSLExternalSemaSource.cpp (+5-2) 
- (modified) clang/lib/Sema/SemaDeclAttr.cpp (+3) 
- (modified) clang/lib/Sema/SemaHLSL.cpp (+23) 
- (modified) clang/test/AST/HLSL/RWBuffer-AST.hlsl (+2) 
- (modified) clang/test/Misc/pragma-attribute-supported-attributes-list.test (+1) 
- (added) clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl (+32) 
- (added) clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl (+15) 
- (modified) clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl (+2) 


``````````diff
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 8ac2079099c854..87d0302f3d332c 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4599,8 +4599,17 @@ def HLSLResource : InheritableAttr {
           "CBuffer", "Sampler", "TBuffer", "RTAccelerationStructure",
           "FeedbackTexture2D", "FeedbackTexture2DArray"
         ],
-        /*opt=*/0, /*fake=*/0, /*isExternalType=*/1, /*isCovered=*/0>,
-    DefaultBoolArgument<"isROV", /*default=*/0>
+        /*opt=*/0, /*fake=*/0, /*isExternalType=*/1, /*isCovered=*/0>
+  ];
+  let Documentation = [InternalOnly];
+}
+
+def HLSLROV : InheritableAttr {
+  let Spellings = [CXX11<"hlsl", "is_rov">];
+  let Subjects = SubjectList<[Struct]>;
+  let LangOpts = [HLSL];
+  let Args = [
+	BoolArgument<"IsROV">
   ];
   let Documentation = [InternalOnly];
 }
diff --git a/clang/include/clang/Sema/SemaHLSL.h b/clang/include/clang/Sema/SemaHLSL.h
index 2ddbee67c414bb..d60cb2a57d4918 100644
--- a/clang/include/clang/Sema/SemaHLSL.h
+++ b/clang/include/clang/Sema/SemaHLSL.h
@@ -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 handleROVAttr(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);
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index a2c3e76f77b7c7..0ae51ca3a8d7c0 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -296,12 +296,13 @@ void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, GlobalVariable *GV) {
   for (auto *FD : RD->fields()) {
     const auto *HLSLResAttr = FD->getAttr<HLSLResourceAttr>();
     const auto *HLSLResClassAttr = FD->getAttr<HLSLResourceClassAttr>();
-    if (!HLSLResAttr || !HLSLResClassAttr)
+    const auto *ROVAttr = FD->getAttr<HLSLROVAttr>();
+    if (!HLSLResAttr || !HLSLResClassAttr || !ROVAttr)
       continue;
 
     llvm::hlsl::ResourceClass RC = HLSLResClassAttr->getResourceClass();
     llvm::hlsl::ResourceKind RK = HLSLResAttr->getResourceKind();
-    bool IsROV = HLSLResAttr->getIsROV();
+    bool IsROV = ROVAttr->getIsROV();
     llvm::hlsl::ElementType ET = calculateElementType(CGM.getContext(), Ty);
 
     BufferResBinding Binding(D->getAttr<HLSLResourceBindingAttr>());
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 6ee90d15d7a6d1..b3285f37a634b5 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -120,8 +120,11 @@ struct BuiltinTypeDeclBuilder {
     Attr *ResourceClassAttr =
         HLSLResourceClassAttr::CreateImplicit(Record->getASTContext(), RC);
     Attr *ResourceAttr =
-        HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RK, IsROV);
-    addMemberVariable("h", Ty, {ResourceClassAttr, ResourceAttr}, Access);
+        HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RK);
+    Attr *ROVAttr = HLSLROVAttr::CreateImplicit(Record->getASTContext(), IsROV);
+
+    addMemberVariable("h", Ty, {ResourceClassAttr, ResourceAttr, ROVAttr},
+                      Access);
     return *this;
   }
 
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index bcb1424825df00..bb05aea04d0bb9 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6901,6 +6901,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
   case ParsedAttr::AT_HLSLResourceBinding:
     S.HLSL().handleResourceBindingAttr(D, AL);
     break;
+  case ParsedAttr::AT_HLSLROV:
+    S.HLSL().handleROVAttr(D, AL);
+    break;
   case ParsedAttr::AT_HLSLResourceClass:
     S.HLSL().handleResourceClassAttr(D, AL);
     break;
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index a9c0c57e88221d..3398329e54e770 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -437,6 +437,29 @@ void SemaHLSL::handleShaderAttr(Decl *D, const ParsedAttr &AL) {
     D->addAttr(NewAttr);
 }
 
+void SemaHLSL::handleROVAttr(Decl *D, const ParsedAttr &AL) {
+  if (!AL.isArgExpr(0)) {
+    Diag(AL.getLoc(), diag::err_attribute_argument_type)
+        << AL << AANT_ArgumentIntOrBool;
+    return;
+  }
+
+  clang::Expr *E = AL.getArgAsExpr(0);
+  bool isROV = false;
+  bool isBooleanCondition =
+      E->EvaluateAsBooleanCondition(isROV, getASTContext());
+  SourceLocation ArgLoc = E->getExprLoc();
+  SourceRange ArgRange = E->getSourceRange();
+
+  // Validate.
+  if (!isBooleanCondition) {
+    Diag(ArgLoc, diag::warn_attribute_type_not_supported) << "ROV" << ArgRange;
+    return;
+  }
+
+  D->addAttr(HLSLROVAttr::Create(getASTContext(), isROV, ArgLoc));
+}
+
 void SemaHLSL::handleResourceClassAttr(Decl *D, const ParsedAttr &AL) {
   if (!AL.isArgIdent(0)) {
     Diag(AL.getLoc(), diag::err_attribute_argument_type)
diff --git a/clang/test/AST/HLSL/RWBuffer-AST.hlsl b/clang/test/AST/HLSL/RWBuffer-AST.hlsl
index 1f6ef60e121ea5..b5f72c3a114f12 100644
--- a/clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ b/clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -33,6 +33,7 @@ RWBuffer<float> Buffer;
 // 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: HLSLROVAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit
 
 // 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'
@@ -62,3 +63,4 @@ RWBuffer<float> Buffer;
 // 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: HLSLROVAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit
\ No newline at end of file
diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
index 0f7dcab7c4248d..d5e3d85358aeee 100644
--- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -82,6 +82,7 @@
 // CHECK-NEXT: GNUInline (SubjectMatchRule_function)
 // CHECK-NEXT: HIPManaged (SubjectMatchRule_variable)
 // CHECK-NEXT: HLSLResourceClass (SubjectMatchRule_record_not_is_union)
+// CHECK-NEXT: HLSLROV (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_is_rov_attr.hlsl b/clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl
new file mode 100644
index 00000000000000..ac324419d720b8
--- /dev/null
+++ b/clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s
+
+
+// CHECK: -HLSLROVAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit
+struct [[hlsl::is_rov(true)]] Eg1 {
+  int i;  
+};
+
+Eg1 e1;
+
+// CHECK: -CXXRecordDecl 0x{{[0-9a-f]+}} <line:13:1, line:15:1> line:13:32 referenced struct Eg2 definition
+// CHECK: -HLSLROVAttr 0x{{[0-9a-f]+}} <col:23>
+struct [[hlsl::is_rov(false)]] Eg2 {
+  int i;
+};
+Eg2 e2;
+
+// CHECK: -CXXRecordDecl 0x{{[0-9a-f]+}} <line:20:1, line:22:1> line:20:32 referenced struct Eg3 definition
+// CHECK: -HLSLROVAttr 0x{{[0-9a-f]+}} <col:23>
+struct [[hlsl::is_rov(false)]] Eg3 {
+  int i;
+}; 
+Eg3 e3;
+
+// CHECK: -CXXRecordDecl 0x{{[0-9a-f]+}} <line:27:1, line:29:1> line:27:32 referenced struct Eg4 definition
+// CHECK: -HLSLROVAttr 0x{{[0-9a-f]+}} <col:23>
+struct [[hlsl::is_rov(false)]] Eg4 {
+  int i;
+};
+Eg4 e4;
+
+RWBuffer<int> In : register(u1);
diff --git a/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl b/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl
new file mode 100644
index 00000000000000..9fa0806d86936c
--- /dev/null
+++ b/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s -verify
+
+// expected-error at +1{{'is_rov' attribute takes one argument}}
+struct [[hlsl::is_rov()]] Eg1 {
+  int i;  
+};
+
+Eg1 e1;
+
+// expected-error at +1{{use of undeclared identifier 'gibberish'}}
+struct [[hlsl::is_rov(gibberish)]] Eg2 {
+  int i;  
+};
+
+Eg2 e2;
diff --git a/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl b/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
index 6b7bcbc35b8f89..59fcf3b77be8b7 100644
--- a/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
+++ b/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
@@ -5,10 +5,12 @@
 // 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
+// CHECK: -HLSLROVAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit
 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
+// CHECK: -HLSLROVAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit
 RWBuffer<float> Buffer1;

``````````

</details>


https://github.com/llvm/llvm-project/pull/102414


More information about the cfe-commits mailing list