Skip to content

Commit 1a0de96

Browse files
hekotahekota
authored
[HLSL] Constant buffer layout struct update (#124840)
Updates layout struct to be `struct` with public fields instead of `class` with private fields and chang the name prefix to `__cblayout_`. Also adds Packed attribute to prevent struct padding and filters additional types from the layout that were missed previously (arrays of resources and groupshared declarations).
1 parent 2d66ab5 commit 1a0de96

File tree

5 files changed

+124
-100
lines changed

5 files changed

+124
-100
lines changed

‎clang/lib/Sema/SemaHLSL.cpp

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,11 @@ static bool isZeroSizedArray(const ConstantArrayType *CAT) {
269269
return CAT != nullptr;
270270
}
271271

272-
// Returns true if the record type is an HLSL resource class
273-
static bool isResourceRecordType(const Type *Ty) {
272+
// Returns true if the record type is an HLSL resource class or an array of
273+
// resource classes
274+
static bool isResourceRecordTypeOrArrayOf(const Type *Ty) {
275+
while (const ConstantArrayType *CAT = dyn_cast<ConstantArrayType>(Ty))
276+
Ty = CAT->getArrayElementTypeNoTypeQual();
274277
return HLSLAttributedResourceType::findHandleTypeOnResource(Ty) != nullptr;
275278
}
276279

@@ -279,15 +282,15 @@ static bool isResourceRecordType(const Type *Ty) {
279282
// array, or a builtin intangible type. Returns false it is a valid leaf element
280283
// type or if it is a record type that needs to be inspected further.
281284
static bool isInvalidConstantBufferLeafElementType(const Type *Ty) {
282-
if (Ty->isRecordType()) {
283-
if (isResourceRecordType(Ty) || Ty->getAsCXXRecordDecl()->isEmpty())
284-
return true;
285-
return false;
286-
}
285+
Ty = Ty->getUnqualifiedDesugaredType();
286+
if (isResourceRecordTypeOrArrayOf(Ty))
287+
return true;
288+
if (Ty->isRecordType())
289+
return Ty->getAsCXXRecordDecl()->isEmpty();
287290
if (Ty->isConstantArrayType() &&
288291
isZeroSizedArray(cast<ConstantArrayType>(Ty)))
289292
return true;
290-
if (Ty->isHLSLBuiltinIntangibleType())
293+
if (Ty->isHLSLBuiltinIntangibleType() || Ty->isHLSLAttributedResourceType())
291294
return true;
292295
return false;
293296
}
@@ -339,7 +342,7 @@ static IdentifierInfo *getHostLayoutStructName(Sema &S, NamedDecl *BaseDecl,
339342
ASTContext &AST = S.getASTContext();
340343

341344
IdentifierInfo *NameBaseII = BaseDecl->getIdentifier();
342-
llvm::SmallString<64> Name("__layout_");
345+
llvm::SmallString<64> Name("__cblayout_");
343346
if (NameBaseII) {
344347
Name.append(NameBaseII->getName());
345348
} else {
@@ -393,7 +396,7 @@ static FieldDecl *createFieldForHostLayoutStruct(Sema &S, const Type *Ty,
393396
auto *Field = FieldDecl::Create(AST, LayoutStruct, SourceLocation(),
394397
SourceLocation(), II, QT, TSI, nullptr, false,
395398
InClassInitStyle::ICIS_NoInit);
396-
Field->setAccess(AccessSpecifier::AS_private);
399+
Field->setAccess(AccessSpecifier::AS_public);
397400
return Field;
398401
}
399402

@@ -417,9 +420,11 @@ static CXXRecordDecl *createHostLayoutStruct(Sema &S,
417420
if (CXXRecordDecl *RD = findRecordDeclInContext(II, DC))
418421
return RD;
419422

420-
CXXRecordDecl *LS = CXXRecordDecl::Create(
421-
AST, TagDecl::TagKind::Class, DC, SourceLocation(), SourceLocation(), II);
423+
CXXRecordDecl *LS =
424+
CXXRecordDecl::Create(AST, TagDecl::TagKind::Struct, DC, SourceLocation(),
425+
SourceLocation(), II);
422426
LS->setImplicit(true);
427+
LS->addAttr(PackedAttr::CreateImplicit(AST));
423428
LS->startDefinition();
424429

425430
// copy base struct, create HLSL Buffer compatible version if needed
@@ -461,25 +466,27 @@ static CXXRecordDecl *createHostLayoutStruct(Sema &S,
461466

462467
// Creates host layout struct for HLSL Buffer. The struct will include only
463468
// fields of types that are allowed in HLSL buffer and it will filter out:
464-
// - static variable declarations
469+
// - static or groupshared variable declarations
465470
// - resource classes
466471
// - empty structs
467472
// - zero-sized arrays
468473
// - non-variable declarations
469-
// The layour struct will be added to the HLSLBufferDecl declarations.
474+
// The layout struct will be added to the HLSLBufferDecl declarations.
470475
void createHostLayoutStructForBuffer(Sema &S, HLSLBufferDecl *BufDecl) {
471476
ASTContext &AST = S.getASTContext();
472477
IdentifierInfo *II = getHostLayoutStructName(S, BufDecl, true);
473478

474479
CXXRecordDecl *LS =
475-
CXXRecordDecl::Create(AST, TagDecl::TagKind::Class, BufDecl,
480+
CXXRecordDecl::Create(AST, TagDecl::TagKind::Struct, BufDecl,
476481
SourceLocation(), SourceLocation(), II);
482+
LS->addAttr(PackedAttr::CreateImplicit(AST));
477483
LS->setImplicit(true);
478484
LS->startDefinition();
479485

480486
for (Decl *D : BufDecl->decls()) {
481487
VarDecl *VD = dyn_cast<VarDecl>(D);
482-
if (!VD || VD->getStorageClass() == SC_Static)
488+
if (!VD || VD->getStorageClass() == SC_Static ||
489+
VD->getType().getAddressSpace() == LangAS::hlsl_groupshared)
483490
continue;
484491
const Type *Ty = VD->getType()->getUnqualifiedDesugaredType();
485492
if (FieldDecl *FD =

‎clang/test/AST/HLSL/ast-dump-comment-cbuffer.hlsl

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,3 @@ cbuffer A {
2727
// AST-NEXT: TextComment {{.*}} Text=" CBuffer decl."
2828
// AST-NEXT: VarDecl {{.*}} a 'hlsl_constant float'
2929
// AST-NEXT: VarDecl {{.*}} b 'hlsl_constant int'
30-
// AST-NEXT: CXXRecordDecl {{.*}} implicit class __layout_A definition
31-
// AST: FieldDecl {{.*}} a 'float'
32-
// AST-NEXT: FieldDecl {{.*}} b 'int'

‎clang/test/AST/HLSL/cbuffer.hlsl

Lines changed: 84 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -48,94 +48,108 @@ struct TwoFloats {
4848
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
4949
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
5050
cbuffer CB {
51-
// CHECK: VarDecl {{.*}} col:9 used a1 'hlsl_constant float'
51+
// CHECK: VarDecl {{.*}} used a1 'hlsl_constant float'
5252
float a1;
53-
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB definition
54-
// CHECK: FieldDecl {{.*}} a1 'float'
53+
// CHECK: CXXRecordDecl {{.*}} implicit referenced struct __cblayout_CB definition
54+
// CHECK: PackedAttr
55+
// CHECK-NEXT: FieldDecl {{.*}} a1 'float'
5556
}
56-
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_CB), "");
57+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __cblayout_CB), "");
5758

5859
// Check that buffer layout struct does not include resources or empty types
59-
// CHECK: HLSLBufferDecl {{.*}} line:62:9 cbuffer CB
60+
// CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 3]]:9 cbuffer CB
6061
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
6162
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
6263
cbuffer CB {
63-
// CHECK: VarDecl {{.*}} col:9 used a2 'hlsl_constant float'
64+
// CHECK: VarDecl {{.*}} used a2 'hlsl_constant float'
6465
float a2;
65-
// CHECK: VarDecl {{.*}} col:19 b2 'RWBuffer<float>':'hlsl::RWBuffer<float>'
66+
// CHECK: VarDecl {{.*}} b2 'RWBuffer<float>':'hlsl::RWBuffer<float>'
6667
RWBuffer<float> b2;
67-
// CHECK: VarDecl {{.*}} col:15 c2 'EmptyStruct'
68+
// CHECK: VarDecl {{.*}} c2 'EmptyStruct'
6869
EmptyStruct c2;
69-
// CHECK: VarDecl {{.*}} col:9 d2 'float[0]'
70+
// CHECK: VarDecl {{.*}} d2 'float[0]'
7071
float d2[0];
71-
// CHECK: VarDecl {{.*}} col:9 e2 'hlsl_constant float'
72+
// CHECK: VarDecl {{.*}} f2 'RWBuffer<float>[2]'
73+
RWBuffer<float> f2[2];
74+
// CHECK: VarDecl {{.*}} g2 'groupshared float'
75+
groupshared float g2;
76+
// CHECK: VarDecl {{.*}} h2 '__hlsl_resource_t'
77+
__hlsl_resource_t h2;
78+
// CHECK: VarDecl {{.*}} e2 'hlsl_constant float'
7279
float e2;
73-
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_1 definition
74-
// CHECK: FieldDecl {{.*}} a2 'float'
80+
// CHECK: CXXRecordDecl {{.*}} implicit referenced struct __cblayout_CB_1 definition
81+
// CHECK: PackedAttr
82+
// CHECK-NEXT: FieldDecl {{.*}} a2 'float'
7583
// CHECK-NEXT: FieldDecl {{.*}} e2 'float'
7684
}
77-
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layout_CB_1), "");
85+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __cblayout_CB_1), "");
7886

7987
// Check that layout struct is created for B and the empty struct C is removed
80-
// CHECK: HLSLBufferDecl {{.*}} line:83:9 cbuffer CB
88+
// CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 3]]:9 cbuffer CB
8189
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
8290
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
8391
cbuffer CB {
84-
// CHECK: VarDecl {{.*}} col:5 used s1 'hlsl_constant A'
92+
// CHECK: VarDecl {{.*}} used s1 'hlsl_constant A'
8593
A s1;
86-
// CHECK: VarDecl {{.*}} col:5 s2 'hlsl_constant B'
94+
// CHECK: VarDecl {{.*}} s2 'hlsl_constant B'
8795
B s2;
88-
// CHECK: VarDecl {{.*}} col:12 s3 'CTypedef':'C'
96+
// CHECK: VarDecl {{.*}} s3 'CTypedef':'C'
8997
CTypedef s3;
90-
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_2 definition
91-
// CHECK: FieldDecl {{.*}} s1 'A'
92-
// CHECK: FieldDecl {{.*}} s2 '__layout_B'
98+
// CHECK: CXXRecordDecl {{.*}} implicit referenced struct __cblayout_CB_2 definition
99+
// CHECK: PackedAttr
100+
// CHECK-NEXT: FieldDecl {{.*}} s1 'A'
101+
// CHECK-NEXT: FieldDecl {{.*}} s2 '__cblayout_B'
93102
}
94-
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_B definition
95-
// CHECK: FieldDecl {{.*}} a 'float'
103+
// CHECK: CXXRecordDecl {{.*}} implicit referenced struct __cblayout_B definition
104+
// CHECK: PackedAttr
105+
// CHECK-NEXT: FieldDecl {{.*}} a 'float'
96106

97-
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_B), "");
98-
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layout_CB_2), "");
107+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __cblayout_B), "");
108+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __cblayout_CB_2), "");
99109

