[HLSL] Define RasterizerOrderedBuffer resource #74897

bogner · 2023-12-08T23:41:23Z

Define HLSL's RasterizerOrderedBuffer resource type through the
external sema source. This doesn't fully work as is, but defining it
allows us to exercise the ROV logic in the DirectX backend from HLSL
rather than having to manually edit metadata, so it's useful for
further testing and development.

Created using spr 1.3.5 [skip ci]

Created using spr 1.3.5

llvmbot · 2023-12-08T23:41:51Z

@llvm/pr-subscribers-clang

Author: Justin Bogner (bogner)

Changes

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

3 Files Affected:

(modified) clang/include/clang/Basic/Attr.td (+2-1)
(modified) clang/lib/Sema/HLSLExternalSemaSource.cpp (+19-7)
(added) clang/test/CodeGenHLSL/builtins/RasterizerOrderedBuffer-annotations.hlsl (+20)

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 121ed203829ce..716dff314f8c1 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4250,7 +4250,8 @@ def HLSLResource : InheritableAttr {
                             "StructuredBuffer", "CBuffer", "Sampler",
                             "TBuffer", "RTAccelerationStructure",
                             "FeedbackTexture2D", "FeedbackTexture2DArray"],
-                           /*opt=*/0, /*fake=*/0, /*isExternalType=*/1>
+                           /*opt=*/0, /*fake=*/0, /*isExternalType=*/1>,
+              DefaultBoolArgument<"isROV", /*default=*/0>
               ];
   let Documentation = [InternalOnly];
 }
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 8ed6480a9f5c9..a9c4c6f67e6bc 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -116,11 +116,11 @@ struct BuiltinTypeDeclBuilder {
   }
 
   BuiltinTypeDeclBuilder &annotateResourceClass(ResourceClass RC,
-                                                ResourceKind RK) {
+                                                ResourceKind RK, bool IsROV) {
     if (Record->isCompleteDefinition())
       return *this;
-    Record->addAttr(
-        HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RC, RK));
+    Record->addAttr(HLSLResourceAttr::CreateImplicit(Record->getASTContext(),
+                                                     RC, RK, IsROV));
     return *this;
   }
 
@@ -478,12 +478,12 @@ void HLSLExternalSemaSource::defineTrivialHLSLTypes() {
 
 /// Set up common members and attributes for buffer types
 static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
-                                              ResourceClass RC,
-                                              ResourceKind RK) {
+                                              ResourceClass RC, ResourceKind RK,
+                                              bool IsROV) {
   return BuiltinTypeDeclBuilder(Decl)
       .addHandleMember()
       .addDefaultHandleConstructor(S, RC)
-      .annotateResourceClass(RC, RK);
+      .annotateResourceClass(RC, RK, IsROV);
 }
 
 void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
@@ -493,10 +493,22 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
              .Record;
   onCompletion(Decl, [this](CXXRecordDecl *Decl) {
     setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
-                    ResourceKind::TypedBuffer)
+                    ResourceKind::TypedBuffer, /*IsROV=*/false)
         .addArraySubscriptOperators()
         .completeDefinition();
   });
