Skip to content

[HLSL] Add concepts for Structured buffers #119643

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Dec 19, 2024
Merged

[HLSL] Add concepts for Structured buffers #119643

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cbcdcd3
add concepts for raw buffers
bob80905 Dec 12, 2024
7d11952
add condition that sizeof >= 1
bob80905 Dec 12, 2024
9cf81d8
update concept AST
bob80905 Dec 12, 2024
140b7e6
rephrase rawbuffer to structured buffer
bob80905 Dec 13, 2024
6fb774a
rename function
bob80905 Dec 13, 2024
48ea30c
add helpful comments
bob80905 Dec 19, 2024
b46c4c0
use logical not rather than bitwise
bob80905 Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang/lib/AST/Type.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5118,7 +5118,7 @@ bool Type::isHLSLIntangibleType() const {

CXXRecordDecl *RD = RT->getAsCXXRecordDecl();
assert(RD != nullptr &&
"all HLSL struct and classes should be CXXRecordDecl");
"all HLSL structs and classes should be CXXRecordDecl");
assert(RD->isCompleteDefinition() && "expecting complete type");
return RD->isHLSLIntangible();
}
Expand Down
91 changes: 78 additions & 13 deletions clang/lib/Sema/HLSLExternalSemaSource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,10 @@ static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
.addDefaultHandleConstructor();
}

// This function is responsible for constructing the constraint expression for
// this concept:
// template<typename T> concept is_typed_resource_element_compatible =
// __is_typed_resource_element_compatible<T>;
static Expr *constructTypedBufferConstraintExpr(Sema &S, SourceLocation NameLoc,
TemplateTypeParmDecl *T) {
ASTContext &Context = S.getASTContext();
Expand All @@ -868,8 +872,58 @@ static Expr *constructTypedBufferConstraintExpr(Sema &S, SourceLocation NameLoc,
return TypedResExpr;
}

static ConceptDecl *constructTypedBufferConceptDecl(Sema &S,
NamespaceDecl *NSD) {
// This function is responsible for constructing the constraint expression for
// this concept:
// template<typename T> concept is_structured_resource_element_compatible =
// !__is_intangible<T> && sizeof(T) >= 1;
static Expr *constructStructuredBufferConstraintExpr(Sema &S,
SourceLocation NameLoc,
TemplateTypeParmDecl *T) {
Comment on lines +879 to +881
Copy link
Contributor

@bogner bogner Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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());

// element types also may not be of 0 size
UnaryExprOrTypeTraitExpr *SizeOfExpr = new (Context) UnaryExprOrTypeTraitExpr(
UETT_SizeOf, TTypeSourceInfo, BoolTy, NameLoc, NameLoc);

// Create a BinaryOperator that checks if the size of the type is not equal to
// 1 Empty structs have a size of 1 in HLSL, so we need to check for that
IntegerLiteral *rhs = IntegerLiteral::Create(
Context, llvm::APInt(Context.getTypeSize(Context.getSizeType()), 1, true),
Context.getSizeType(), NameLoc);

BinaryOperator *SizeGEQOneExpr =
BinaryOperator::Create(Context, SizeOfExpr, rhs, BO_GE, BoolTy, VK_LValue,
OK_Ordinary, NameLoc, FPOptionsOverride());

// Combine the two constraints
BinaryOperator *CombinedExpr = BinaryOperator::Create(
Context, NotIntangibleExpr, SizeGEQOneExpr, BO_LAnd, BoolTy, VK_LValue,
OK_Ordinary, NameLoc, FPOptionsOverride());

return CombinedExpr;
}

static ConceptDecl *constructBufferConceptDecl(Sema &S, NamespaceDecl *NSD,
bool isTypedBuffer) {
ASTContext &Context = S.getASTContext();
DeclContext *DC = NSD->getDeclContext();
SourceLocation DeclLoc = SourceLocation();
Expand All @@ -890,9 +944,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_structured_resource_element_compatible"));
ConstraintExpr = constructStructuredBufferConstraintExpr(S, DeclLoc, T);
}

// Create a ConceptDecl
ConceptDecl *CD =
Expand All @@ -910,8 +973,10 @@ static ConceptDecl *constructTypedBufferConceptDecl(Sema &S,

void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
CXXRecordDecl *Decl;
ConceptDecl *TypedBufferConcept =
constructTypedBufferConceptDecl(*SemaPtr, HLSLNamespace);
ConceptDecl *TypedBufferConcept = constructBufferConceptDecl(
*SemaPtr, HLSLNamespace, /*isTypedBuffer*/ true);
ConceptDecl *StructuredBufferConcept = constructBufferConceptDecl(
*SemaPtr, HLSLNamespace, /*isTypedBuffer*/ false);
Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWBuffer")
.addSimpleTemplateParams({"element_type"}, TypedBufferConcept)
.finalizeForwardDeclaration();
Expand All @@ -926,7 +991,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {

Decl =
BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RasterizerOrderedBuffer")
.addSimpleTemplateParams({"element_type"})
.addSimpleTemplateParams({"element_type"}, StructuredBufferConcept)
.finalizeForwardDeclaration();
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
Expand All @@ -937,7 +1002,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
});

Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "StructuredBuffer")
.addSimpleTemplateParams({"element_type"})
.addSimpleTemplateParams({"element_type"}, StructuredBufferConcept)
.finalizeForwardDeclaration();
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::SRV, ResourceKind::RawBuffer,
Expand All @@ -947,7 +1012,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
});

Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWStructuredBuffer")
.addSimpleTemplateParams({"element_type"})
.addSimpleTemplateParams({"element_type"}, StructuredBufferConcept)
.finalizeForwardDeclaration();
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, ResourceKind::RawBuffer,
Expand All @@ -960,7 +1025,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {

Decl =
BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "AppendStructuredBuffer")
.addSimpleTemplateParams({"element_type"})
.addSimpleTemplateParams({"element_type"}, StructuredBufferConcept)
.finalizeForwardDeclaration();
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, ResourceKind::RawBuffer,
Expand All @@ -971,7 +1036,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {

Decl =
BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "ConsumeStructuredBuffer")
.addSimpleTemplateParams({"element_type"})
.addSimpleTemplateParams({"element_type"}, StructuredBufferConcept)
.finalizeForwardDeclaration();
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, ResourceKind::RawBuffer,
Expand All @@ -982,7 +1047,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {

Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace,
"RasterizerOrderedStructuredBuffer")
.addSimpleTemplateParams({"element_type"})
.addSimpleTemplateParams({"element_type"}, StructuredBufferConcept)
.finalizeForwardDeclaration();
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, ResourceKind::RawBuffer,
Expand Down
16 changes: 16 additions & 0 deletions clang/test/AST/HLSL/StructuredBuffers-AST.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_structured_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

