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

Joshua Batista via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 13 14:54:41 PDT 2024


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

>From c35e4ec3f8ea27eedc0658921d8d9055451acd91 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Wed, 7 Aug 2024 19:34:54 -0700
Subject: [PATCH 1/6] split out ROV from resource attr

---
 clang/include/clang/Basic/Attr.td             | 13 ++++++--
 clang/include/clang/Sema/SemaHLSL.h           |  1 +
 clang/lib/CodeGen/CGHLSLRuntime.cpp           |  5 +--
 clang/lib/Sema/HLSLExternalSemaSource.cpp     |  7 ++--
 clang/lib/Sema/SemaDeclAttr.cpp               |  3 ++
 clang/lib/Sema/SemaHLSL.cpp                   | 23 +++++++++++++
 clang/test/AST/HLSL/RWBuffer-AST.hlsl         |  2 ++
 ...a-attribute-supported-attributes-list.test |  1 +
 clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl   | 32 +++++++++++++++++++
 .../ParserHLSL/hlsl_is_rov_attr_error.hlsl    | 15 +++++++++
 .../hlsl_resource_handle_attrs.hlsl           |  2 ++
 11 files changed, 98 insertions(+), 6 deletions(-)
 create mode 100644 clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl
 create mode 100644 clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl

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;

>From cd985ee6cb2eb900ac5d72b37cf3098d64b5f6fa Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Wed, 7 Aug 2024 19:43:28 -0700
Subject: [PATCH 2/6] add newline

---
 clang/test/AST/HLSL/RWBuffer-AST.hlsl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/test/AST/HLSL/RWBuffer-AST.hlsl b/clang/test/AST/HLSL/RWBuffer-AST.hlsl
index b5f72c3a114f12..70944becb64c58 100644
--- a/clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ b/clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -63,4 +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
+// CHECK-NEXT: HLSLROVAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit

>From 33a61ef420215a746a15426d646f39fcdb118a6a Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Thu, 8 Aug 2024 12:25:36 -0700
Subject: [PATCH 3/6] is_rov now takes no args, adjust tests

---
 clang/include/clang/Basic/Attr.td             |  5 +---
 clang/lib/CodeGen/CGHLSLRuntime.cpp           |  4 +--
 clang/lib/Sema/HLSLExternalSemaSource.cpp     | 10 ++++---
 clang/lib/Sema/SemaDeclAttr.cpp               |  2 +-
 clang/lib/Sema/SemaHLSL.cpp                   | 23 ----------------
 clang/test/AST/HLSL/RWBuffer-AST.hlsl         |  2 --
 ...a-attribute-supported-attributes-list.test |  2 +-
 clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl   | 27 ++-----------------
 .../ParserHLSL/hlsl_is_rov_attr_error.hlsl    |  4 +--
 .../hlsl_resource_handle_attrs.hlsl           | 15 +++++------
 10 files changed, 22 insertions(+), 72 deletions(-)

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 87d0302f3d332c..20867f5c8475d7 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4607,10 +4607,7 @@ def HLSLResource : InheritableAttr {
 def HLSLROV : InheritableAttr {
   let Spellings = [CXX11<"hlsl", "is_rov">];
   let Subjects = SubjectList<[Struct]>;
-  let LangOpts = [HLSL];
-  let Args = [
-	BoolArgument<"IsROV">
-  ];
+  let LangOpts = [HLSL]; 
   let Documentation = [InternalOnly];
 }
 
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 0ae51ca3a8d7c0..3e76bbd28e0c31 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -297,12 +297,12 @@ void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, GlobalVariable *GV) {
     const auto *HLSLResAttr = FD->getAttr<HLSLResourceAttr>();
     const auto *HLSLResClassAttr = FD->getAttr<HLSLResourceClassAttr>();
     const auto *ROVAttr = FD->getAttr<HLSLROVAttr>();
-    if (!HLSLResAttr || !HLSLResClassAttr || !ROVAttr)
+    if (!HLSLResAttr || !HLSLResClassAttr)
       continue;
 
     llvm::hlsl::ResourceClass RC = HLSLResClassAttr->getResourceClass();
     llvm::hlsl::ResourceKind RK = HLSLResAttr->getResourceKind();
-    bool IsROV = ROVAttr->getIsROV();
+    bool IsROV = FD->getAttr<HLSLROVAttr>() ? true : false;
     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 b3285f37a634b5..e5a11c97e9da27 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -116,15 +116,17 @@ struct BuiltinTypeDeclBuilder {
             QualType(TTD->getTypeForDecl(), 0));
     }
     // add handle member