+
+  Decl =
+      BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RasterizerOrderedBuffer")
+          .addSimpleTemplateParams({"element_type"})
+          .Record;
+  onCompletion(Decl, [this](CXXRecordDecl *Decl) {
+    setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
+                    ResourceKind::TypedBuffer, /*IsROV=*/true)
+        .addArraySubscriptOperators()
+        .completeDefinition();
+  });
+
 }
 
 void HLSLExternalSemaSource::onCompletion(CXXRecordDecl *Record,
diff --git a/clang/test/CodeGenHLSL/builtins/RasterizerOrderedBuffer-annotations.hlsl b/clang/test/CodeGenHLSL/builtins/RasterizerOrderedBuffer-annotations.hlsl
new file mode 100644
index 0000000000000..38dfba68ba325
--- /dev/null
+++ b/clang/test/CodeGenHLSL/builtins/RasterizerOrderedBuffer-annotations.hlsl
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+RasterizerOrderedBuffer<float> Buffer1;
+RasterizerOrderedBuffer<vector<float, 4> > BufferArray[4];
+
+RasterizerOrderedBuffer<float> Buffer2 : register(u3);
+RasterizerOrderedBuffer<vector<float, 4> > BufferArray2[4] : register(u4);
+
+RasterizerOrderedBuffer<float> Buffer3 : register(u3, space1);
+RasterizerOrderedBuffer<vector<float, 4> > BufferArray3[4] : register(u4, space1);
+
+[numthreads(1,1,1)] void main() {}
+
+// CHECK: !hlsl.uavs = !{![[Single:[0-9]+]], ![[Array:[0-9]+]], ![[SingleAllocated:[0-9]+]], ![[ArrayAllocated:[0-9]+]], ![[SingleSpace:[0-9]+]], ![[ArraySpace:[0-9]+]]}
+// CHECK-DAG: ![[Single]] = !{ptr @"?Buffer1@@3V?$RasterizerOrderedBuffer@M@hlsl@@A", !"RasterizerOrderedBuffer<float>", i32 10, i1 true, i32 -1, i32 0}
+// CHECK-DAG: ![[Array]] = !{ptr @"?BufferArray@@3PAV?$RasterizerOrderedBuffer@T?$__vector@M$03@__clang@@@hlsl@@A", !"RasterizerOrderedBuffer<vector<float, 4> >", i32 10, i1 true, i32 -1, i32 0}
+// CHECK-DAG: ![[SingleAllocated]] = !{ptr @"?Buffer2@@3V?$RasterizerOrderedBuffer@M@hlsl@@A", !"RasterizerOrderedBuffer<float>", i32 10, i1 true, i32 3, i32 0}
+// CHECK-DAG: ![[ArrayAllocated]] = !{ptr @"?BufferArray2@@3PAV?$RasterizerOrderedBuffer@T?$__vector@M$03@__clang@@@hlsl@@A", !"RasterizerOrderedBuffer<vector<float, 4> >", i32 10, i1 true, i32 4, i32 0}
+// CHECK-DAG: ![[SingleSpace]] = !{ptr @"?Buffer3@@3V?$RasterizerOrderedBuffer@M@hlsl@@A", !"RasterizerOrderedBuffer<float>", i32 10, i1 true, i32 3, i32 1}
+// CHECK-DAG: ![[ArraySpace]] = !{ptr @"?BufferArray3@@3PAV?$RasterizerOrderedBuffer@T?$__vector@M$03@__clang@@@hlsl@@A", !"RasterizerOrderedBuffer<vector<float, 4> >", i32 10, i1 true, i32 4, i32 1}

llvmbot · 2023-12-08T23:41:51Z

@llvm/pr-subscribers-hlsl

Author: Justin Bogner (bogner)

Changes

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

3 Files Affected:

(modified) clang/include/clang/Basic/Attr.td (+2-1)
(modified) clang/lib/Sema/HLSLExternalSemaSource.cpp (+19-7)
(added) clang/test/CodeGenHLSL/builtins/RasterizerOrderedBuffer-annotations.hlsl (+20)

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 121ed203829ce..716dff314f8c1 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4250,7 +4250,8 @@ def HLSLResource : InheritableAttr {
                             "StructuredBuffer", "CBuffer", "Sampler",
                             "TBuffer", "RTAccelerationStructure",
                             "FeedbackTexture2D", "FeedbackTexture2DArray"],
-                           /*opt=*/0, /*fake=*/0, /*isExternalType=*/1>
+                           /*opt=*/0, /*fake=*/0, /*isExternalType=*/1>,
+              DefaultBoolArgument<"isROV", /*default=*/0>
               ];
   let Documentation = [InternalOnly];
 }
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 8ed6480a9f5c9..a9c4c6f67e6bc 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -116,11 +116,11 @@ struct BuiltinTypeDeclBuilder {
   }
 
   BuiltinTypeDeclBuilder &annotateResourceClass(ResourceClass RC,
-                                                ResourceKind RK) {
+                                                ResourceKind RK, bool IsROV) {
     if (Record->isCompleteDefinition())
       return *this;
-    Record->addAttr(
-        HLSLResourceAttr::CreateImplicit(Record->getASTContext(), RC, RK));
+    Record->addAttr(HLSLResourceAttr::CreateImplicit(Record->getASTContext(),
+                                                     RC, RK, IsROV));
     return *this;
   }
 