100110
// check that layout struct is created for D because of its base struct
101-
// CHECK: HLSLBufferDecl {{.*}} line:104:9 cbuffer CB
111+
// CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 3]]:9 cbuffer CB
102112
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
103113
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
104114
cbuffer CB {
105115
// CHECK: VarDecl {{.*}} s4 'hlsl_constant D'
106116
D s4;
107-
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_3 definition
108-
// CHECK: FieldDecl {{.*}} s4 '__layout_D'
117+
// CHECK: CXXRecordDecl {{.*}} implicit referenced struct __cblayout_CB_3 definition
118+
// CHECK: PackedAttr
119+
// CHECK-NEXT: FieldDecl {{.*}} s4 '__cblayout_D'
109120
}
110-
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_D definition
111-
// CHECK: public '__layout_B'
112-
// CHECK: FieldDecl {{.*}} b 'float'
113-
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layout_D), "");
114-
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layout_CB_3), "");
121+
// CHECK: CXXRecordDecl {{.*}} implicit referenced struct __cblayout_D definition
122+
// CHECK: public '__cblayout_B'
123+
// CHECK: PackedAttr
124+
// CHECK-NEXT: FieldDecl {{.*}} b 'float'
125+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __cblayout_D), "");
126+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __cblayout_CB_3), "");
115127