-    llvm::SmallVector<Attr *, 2> Attrs;
     Attr *ResourceClassAttr =
         HLSLResourceClassAttr::CreateImplicit(Record->getASTContext(), RC);
     Attr *ResourceAttr =
         HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RK);
-    Attr *ROVAttr = HLSLROVAttr::CreateImplicit(Record->getASTContext(), IsROV);
+    if (IsROV) {
+      Attr *ROVAttr = HLSLROVAttr::CreateImplicit(Record->getASTContext());
+      addMemberVariable("h", Ty, {ResourceClassAttr, ResourceAttr, ROVAttr},
+                        Access);
+    } else
+      addMemberVariable("h", Ty, {ResourceClassAttr, ResourceAttr}, Access);
 
-    addMemberVariable("h", Ty, {ResourceClassAttr, ResourceAttr, ROVAttr},
-                      Access);
     return *this;
   }
 
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index bb05aea04d0bb9..3b5e984f4ee773 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6902,7 +6902,7 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
     S.HLSL().handleResourceBindingAttr(D, AL);
     break;
   case ParsedAttr::AT_HLSLROV:
-    S.HLSL().handleROVAttr(D, AL);
+    handleSimpleAttribute<HLSLROVAttr>(S, D, AL);
     break;
   case ParsedAttr::AT_HLSLResourceClass:
     S.HLSL().handleResourceClassAttr(D, AL);
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index 3398329e54e770..a9c0c57e88221d 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -437,29 +437,6 @@ 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 70944becb64c58..1f6ef60e121ea5 100644
--- a/clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ b/clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -33,7 +33,6 @@ 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'
@@ -63,4 +62,3 @@ 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
diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
index d5e3d85358aeee..abd84ddc59fb83 100644
--- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -81,8 +81,8 @@
 // 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: HLSLROV (SubjectMatchRule_record_not_is_union)
+// CHECK-NEXT: HLSLResourceClass (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
index ac324419d720b8..29850828ad3bc2 100644
--- a/clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl
+++ b/clang/test/ParserHLSL/hlsl_is_rov_attr.hlsl
@@ -1,32 +1,9 @@
 // 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 {
+// CHECK: -HLSLROVAttr 0x{{[0-9a-f]+}} <col:10, col:16>
+struct [[hlsl::is_rov]] 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
index 9fa0806d86936c..a21fed22220b6d 100644
--- a/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl
+++ b/clang/test/ParserHLSL/hlsl_is_rov_attr_error.hlsl
@@ -1,7 +1,7 @@
 // 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 {
+// expected-error at +1{{'is_rov' attribute takes no arguments}}
+struct [[hlsl::is_rov(3)]] Eg1 {
   int i;  
 };
 
diff --git a/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl b/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
index 59fcf3b77be8b7..320d1160e761dd 100644
--- a/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
+++ b/clang/test/ParserHLSL/hlsl_resource_handle_attrs.hlsl
@@ -1,16 +1,15 @@
 // 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: -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
-RasterizerOrderedBuffer<vector<float, 4> > BufferArray3[4] : register(u4, space1);
+RWBuffer<float> Buffer1;
 
-// 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: -ClassTemplateDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> implicit RasterizerOrderedBuffer
+// CHECK: -CXXRecordDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> implicit class RasterizerOrderedBuffer 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
 // CHECK: -HLSLROVAttr 0x{{[0-9a-f]+}} <<invalid sloc>> Implicit
-RWBuffer<float> Buffer1;
+RasterizerOrderedBuffer<vector<float, 4> > BufferArray3[4] : register(u4, space1);

