-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[HLSL] Add Buffer def to frontend #141086
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-clang @llvm/pr-subscribers-hlsl Author: Ashley Coleman (V-FEXrt) ChangesFixes #138902 Defines the Full diff: https://github.com/llvm/llvm-project/pull/141086.diff 4 Files Affected:
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 38bde7c28e946..726581d131623 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -266,6 +266,19 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
*SemaPtr, HLSLNamespace, /*isTypedBuffer*/ true);
ConceptDecl *StructuredBufferConcept = constructBufferConceptDecl(
*SemaPtr, HLSLNamespace, /*isTypedBuffer*/ false);
+
+ Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "Buffer")
+ .addSimpleTemplateParams({"element_type"}, TypedBufferConcept)
+ .finalizeForwardDeclaration();
+
+ onCompletion(Decl, [this](CXXRecordDecl *Decl) {
+ setupBufferType(Decl, *SemaPtr, ResourceClass::SRV, /*IsROV=*/false,
+ /*RawBuffer=*/false)
+ .addArraySubscriptOperators()
+ .addLoadMethods()
+ .completeDefinition();
+ });
+
Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWBuffer")
.addSimpleTemplateParams({"element_type"}, TypedBufferConcept)
.finalizeForwardDeclaration();
diff --git a/clang/test/SemaHLSL/BuiltIns/Buffers.hlsl b/clang/test/SemaHLSL/BuiltIns/Buffers.hlsl
new file mode 100644
index 0000000000000..477a16a454a9c
--- /dev/null
+++ b/clang/test/SemaHLSL/BuiltIns/Buffers.hlsl
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -Wno-hlsl-implicit-binding -triple dxil-pc-shadermodel6.0-compute -x hlsl -fsyntax-only -verify %s
+
+typedef vector<float, 3> float3;
+typedef vector<double, 2> double2;
+typedef vector<double, 3> double3;
+
+
+// expected-error@+1 {{class template 'Buffer' requires template arguments}}
+Buffer BufferErr1;
+
+// expected-error@+1 {{too few template arguments for class template 'Buffer'}}
+Buffer<> BufferErr2;
+
+// test implicit Buffer concept
+Buffer<int> r1;
+Buffer<float> r2;
+Buffer<float3> Buff;
+Buffer<double2> r4;
+
+// expected-error@+4 {{constraints not satisfied for class template 'Buffer'}}
+// expected-note@*:* {{template declaration from hidden source: template <typename element_type> requires __is_typed_resource_element_compatible<element_type> class Buffer}}
+// expected-note@*:* {{because 'hlsl::Buffer<int>' does not satisfy '__is_typed_resource_element_compatible'}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(hlsl::Buffer<int>)' evaluated to false}}
+Buffer<Buffer<int> > r5;
+
+struct s {
+ int x;
+};
+
+struct Empty {};
+
+template<typename T> struct TemplatedBuffer {
+ T a;
+};
+
+template<typename T> struct TemplatedVector {
+ vector<T, 4> v;
+};
+
+// structs not allowed
+// expected-error@+4 {{constraints not satisfied for class template 'Buffer'}}
+// expected-note@*:* {{template declaration from hidden source: template <typename element_type> requires __is_typed_resource_element_compatible<element_type> class Buffer}}
+// expected-note@*:* {{because 's' does not satisfy '__is_typed_resource_element_compatible'}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(s)' evaluated to false}}
+Buffer<s> r6;
+// expected-error@+3 {{constraints not satisfied for class template 'Buffer'}}
+// expected-note@*:* {{because 'Empty' does not satisfy '__is_typed_resource_element_compatible'}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(Empty)' evaluated to false}}
+Buffer<Empty> r7;
+
+// expected-error@+3 {{constraints not satisfied for class template 'Buffer'}}
+// expected-note@*:* {{because 'TemplatedBuffer<int>' does not satisfy '__is_typed_resource_element_compatible'}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(TemplatedBuffer<int>)' evaluated to false}}
+Buffer<TemplatedBuffer<int> > r8;
+// expected-error@+3 {{constraints not satisfied for class template 'Buffer'}}
+// expected-note@*:* {{because 'TemplatedVector<int>' does not satisfy '__is_typed_resource_element_compatible'}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(TemplatedVector<int>)' evaluated to false}}
+Buffer<TemplatedVector<int> > r9;
+
+// arrays not allowed
+// expected-error@+3 {{constraints not satisfied for class template 'Buffer'}}
+// expected-note@*:* {{because 'half[4]' does not satisfy '__is_typed_resource_element_compatible'}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(half[4])' evaluated to false}}
+Buffer<half[4]> r10;
+
+typedef vector<int, 8> int8;
+// expected-error@+3 {{constraints not satisfied for class template 'Buffer'}}
+// expected-note@*:* {{because 'vector<int, 8>' (vector of 8 'int' values) does not satisfy '__is_typed_resource_element_compatible'}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(vector<int, 8>)' evaluated to false}}
+Buffer<int8> r11;
+
+typedef int MyInt;
+Buffer<MyInt> r12;
+
+// expected-error@+3 {{constraints not satisfied for class template 'Buffer'}}
+// expected-note@*:* {{because 'bool' does not satisfy '__is_typed_resource_element_compatible'}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(bool)' evaluated to false}}
+Buffer<bool> r13;
+
+// expected-error@+3 {{constraints not satisfied for class template 'Buffer'}}
+// expected-note@*:* {{because 'vector<bool, 2>' (vector of 2 'bool' values) does not satisfy '__is_typed_resource_element_compatible'}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(vector<bool, 2>)' evaluated to false}}
+Buffer<vector<bool, 2>> r14;
+
+enum numbers { one, two, three };
+
+// expected-error@+3 {{constraints not satisfied for class template 'Buffer'}}
+// expected-note@*:* {{because 'numbers' does not satisfy '__is_typed_resource_element_compatible'}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(numbers)' evaluated to false}}
+Buffer<numbers> r15;
+
+// expected-error@+3 {{constraints not satisfied for class template 'Buffer'}}
+// expected-note@*:* {{because 'vector<double, 3>' (vector of 3 'double' values) does not satisfy '__is_typed_resource_element_compatible'}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(vector<double, 3>)' evaluated to false}}
+Buffer<double3> r16;
+
+
+struct threeDoubles {
+ double a;
+ double b;
+ double c;
+};
+
+// expected-error@+3 {{constraints not satisfied for class template 'Buffer'}}
+// expected-note@*:* {{because 'threeDoubles' does not satisfy '__is_typed_resource_element_compatible'}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(threeDoubles)' evaluated to false}}
+Buffer<threeDoubles> BufferErr3;
+
+
+[numthreads(1,1,1)]
+void main() {
+ (void)Buff.__handle; // expected-error {{'__handle' is a private member of 'hlsl::Buffer<vector<float, 3>>'}}
+ // expected-note@* {{implicitly declared private here}}
+}
diff --git a/clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl b/clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl
index 91e96b995585f..b4f59393a1768 100644
--- a/clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl
@@ -14,7 +14,7 @@ RWBuffer<> BufferErr2;
// test implicit RWBuffer concept
RWBuffer<int> r1;
RWBuffer<float> r2;
-RWBuffer<float3> Buffer;
+RWBuffer<float3> Buff;
RWBuffer<double2> r4;
// expected-error@+4 {{constraints not satisfied for class template 'RWBuffer'}}
@@ -109,6 +109,6 @@ RWBuffer<threeDoubles> BufferErr3;
[numthreads(1,1,1)]
void main() {
- (void)Buffer.__handle; // expected-error {{'__handle' is a private member of 'hlsl::RWBuffer<vector<float, 3>>'}}
+ (void)Buff.__handle; // expected-error {{'__handle' is a private member of 'hlsl::RWBuffer<vector<float, 3>>'}}
// expected-note@* {{implicitly declared private here}}
}
diff --git a/clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl b/clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl
index 991b04c80ac86..6596f73b25170 100644
--- a/clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl
@@ -2,7 +2,7 @@
typedef vector<float, 3> float3;
-StructuredBuffer<float3> Buffer;
+StructuredBuffer<float3> Buff;
// expected-error@+2 {{class template 'StructuredBuffer' requires template arguments}}
// expected-note@*:* {{template declaration from hidden source: template <typename element_type> requires __is_structured_resource_element_compatible<element_type> class StructuredBuffer {}}}
@@ -26,6 +26,6 @@ StructuredBuffer<Empty> BufferErr4;
[numthreads(1,1,1)]
void main() {
- (void)Buffer.__handle; // expected-error {{'__handle' is a private member of 'hlsl::StructuredBuffer<vector<float, 3>>'}}
+ (void)Buff.__handle; // expected-error {{'__handle' is a private member of 'hlsl::StructuredBuffer<vector<float, 3>>'}}
// expected-note@* {{implicitly declared private here}}
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also add AST tests in clang/test/AST/HLSL/TypedBuffers-AST.hlsl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this/will this handle enforcing that it is a read-only buffer?
@inbelic From Justin, looks like it doesn't :) (and this is a bug) https://hlsl.godbolt.org/z/sqMMffh6K I'm going to spin that off into its own task separate from this one so this doesn't get bogged down |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM given we have a way of tracking making sure it will be read-only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good (noting that we're matching the pre-existing buggy behaviour of StructuredBuffer and will follow up to fix that for both)
Fixes #138902
Defines the
Buffer<>
type in the clang frontend. Lowering from IR->Target Machine is already handled by other code