116128
// check that layout struct is created for E because because its base struct
117129
// is empty and should be eliminated, and BTypedef should reuse the previously
118-
// defined '__layout_B'
119-
// CHECK: HLSLBufferDecl {{.*}} line:122:9 cbuffer CB
130+
// defined '__cblayout_B'
131+
// CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 3]]:9 cbuffer CB
120132
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
121133
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
122134
cbuffer CB {
123135
// CHECK: VarDecl {{.*}} s5 'hlsl_constant E'
124136
E s5;
125137
// CHECK: VarDecl {{.*}} s6 'hlsl_constant BTypedef':'hlsl_constant B'
126138
BTypedef s6;
127-
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_4 definition
128-
// CHECK: FieldDecl {{.*}} s5 '__layout_E'
129-
// CHECK: FieldDecl {{.*}} s6 '__layout_B'
139+
// CHECK: CXXRecordDecl {{.*}} implicit referenced struct __cblayout_CB_4 definition
140+
// CHECK: PackedAttr
141+
// CHECK-NEXT: FieldDecl {{.*}} s5 '__cblayout_E'
142+
// CHECK-NEXT: FieldDecl {{.*}} s6 '__cblayout_B'
130143
}
131-
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_E definition
132-
// CHECK: FieldDecl {{.*}} c 'float'
133-
// CHECK-NOT: CXXRecordDecl {{.*}} class __layout_B definition
134-
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_E), "");
135-
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layout_CB_4), "");
144+
// CHECK: CXXRecordDecl {{.*}} implicit referenced struct __cblayout_E definition
145+
// CHECK: PackedAttr
146+
// CHECK-NEXT: FieldDecl {{.*}} c 'float'
147+
// CHECK-NOT: CXXRecordDecl {{.*}} struct __cblayout_B definition
148+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __cblayout_E), "");
149+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __cblayout_CB_4), "");
136150

