[HLSL] Add concepts for Structured buffers #119643

bob80905 · 2024-12-12T01:04:43Z

This PR adds concept validation to structured buffers, in the same way that it was done for typed buffers (like RWBuffer) in #116413.
This PR should also be responsible for introducing rejection of 0 size elements for structured buffers.
Fixes #117406

llvmbot · 2024-12-12T01:05:17Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-hlsl

Author: Joshua Batista (bob80905)

Changes

This PR adds concept validation to raw buffers like Structured buffers, in the same way that it was done for typed buffers in #116413.
This PR should also be responsible for testing 0 size elements for raw buffers.
Fixes #117406

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

3 Files Affected:

(modified) clang/lib/Sema/HLSLExternalSemaSource.cpp (+50-13)
(modified) clang/test/AST/HLSL/StructuredBuffers-AST.hlsl (+16)
(added) clang/test/AST/HLSL/is_raw_resource_element_compatible_concept.hlsl (+10)

diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index 79fc2751b73812..e109e8b9abbfa7 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -868,8 +868,34 @@ static Expr *constructTypedBufferConstraintExpr(Sema &S, SourceLocation NameLoc,
   return TypedResExpr;
 }
 
-static ConceptDecl *constructTypedBufferConceptDecl(Sema &S,
-                                                    NamespaceDecl *NSD) {
+static Expr *constructRawBufferConstraintExpr(Sema &S, SourceLocation NameLoc,
+                                              TemplateTypeParmDecl *T) {
+  ASTContext &Context = S.getASTContext();
+
+  // Obtain the QualType for 'bool'
+  QualType BoolTy = Context.BoolTy;
+
+  // Create a QualType that points to this TemplateTypeParmDecl
+  QualType TType = Context.getTypeDeclType(T);
+
+  // Create a TypeSourceInfo for the template type parameter 'T'
+  TypeSourceInfo *TTypeSourceInfo =
+      Context.getTrivialTypeSourceInfo(TType, NameLoc);
+
+  TypeTraitExpr *IsIntangibleExpr =
+      TypeTraitExpr::Create(Context, BoolTy, NameLoc, UTT_IsIntangibleType,
+                            {TTypeSourceInfo}, NameLoc, true);
+
+  // negate IsIntangibleExpr
+  UnaryOperator *NotIntangibleExpr = UnaryOperator::Create(
+      Context, IsIntangibleExpr, UO_Not, BoolTy, VK_LValue, OK_Ordinary,
+      NameLoc, false, FPOptionsOverride());
+
+  return NotIntangibleExpr;
+}
+
+static ConceptDecl *constructTypedBufferConceptDecl(Sema &S, NamespaceDecl *NSD,
+                                                    bool isTypedBuffer) {
   ASTContext &Context = S.getASTContext();
   DeclContext *DC = NSD->getDeclContext();
   SourceLocation DeclLoc = SourceLocation();
@@ -890,9 +916,18 @@ static ConceptDecl *constructTypedBufferConceptDecl(Sema &S,
   TemplateParameterList *ConceptParams = TemplateParameterList::Create(
       Context, DeclLoc, DeclLoc, {T}, DeclLoc, nullptr);
 
-  DeclarationName DeclName = DeclarationName(
-      &Context.Idents.get("__is_typed_resource_element_compatible"));
-  Expr *ConstraintExpr = constructTypedBufferConstraintExpr(S, DeclLoc, T);
+  DeclarationName DeclName;
+  Expr *ConstraintExpr = nullptr;
+
+  if (isTypedBuffer) {
+    DeclName = DeclarationName(
+        &Context.Idents.get("__is_typed_resource_element_compatible"));
+    ConstraintExpr = constructTypedBufferConstraintExpr(S, DeclLoc, T);
+  } else {
+    DeclName = DeclarationName(
+        &Context.Idents.get("__is_raw_resource_element_compatible"));
+    ConstraintExpr = constructRawBufferConstraintExpr(S, DeclLoc, T);
+  }
 
   // Create a ConceptDecl
   ConceptDecl *CD =
@@ -910,8 +945,10 @@ static ConceptDecl *constructTypedBufferConceptDecl(Sema &S,
 
 void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
   CXXRecordDecl *Decl;
-  ConceptDecl *TypedBufferConcept =
-      constructTypedBufferConceptDecl(*SemaPtr, HLSLNamespace);
+  ConceptDecl *TypedBufferConcept = constructTypedBufferConceptDecl(
+      *SemaPtr, HLSLNamespace, /*isTypedBuffer*/ true);
+  ConceptDecl *RawBufferConcept = constructTypedBufferConceptDecl(
+      *SemaPtr, HLSLNamespace, /*isTypedBuffer*/ false);
   Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWBuffer")
              .addSimpleTemplateParams({"element_type"}, TypedBufferConcept)
              .finalizeForwardDeclaration();
@@ -926,7 +963,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
 
   Decl =
       BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RasterizerOrderedBuffer")
-          .addSimpleTemplateParams({"element_type"})
+          .addSimpleTemplateParams({"element_type"}, RawBufferConcept)
           .finalizeForwardDeclaration();
   onCompletion(Decl, [this](CXXRecordDecl *Decl) {
     setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
@@ -937,7 +974,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
   });
 
   Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "StructuredBuffer")
-             .addSimpleTemplateParams({"element_type"})
+             .addSimpleTemplateParams({"element_type"}, RawBufferConcept)
              .finalizeForwardDeclaration();
   onCompletion(Decl, [this](CXXRecordDecl *Decl) {
     setupBufferType(Decl, *SemaPtr, ResourceClass::SRV, ResourceKind::RawBuffer,
@@ -947,7 +984,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
   });
 
   Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWStructuredBuffer")
-             .addSimpleTemplateParams({"element_type"})
+             .addSimpleTemplateParams({"element_type"}, RawBufferConcept)
              .finalizeForwardDeclaration();
   onCompletion(Decl, [this](CXXRecordDecl *Decl) {
     setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, ResourceKind::RawBuffer,
@@ -960,7 +997,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
 
   Decl =
       BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "AppendStructuredBuffer")
-          .addSimpleTemplateParams({"element_type"})
+          .addSimpleTemplateParams({"element_type"}, RawBufferConcept)
           .finalizeForwardDeclaration();
   onCompletion(Decl, [this](CXXRecordDecl *Decl) {
     setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, ResourceKind::RawBuffer,
@@ -971,7 +1008,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
 
   Decl =
       BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "ConsumeStructuredBuffer")
-          .addSimpleTemplateParams({"element_type"})
+          .addSimpleTemplateParams({"element_type"}, RawBufferConcept)
           .finalizeForwardDeclaration();
   onCompletion(Decl, [this](CXXRecordDecl *Decl) {
     setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, ResourceKind::RawBuffer,
@@ -982,7 +1019,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
 
   Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace,
                                 "RasterizerOrderedStructuredBuffer")
-             .addSimpleTemplateParams({"element_type"})
+             .addSimpleTemplateParams({"element_type"}, RawBufferConcept)
              .finalizeForwardDeclaration();
   onCompletion(Decl, [this](CXXRecordDecl *Decl) {
     setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, ResourceKind::RawBuffer,
diff --git a/clang/test/AST/HLSL/StructuredBuffers-AST.hlsl b/clang/test/AST/HLSL/StructuredBuffers-AST.hlsl
index 6cb4379ef5f556..3e71333da08fd8 100644
--- a/clang/test/AST/HLSL/StructuredBuffers-AST.hlsl
+++ b/clang/test/AST/HLSL/StructuredBuffers-AST.hlsl
@@ -50,6 +50,14 @@
 
 // EMPTY: ClassTemplateDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> implicit [[RESOURCE]]
 // EMPTY-NEXT: TemplateTypeParmDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> typename depth 0 index 0 element_type
+// EMPTY-NEXT: ConceptSpecializationExpr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> 'bool' Concept 0x{{[0-9A-Fa-f]+}} '__is_raw_resource_element_compatible'
+// EMPTY-NEXT: ImplicitConceptSpecializationDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc>
+// EMPTY-NEXT: TemplateArgument type 'type-parameter-0-0'
+// EMPTY-NEXT: TemplateTypeParmType 0x{{[0-9A-Fa-f]+}} 'type-parameter-0-0' dependent depth 0 index 0
+// EMPTY-NEXT: TemplateTypeParm 0x{{[0-9A-Fa-f]+}} ''
+// EMPTY-NEXT: TemplateArgument type 'element_type':'type-parameter-0-0'
+// EMPTY-NEXT: TemplateTypeParmType 0x{{[0-9A-Fa-f]+}} 'element_type' dependent depth 0 index 0
+// EMPTY-NEXT: TemplateTypeParm 0x{{[0-9A-Fa-f]+}} 'element_type'
 // EMPTY-NEXT: CXXRecordDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> implicit <undeserialized declarations> class [[RESOURCE]]
 // EMPTY-NEXT: FinalAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit final
 
@@ -64,6 +72,14 @@ RESOURCE<float> Buffer;
 
 // CHECK: ClassTemplateDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> implicit [[RESOURCE]]
 // CHECK-NEXT: TemplateTypeParmDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> typename depth 0 index 0 element_type
+// CHECK-NEXT: ConceptSpecializationExpr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> 'bool' Concept 0x{{[0-9A-Fa-f]+}} '__is_raw_resource_element_compatible'
+// CHECK-NEXT: ImplicitConceptSpecializationDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc>
+// CHECK-NEXT: TemplateArgument type 'type-parameter-0-0'
+// CHECK-NEXT: TemplateTypeParmType 0x{{[0-9A-Fa-f]+}} 'type-parameter-0-0' dependent depth 0 index 0
+// CHECK-NEXT: TemplateTypeParm 0x{{[0-9A-Fa-f]+}} ''
+// CHECK-NEXT: TemplateArgument type 'element_type':'type-parameter-0-0'
+// CHECK-NEXT: TemplateTypeParmType 0x{{[0-9A-Fa-f]+}} 'element_type' dependent depth 0 index 0
+// CHECK-NEXT: TemplateTypeParm 0x{{[0-9A-Fa-f]+}} 'element_type'
 // CHECK-NEXT: CXXRecordDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> implicit class [[RESOURCE]] definition
 
 // CHECK: FinalAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit final
diff --git a/clang/test/AST/HLSL/is_raw_resource_element_compatible_concept.hlsl b/clang/test/AST/HLSL/is_raw_resource_element_compatible_concept.hlsl
new file mode 100644
index 00000000000000..9c0b151739090a
--- /dev/null
+++ b/clang/test/AST/HLSL/is_raw_resource_element_compatible_concept.hlsl
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -ast-dump-filter=__is_raw_resource_element_compatible %s | FileCheck %s
+
+// CHECK: ConceptDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> __is_raw_resource_element_compatible
+// CHECK: |-TemplateTypeParmDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> referenced typename depth 0 index 0 element_type
+// CHECK:  `-UnaryOperator 0x{{[0-9a-f]+}} <<invalid sloc>> 'bool' lvalue prefix '~' cannot overflow
+// CHECK:    `-TypeTraitExpr 0x{{[0-9a-f]+}} <<invalid sloc>> 'bool' __builtin_hlsl_is_intangible
+// CHECK:      `-TemplateTypeParmType 0x{{[0-9a-f]+}} 'element_type' dependent depth 0 index 0
+// CHECK:	     `-TemplateTypeParm 0x{{[0-9a-f]+}} 'element_type'
+
+StructuredBuffer<float> Buffer;

bogner · 2024-12-13T00:31:52Z

This should really be talking about concepts for structured buffers, not raw buffers. At the HLSL language level, there is nothing called a RawBuffer - there is ByteAddressBuffer and various kinds of StructuredBuffer, which are represented lower in the stack as a "raw" buffer. Given that there are no concepts for ByteAddressBuffer (since it isn't a template), these concepts apply specifically to structured buffers, so the naming should reflect that.

hekota · 2024-12-13T23:37:40Z

clang/lib/Sema/HLSLExternalSemaSource.cpp

+}
+
+static ConceptDecl *constructTypedBufferConceptDecl(Sema &S, NamespaceDecl *NSD,
+                                                    bool isTypedBuffer) {


This function should not have "Typed" in the name since it can create both typed and structured buffer concept decls.

bogner · 2024-12-19T18:30:04Z

clang/lib/Sema/HLSLExternalSemaSource.cpp

+static Expr *constructStructuredBufferConstraintExpr(Sema &S,
+                                                     SourceLocation NameLoc,
+                                                     TemplateTypeParmDecl *T) {


It would be nice to have a comment that spells out this expression in C++ so that it's easy to understand what exactly we're constructing here.

hekota · 2024-12-19T20:07:31Z

clang/test/AST/HLSL/is_structured_resource_element_compatible_concept.hlsl

+// CHECK: ConceptDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> __is_structured_resource_element_compatible
+// CHECK: |-TemplateTypeParmDecl 0x{{[0-9a-f]+}} <<invalid sloc>> <invalid sloc> referenced typename depth 0 index 0 element_type
+// CHECK: `-BinaryOperator 0x{{[0-9a-f]+}} <<invalid sloc>> 'bool' lvalue '&&'
+// CHECK:   |-UnaryOperator 0x{{[0-9a-f]+}} <<invalid sloc>> 'bool' lvalue prefix '~' cannot overflow


~ is bitwise NOT, it should be ! logical NOT.

You'll need to use UO_LNot when creating the unary expr.

Nice catch!

hekota

LGTM! Please add [HLSL] to the title.

add concepts for raw buffers

cbcdcd3

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 12, 2024

bob80905 added 2 commits December 12, 2024 14:48

add condition that sizeof >= 1

7d11952

update concept AST

9cf81d8

rephrase rawbuffer to structured buffer

140b7e6

hekota reviewed Dec 13, 2024

View reviewed changes

rename function

6fb774a

bob80905 changed the title ~~Add concepts for raw buffers~~ Add concepts for Structured buffers Dec 16, 2024

bogner approved these changes Dec 19, 2024

View reviewed changes

add helpful comments

48ea30c

hekota reviewed Dec 19, 2024

View reviewed changes

use logical not rather than bitwise

b46c4c0

hekota approved these changes Dec 19, 2024

View reviewed changes

bob80905 changed the title ~~Add concepts for Structured buffers~~ [HLSL] Add concepts for Structured buffers Dec 19, 2024

bob80905 merged commit 6a01ac7 into llvm:main Dec 19, 2024
8 checks passed

bob80905 mentioned this pull request Jan 7, 2025

Validation for resource element types llvm/wg-hlsl#74

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[HLSL] Add concepts for Structured buffers #119643

[HLSL] Add concepts for Structured buffers #119643

Uh oh!

bob80905 commented Dec 12, 2024 •

edited

Loading

Uh oh!

llvmbot commented Dec 12, 2024 •

edited

Loading

Uh oh!

bogner commented Dec 13, 2024

Uh oh!

hekota Dec 13, 2024

Uh oh!

bogner Dec 19, 2024

Uh oh!

hekota Dec 19, 2024

Uh oh!

hekota Dec 19, 2024

Uh oh!

bob80905 Dec 19, 2024

Uh oh!

hekota left a comment

Uh oh!

Uh oh!

Uh oh!

[HLSL] Add concepts for Structured buffers #119643

[HLSL] Add concepts for Structured buffers #119643

Uh oh!

Conversation

bob80905 commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bogner commented Dec 13, 2024

Uh oh!

hekota Dec 13, 2024

Choose a reason for hiding this comment

Uh oh!

bogner Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

hekota Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

hekota Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

bob80905 Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

hekota left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bob80905 commented Dec 12, 2024 •

edited

Loading

llvmbot commented Dec 12, 2024 •

edited

Loading