@@ -478,12 +478,12 @@ void HLSLExternalSemaSource::defineTrivialHLSLTypes() {
 
 /// Set up common members and attributes for buffer types
 static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
-                                              ResourceClass RC,
-                                              ResourceKind RK) {
+                                              ResourceClass RC, ResourceKind RK,
+                                              bool IsROV) {
   return BuiltinTypeDeclBuilder(Decl)
       .addHandleMember()
       .addDefaultHandleConstructor(S, RC)
-      .annotateResourceClass(RC, RK);
+      .annotateResourceClass(RC, RK, IsROV);
 }
 
 void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
@@ -493,10 +493,22 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
              .Record;
   onCompletion(Decl, [this](CXXRecordDecl *Decl) {
     setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
-                    ResourceKind::TypedBuffer)
+                    ResourceKind::TypedBuffer, /*IsROV=*/false)
         .addArraySubscriptOperators()
         .completeDefinition();
   });
+
+  Decl =
+      BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RasterizerOrderedBuffer")
+          .addSimpleTemplateParams({"element_type"})
+          .Record;
+  onCompletion(Decl, [this](CXXRecordDecl *Decl) {
+    setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
+                    ResourceKind::TypedBuffer, /*IsROV=*/true)
+        .addArraySubscriptOperators()
+        .completeDefinition();
+  });
+
 }
 
 void HLSLExternalSemaSource::onCompletion(CXXRecordDecl *Record,
diff --git a/clang/test/CodeGenHLSL/builtins/RasterizerOrderedBuffer-annotations.hlsl b/clang/test/CodeGenHLSL/builtins/RasterizerOrderedBuffer-annotations.hlsl
new file mode 100644
index 0000000000000..38dfba68ba325
--- /dev/null
+++ b/clang/test/CodeGenHLSL/builtins/RasterizerOrderedBuffer-annotations.hlsl
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+RasterizerOrderedBuffer<float> Buffer1;
+RasterizerOrderedBuffer<vector<float, 4> > BufferArray[4];
+
+RasterizerOrderedBuffer<float> Buffer2 : register(u3);
+RasterizerOrderedBuffer<vector<float, 4> > BufferArray2[4] : register(u4);
+
+RasterizerOrderedBuffer<float> Buffer3 : register(u3, space1);
+RasterizerOrderedBuffer<vector<float, 4> > BufferArray3[4] : register(u4, space1);
+
+[numthreads(1,1,1)] void main() {}
+
+// CHECK: !hlsl.uavs = !{![[Single:[0-9]+]], ![[Array:[0-9]+]], ![[SingleAllocated:[0-9]+]], ![[ArrayAllocated:[0-9]+]], ![[SingleSpace:[0-9]+]], ![[ArraySpace:[0-9]+]]}
+// CHECK-DAG: ![[Single]] = !{ptr @"?Buffer1@@3V?$RasterizerOrderedBuffer@M@hlsl@@A", !"RasterizerOrderedBuffer<float>", i32 10, i1 true, i32 -1, i32 0}
+// CHECK-DAG: ![[Array]] = !{ptr @"?BufferArray@@3PAV?$RasterizerOrderedBuffer@T?$__vector@M$03@__clang@@@hlsl@@A", !"RasterizerOrderedBuffer<vector<float, 4> >", i32 10, i1 true, i32 -1, i32 0}
+// CHECK-DAG: ![[SingleAllocated]] = !{ptr @"?Buffer2@@3V?$RasterizerOrderedBuffer@M@hlsl@@A", !"RasterizerOrderedBuffer<float>", i32 10, i1 true, i32 3, i32 0}
+// CHECK-DAG: ![[ArrayAllocated]] = !{ptr @"?BufferArray2@@3PAV?$RasterizerOrderedBuffer@T?$__vector@M$03@__clang@@@hlsl@@A", !"RasterizerOrderedBuffer<vector<float, 4> >", i32 10, i1 true, i32 4, i32 0}
+// CHECK-DAG: ![[SingleSpace]] = !{ptr @"?Buffer3@@3V?$RasterizerOrderedBuffer@M@hlsl@@A", !"RasterizerOrderedBuffer<float>", i32 10, i1 true, i32 3, i32 1}
+// CHECK-DAG: ![[ArraySpace]] = !{ptr @"?BufferArray3@@3PAV?$RasterizerOrderedBuffer@T?$__vector@M$03@__clang@@@hlsl@@A", !"RasterizerOrderedBuffer<vector<float, 4> >", i32 10, i1 true, i32 4, i32 1}

github-actions · 2023-12-08T23:43:49Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 88d754f74df5d396ae3f820e1367dc0fdd91f081 2dfc247aefb5614f1863480e31efe8484d9dcb23 -- clang/lib/Sema/HLSLExternalSemaSource.cpp

View the diff from clang-format here.

diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index a9c4c6f67e..1a1febf7a3 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -508,7 +508,6 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
         .addArraySubscriptOperators()
         .completeDefinition();
   });