137151
// check that this produces empty layout struct
138-
// CHECK: HLSLBufferDecl {{.*}} line:141:9 cbuffer CB
152+
// CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 3]]:9 cbuffer CB
139153
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
140154
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
141155
cbuffer CB {
@@ -149,27 +163,30 @@ cbuffer CB {
149163
RWBuffer<float> Buf;
150164
// CHECK: VarDecl {{.*}} ea 'EmptyArrayTypedef':'float[10][0]'
151165
EmptyArrayTypedef ea;
152-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout_CB_5 definition
166+
// CHECK: CXXRecordDecl {{.*}} implicit struct __cblayout_CB_5 definition
167+
// CHECK: PackedAttr
153168
// CHECK-NOT: FieldDecl
154169
}
155170

156171
// check host layout struct with compatible base struct
157-
// CHECK: HLSLBufferDecl {{.*}} line:160:9 cbuffer CB
172+
// CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 3]]:9 cbuffer CB
158173
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
159174
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
160175
cbuffer CB {
161176
// CHECK: VarDecl {{.*}} s8 'hlsl_constant F'
162177
F s8;
163-
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_6 definition
164-
// CHECK: FieldDecl {{.*}} s8 '__layout_F'
178+
// CHECK: CXXRecordDecl {{.*}} implicit referenced struct __cblayout_CB_6 definition
179+
// CHECK: PackedAttr
180+
// CHECK-NEXT: FieldDecl {{.*}} s8 '__cblayout_F'
165181
}
166-
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_F definition
167-
// CHECK: public 'A'
168-
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_F), "");
169-
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_CB_6), "");
182+
// CHECK: CXXRecordDecl {{.*}} implicit referenced struct __cblayout_F definition
183+
// CHECK: public 'A'
184+
// CHECK: PackedAttr
185+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __cblayout_F), "");
186+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __cblayout_CB_6), "");
170187