Expand All @@ -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_structured_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
Expand Down
15 changes: 15 additions & 0 deletions clang/test/AST/HLSL/is_structured_resource_element_compatible_concept.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -ast-dump-filter=__is_structured_resource_element_compatible %s | FileCheck %s

// 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
Copy link
Member

@hekota hekota Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@hekota hekota Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@bob80905 bob80905 Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

// 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'
// CHECK: `-BinaryOperator 0x{{[0-9a-f]+}} <<invalid sloc>> 'bool' lvalue '>='
// CHECK: |-UnaryExprOrTypeTraitExpr 0x{{[0-9a-f]+}} <<invalid sloc>> 'bool' sizeof 'element_type'
// CHECK: `-IntegerLiteral 0x{{[0-9a-f]+}} <<invalid sloc>> 'unsigned long' 1


StructuredBuffer<float> Buffer;
16 changes: 14 additions & 2 deletions clang/test/SemaHLSL/BuiltIns/StructuredBuffers.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,25 @@ typedef vector<float, 3> float3;
StructuredBuffer<float3> Buffer;

// expected-error@+2 {{class template 'StructuredBuffer' requires template arguments}}
// expected-note@*:* {{template declaration from hidden source: template <typename element_type> class StructuredBuffer {}}}
// expected-note@*:* {{template declaration from hidden source: template <typename element_type> requires __is_structured_resource_element_compatible<element_type> class StructuredBuffer {}}}
StructuredBuffer BufferErr1;

// expected-error@+2 {{too few template arguments for class template 'StructuredBuffer'}}
// expected-note@*:* {{template declaration from hidden source: template <typename element_type> class StructuredBuffer {}}}
// expected-note@*:* {{template declaration from hidden source: template <typename element_type> requires __is_structured_resource_element_compatible<element_type> class StructuredBuffer {}}}
StructuredBuffer<> BufferErr2;

// test elements of 0 size
// expected-error@+3{{constraints not satisfied for class template 'StructuredBuffer' [with element_type = int[0]]}}
// expected-note@*:*{{because 'int[0]' does not satisfy '__is_structured_resource_element_compatible'}}
// expected-note@*:*{{because 'sizeof(int[0]) >= 1UL' (0 >= 1) evaluated to false}}
StructuredBuffer<int[0]> BufferErr3;

// In C++, empty structs do have a size of 1. So should HLSL.
// The concept will accept empty structs as element types, despite it being unintuitive.
struct Empty {};
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>>'}}
Expand Down
Loading