-
 }
 
 void HLSLExternalSemaSource::onCompletion(CXXRecordDecl *Record,

Pull Request: llvm#74897

dmpots · 2023-12-09T00:30:39Z

This PR seems to be missing a description.

dmpots

With a description and comment in the test it otherwise LGTM.

dmpots · 2023-12-09T00:39:49Z

clang/test/CodeGenHLSL/builtins/RasterizerOrderedBuffer-annotations.hlsl

@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s


I think this is fine for testing this change, but ROVs are not allowed to be used in CS (only PS).

This shader does compile successfully because they buffers are not used, but a use here will cause a validation error

https://godbolt.org/z/zEWr7bdsP

error: validation errors error: RasterizerOrdered objects are only allowed in 5.0+ pixel shaders. 'Buffer1' Validation failed.

Probably worth a comment in the test for this.

That's a fair point. I went ahead and changed the triple to specify a pixel shader to avoid any confusion / future validation issues

Created using spr 1.3.5 [skip ci]

Created using spr 1.3.5

Created using spr 1.3.5-bogner [skip ci]

Created using spr 1.3.5-bogner

bogner added 2 commits December 8, 2023 15:41

[𝘀𝗽𝗿] changes to main this commit is based on

a1eb5b8

Created using spr 1.3.5 [skip ci]

[𝘀𝗽𝗿] initial version

982396f

Created using spr 1.3.5

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Dec 8, 2023

bogner linked an issue Dec 8, 2023 that may be closed by this pull request

[HLSL] Generate external Sema representations of more resource types #70692

Closed

bogner added a commit to bogner/llvm-project that referenced this pull request Dec 9, 2023

[HLSL] Define RasterizerOrderedBuffer resource

4b446f5

Pull Request: llvm#74897

dmpots approved these changes Dec 9, 2023

View reviewed changes

bogner and others added 4 commits December 9, 2023 01:32

[𝘀𝗽𝗿] changes introduced through rebase

88d754f

Created using spr 1.3.5 [skip ci]

rebase and update test to be a pixel shader

2dfc247

Created using spr 1.3.5

[𝘀𝗽𝗿] changes introduced through rebase

feb4eec

Created using spr 1.3.5-bogner [skip ci]

clang-format

dfd8e56

Created using spr 1.3.5-bogner

bogner changed the base branch from users/bogner/spr/main.hlsl-define-rasterizerorderedbuffer-resource to main December 9, 2023 20:33

bogner merged commit 02e02b9 into main Dec 9, 2023
4 of 7 checks passed

bogner deleted the users/bogner/spr/hlsl-define-rasterizerorderedbuffer-resource branch December 9, 2023 20:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HLSL] Define RasterizerOrderedBuffer resource #74897

[HLSL] Define RasterizerOrderedBuffer resource #74897

bogner commented Dec 8, 2023 •

edited

llvmbot commented Dec 8, 2023

llvmbot commented Dec 8, 2023

github-actions bot commented Dec 8, 2023 •

edited

dmpots commented Dec 9, 2023

dmpots left a comment

dmpots Dec 9, 2023

bogner Dec 9, 2023

		@@ -0,0 +1,20 @@
		// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - %s \| FileCheck %s

[HLSL] Define RasterizerOrderedBuffer resource #74897

[HLSL] Define RasterizerOrderedBuffer resource #74897

Conversation

bogner commented Dec 8, 2023 • edited

llvmbot commented Dec 8, 2023

llvmbot commented Dec 8, 2023

github-actions bot commented Dec 8, 2023 • edited

dmpots commented Dec 9, 2023

dmpots left a comment

Choose a reason for hiding this comment

dmpots Dec 9, 2023

Choose a reason for hiding this comment

bogner Dec 9, 2023

Choose a reason for hiding this comment

bogner commented Dec 8, 2023 •

edited

github-actions bot commented Dec 8, 2023 •

edited