171188
// anonymous structs
172-
// CHECK: HLSLBufferDecl {{.*}} line:175:9 cbuffer CB
189+
// CHECK: HLSLBufferDecl {{.*}} line:[[# @LINE + 3]]:9 cbuffer CB
173190
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
174191
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
175192
cbuffer CB {
@@ -182,26 +199,29 @@ cbuffer CB {
182199
// CHECK: FieldDecl {{.*}} f 'RWBuffer<float>':'hlsl::RWBuffer<float>'
183200
RWBuffer<float> f;
184201
} s9;
185-
// CHECK: VarDecl {{.*}} s9 'hlsl_constant struct (unnamed struct at {{.*}}cbuffer.hlsl:177:3
202+
// CHECK: VarDecl {{.*}} s9 'hlsl_constant struct (unnamed struct at {{.*}}cbuffer.hlsl:[[# @LINE - 8]]:3
186203
// CHECK: CXXRecordDecl {{.*}} struct definition
187204
struct {
188205
// CHECK: FieldDecl {{.*}} g 'float'
189206
float g;
190207
// CHECK: FieldDecl {{.*}} f 'RWBuffer<float>':'hlsl::RWBuffer<float>'
191208
RWBuffer<float> f;
192209
} s10;
193-
// CHECK: VarDecl {{.*}} s10 'hlsl_constant struct (unnamed struct at {{.*}}cbuffer.hlsl:187:3
194-
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_anon definition
195-
// CHECK: FieldDecl {{.*}} e 'float'
196-
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_anon_1 definition
197-
// CHECK: FieldDecl {{.*}} g 'float'
198-
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_7 definition
199-
// CHECK: FieldDecl {{.*}} s9 '__layout_anon'
200-
// CHECK: FieldDecl {{.*}} s10 '__layout_anon_1'
210+
// CHECK: VarDecl {{.*}} s10 'hlsl_constant struct (unnamed struct at {{.*}}cbuffer.hlsl:[[# @LINE - 6]]:3
211+
// CHECK: CXXRecordDecl {{.*}} implicit referenced struct __cblayout_anon definition
212+
// CHECK: PackedAttr
213+
// CHECK-NEXT: FieldDecl {{.*}} e 'float'
214+
// CHECK: CXXRecordDecl {{.*}} implicit referenced struct __cblayout_anon_1 definition
215+
// CHECK: PackedAttr
216+
// CHECK-NEXT: FieldDecl {{.*}} g 'float'
217+
// CHECK: CXXRecordDecl {{.*}} implicit referenced struct __cblayout_CB_7 definition
218+
// CHECK: PackedAttr
219+
// CHECK-NEXT: FieldDecl {{.*}} s9 '__cblayout_anon'
220+
// CHECK-NEXT: FieldDecl {{.*}} s10 '__cblayout_anon_1'
201221
}
202-
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_anon), "");
203-
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_anon_1), "");
204-
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layout_CB_7), "");
222+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __cblayout_anon), "");
223+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __cblayout_anon_1), "");
224+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __cblayout_CB_7), "");
205225

206226
// Add uses for the constant buffer declarations so they are not optimized away
207227
export float foo() {

0 commit comments

Comments
 (0)