>From bb5408e0a3d83b837b04ca07fb0efc3bd2e792ae Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Mon, 12 Aug 2024 10:41:03 -0700
Subject: [PATCH 4/6] use hasAttr

---
 clang/lib/CodeGen/CGHLSLRuntime.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 3e76bbd28e0c31..c26606bd1947da 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -302,7 +302,7 @@ void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, GlobalVariable *GV) {
 
     llvm::hlsl::ResourceClass RC = HLSLResClassAttr->getResourceClass();
     llvm::hlsl::ResourceKind RK = HLSLResAttr->getResourceKind();
-    bool IsROV = FD->getAttr<HLSLROVAttr>() ? true : false;
+    bool IsROV = FD->hasAttr<HLSLROVAttr>();
     llvm::hlsl::ElementType ET = calculateElementType(CGM.getContext(), Ty);
 
     BufferResBinding Binding(D->getAttr<HLSLResourceBindingAttr>());

>From 3884fc3ebbdb726464e707a737662386bc4ded59 Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Tue, 13 Aug 2024 13:35:48 -0700
Subject: [PATCH 5/6] remove unused var, simplify code

---
 clang/lib/CodeGen/CGHLSLRuntime.cpp       |  1 -
 clang/lib/Sema/HLSLExternalSemaSource.cpp | 10 ++++------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index c26606bd1947da..d825b5c53c7045 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -296,7 +296,6 @@ 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>();
-    const auto *ROVAttr = FD->getAttr<HLSLROVAttr>();
     if (!HLSLResAttr || !HLSLResClassAttr)
       continue;
 
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index e5a11c97e9da27..70399f29b258af 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -120,12 +120,10 @@ struct BuiltinTypeDeclBuilder {
         HLSLResourceClassAttr::CreateImplicit(Record->getASTContext(), RC);
     Attr *ResourceAttr =
         HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RK);
-    if (IsROV) {
-      Attr *ROVAttr = HLSLROVAttr::CreateImplicit(Record->getASTContext());
-      addMemberVariable("h", Ty, {ResourceClassAttr, ResourceAttr, ROVAttr},
-                        Access);
-    } else
-      addMemberVariable("h", Ty, {ResourceClassAttr, ResourceAttr}, Access);
+    addMemberVariable("h", Ty, {ResourceClassAttr, ResourceAttr}, Access);
+    if (IsROV)
+      Fields["h"]->addAttr(
+          HLSLROVAttr::CreateImplicit(Record->getASTContext()));
 
     return *this;
   }

>From ea707d89d6af80cf1ebf7cd81b74068a90411e5e Mon Sep 17 00:00:00 2001
From: Joshua Batista <jbatista at microsoft.com>
Date: Tue, 13 Aug 2024 14:54:26 -0700
Subject: [PATCH 6/6] unify attr addition code

---
 clang/lib/Sema/HLSLExternalSemaSource.cpp | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 70399f29b258af..89a0e391920cc6 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -96,8 +96,11 @@ struct BuiltinTypeDeclBuilder {
         nullptr, false, InClassInitStyle::ICIS_NoInit);
     Field->setAccess(Access);
     Field->setImplicit(true);
-    for (Attr *A : Attrs)
-      Field->addAttr(A);
+    for (Attr *A : Attrs) {
+      if (A)
+        Field->addAttr(A);
+    }
+
     Record->addDecl(Field);
     Fields[Name] = Field;
     return *this;
@@ -120,10 +123,10 @@ struct BuiltinTypeDeclBuilder {
         HLSLResourceClassAttr::CreateImplicit(Record->getASTContext(), RC);
     Attr *ResourceAttr =
         HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RK);
-    addMemberVariable("h", Ty, {ResourceClassAttr, ResourceAttr}, Access);
-    if (IsROV)
-      Fields["h"]->addAttr(
-          HLSLROVAttr::CreateImplicit(Record->getASTContext()));
+    Attr *ROVAttr =
+        IsROV ? HLSLROVAttr::CreateImplicit(Record->getASTContext()) : nullptr;
+    addMemberVariable("h", Ty, {ResourceClassAttr, ResourceAttr, ROVAttr},
+                      Access);
 
     return *this;
   }



More information about the cfe-commits mailing list