Skip to content

Commit 75edd2b

Browse files
hekotahekota
committed
code review feedback - rename function and layout struct, add static asserts
1 parent 0233994 commit 75edd2b

File tree

4 files changed

+83
-58
lines changed

4 files changed

+83
-58
lines changed

‎clang/lib/Sema/SemaHLSL.cpp‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -265,29 +265,31 @@ static bool isZeroSizedArray(const ConstantArrayType *CAT) {
265265
return CAT != nullptr;
266266
}
267267

268-
// Returns true if the struct can be used inside HLSL Buffer which means
269-
// that it does not contain intangible types, empty structs, zero-sized arrays,
270-
// and the same is true for its base or embedded structs.
271-
bool isStructHLSLBufferCompatible(const CXXRecordDecl *RD) {
272-
if (RD->getTypeForDecl()->isHLSLIntangibleType() ||
273-
(RD->field_empty() && RD->getNumBases() == 0))
274-
return false;
268+
// Returns true if the struct contains at least one element that prevents it
269+
// from being included inside HLSL Buffer as is, such as an intangible type,
270+
// empty struct, or zero-sized array. If it does, a new implicit layout struct
271+
// needs to be created for HLSL Buffer use that will exclude these unwanted
272+
// declarations (see createHostLayoutStruct function).
273+
static bool requiresImplicitBufferLayoutStructure(const CXXRecordDecl *RD) {
274+
if (RD->getTypeForDecl()->isHLSLIntangibleType() || RD->isEmpty())
275+
return true;
275276
// check fields
276277
for (const FieldDecl *Field : RD->fields()) {
277278
QualType Ty = Field->getType();
278279
if (Ty->isRecordType()) {
279-
if (!isStructHLSLBufferCompatible(Ty->getAsCXXRecordDecl()))
280-
return false;
280+
if (requiresImplicitBufferLayoutStructure(Ty->getAsCXXRecordDecl()))
281+
return true;
281282
} else if (Ty->isConstantArrayType()) {
282283
if (isZeroSizedArray(cast<ConstantArrayType>(Ty)))
283-
return false;
284+
return true;
284285
}
285286
}
286287
// check bases
287288
for (const CXXBaseSpecifier &Base : RD->bases())
288-
if (!isStructHLSLBufferCompatible(Base.getType()->getAsCXXRecordDecl()))
289-
return false;
290-
return true;
289+
if (requiresImplicitBufferLayoutStructure(
290+
Base.getType()->getAsCXXRecordDecl()))
291+
return true;
292+
return false;
291293
}
292294

293295
static CXXRecordDecl *findRecordDecl(Sema &S, IdentifierInfo *II,
@@ -318,15 +320,15 @@ static IdentifierInfo *getHostLayoutStructName(Sema &S,
318320
MustBeUnique = true;
319321
}
320322

321-
std::string Name = "__layout." + NameBase;
323+
std::string Name = "__layout_" + NameBase;
322324
IdentifierInfo *II = &AST.Idents.get(Name, tok::TokenKind::identifier);
323325
if (!MustBeUnique)
324326
return II;
325327

326328
unsigned suffix = 0;
327329
while (true) {
328330
if (suffix != 0)
329-
II = &AST.Idents.get((llvm::Twine(Name) + "." + Twine(suffix)).str(),
331+
II = &AST.Idents.get((llvm::Twine(Name) + "_" + Twine(suffix)).str(),
330332
tok::TokenKind::identifier);
331333
if (!findRecordDecl(S, II, DC))
332334
return II;
@@ -354,7 +356,7 @@ static FieldDecl *createFieldForHostLayoutStruct(Sema &S, const Type *Ty,
354356
if (isResourceRecordType(Ty))
355357
return nullptr;
356358
CXXRecordDecl *RD = Ty->getAsCXXRecordDecl();
357-
if (!isStructHLSLBufferCompatible(RD)) {
359+
if (requiresImplicitBufferLayoutStructure(RD)) {
358360
RD = createHostLayoutStruct(S, RD, BufDecl);
359361
if (!RD)
360362
return nullptr;
@@ -383,7 +385,7 @@ static FieldDecl *createFieldForHostLayoutStruct(Sema &S, const Type *Ty,
383385
// Returns nullptr if the resulting layout struct would be empty.
384386
static CXXRecordDecl *createHostLayoutStruct(Sema &S, CXXRecordDecl *StructDecl,
385387
HLSLBufferDecl *BufDecl) {
386-
assert(!isStructHLSLBufferCompatible(StructDecl) &&
388+
assert(requiresImplicitBufferLayoutStructure(StructDecl) &&
387389
"struct is already HLSL buffer compatible");
388390

389391
ASTContext &AST = S.getASTContext();
@@ -406,7 +408,7 @@ static CXXRecordDecl *createHostLayoutStruct(Sema &S, CXXRecordDecl *StructDecl,
406408
assert(NumBases == 1 && "HLSL supports only one base type");
407409
CXXBaseSpecifier Base = *StructDecl->bases_begin();
408410
CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
409-
if (!isStructHLSLBufferCompatible(BaseDecl)) {
411+
if (requiresImplicitBufferLayoutStructure(BaseDecl)) {
410412
BaseDecl = createHostLayoutStruct(S, BaseDecl, BufDecl);
411413
if (BaseDecl) {
412414
TypeSourceInfo *TSI = AST.getTrivialTypeSourceInfo(

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ tbuffer B {
4545
// AST-NEXT: TextComment {{.*}} Text=" CBuffer decl."
4646
// AST-NEXT: VarDecl {{.*}} a 'float'
4747
// AST-NEXT: VarDecl {{.*}} b 'int'
48-
// AST-NEXT: CXXRecordDecl {{.*}} implicit class __layout.A definition
48+
// AST-NEXT: CXXRecordDecl {{.*}} implicit class __layout_A definition
4949
// AST: FieldDecl {{.*}} a 'float'
5050
// AST-NEXT: FieldDecl {{.*}} b 'int'
5151

@@ -57,6 +57,6 @@ tbuffer B {
5757
// AST-NEXT: TextComment {{.*}} Text=" TBuffer decl."
5858
// AST-NEXT: VarDecl {{.*}} c 'float'
5959
// AST-NEXT: VarDecl {{.*}} d 'int'
60-
// AST-NEXT: CXXRecordDecl {{.*}} implicit class __layout.B definition
60+
// AST-NEXT: CXXRecordDecl {{.*}} implicit class __layout_B definition
6161
// AST: FieldDecl {{.*}} c 'float'
6262
// AST-NEXT: FieldDecl {{.*}} d 'int'

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

Lines changed: 59 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,28 @@ struct F : A {
3535

3636
typedef float EmptyArrayTypedef[10][0];
3737

38-
// CHECK: HLSLBufferDecl {{.*}} line:41:9 cbuffer CB
38+
struct OneFloat {
39+
float a;
40+
};
41+
42+
struct TwoFloats {
43+
float a;
44+
float b;
45+
};
46+
47+
// CHECK: HLSLBufferDecl {{.*}} line:50:9 cbuffer CB
3948
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
4049
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
4150
cbuffer CB {
4251
// CHECK: VarDecl {{.*}} col:9 used a1 'float'
4352
float a1;
44-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.CB definition
53+
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB definition
4554
// CHECK: FieldDecl {{.*}} a1 'float'
4655
}
56+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_CB), "");
4757

4858
// Check that buffer layout struct does not include resources or empty types
49-
// CHECK: HLSLBufferDecl {{.*}} line:52:9 cbuffer CB
59+
// CHECK: HLSLBufferDecl {{.*}} line:62:9 cbuffer CB
5060
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
5161
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
5262
cbuffer CB {
@@ -60,13 +70,14 @@ cbuffer CB {
6070
float d2[0];
6171
// CHECK: VarDecl {{.*}} col:9 e2 'float'
6272
float e2;
63-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.CB.1 definition
73+
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_1 definition
6474
// CHECK: FieldDecl {{.*}} a2 'float'
6575
// CHECK-NEXT: FieldDecl {{.*}} e2 'float'
6676
}
77+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layout_CB_1), "");
6778

6879
// Check that layout struct is created for B and the empty struct C is removed
69-
// CHECK: HLSLBufferDecl {{.*}} line:72:9 cbuffer CB
80+
// CHECK: HLSLBufferDecl {{.*}} line:83:9 cbuffer CB
7081
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
7182
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
7283
cbuffer CB {
@@ -76,47 +87,54 @@ cbuffer CB {
7687
B s2;
7788
// CHECK: VarDecl {{.*}} col:12 s3 'CTypedef':'C
7889
CTypedef s3;
79-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.B definition
90+
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_B definition
8091
// CHECK: FieldDecl {{.*}} a 'float'
81-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.CB.2 definition
92+
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_2 definition
8293
// CHECK: FieldDecl {{.*}} s1 'A'
83-
// CHECK: FieldDecl {{.*}} s2 '__layout.B'
94+
// CHECK: FieldDecl {{.*}} s2 '__layout_B'
8495
}
96+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_B), "");
97+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layout_CB_2), "");
8598

8699
// check that layout struct is created for D because of its base struct
87-
// CHECK: HLSLBufferDecl {{.*}} line:90:9 cbuffer CB
100+
// CHECK: HLSLBufferDecl {{.*}} line:103:9 cbuffer CB
88101
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
89102
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
90103
cbuffer CB {
91104
// CHECK: VarDecl {{.*}} s4 'D'
92105
D s4;
93-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.D definition
106+
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_D definition
107+
// CHECK: public '__layout_B'
94108
// CHECK: FieldDecl {{.*}} b 'float'
95-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.CB.3 definition
96-
// CHECK: FieldDecl {{.*}} s4 '__layout.D'
109+
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_3 definition
110+
// CHECK: FieldDecl {{.*}} s4 '__layout_D'
97111
}
112+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layout_D), "");
113+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layout_CB_3), "");
98114

99115
// check that layout struct is created for E because because its base struct
100116
// is empty and should be eliminated, and BTypedef should reuse the previously
101-
// defined '__layout.B'
102-
// CHECK: HLSLBufferDecl {{.*}} line:105:9 cbuffer CB
117+
// defined '__layout_B'
118+
// CHECK: HLSLBufferDecl {{.*}} line:121:9 cbuffer CB
103119
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
104120
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
105121
cbuffer CB {
106122
// CHECK: VarDecl {{.*}} s5 'E'
107123
E s5;
108124
// CHECK: VarDecl {{.*}} s6 'BTypedef':'B'
109125
BTypedef s6;
110-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.E definition
126+
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_E definition
111127
// CHECK: FieldDecl {{.*}} c 'float'
112-
// CHECK-NOT: CXXRecordDecl {{.*}} implicit class __layout.B definition
113-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.CB.4 definition
114-
// CHECK: FieldDecl {{.*}} s5 '__layout.E'
115-
// CHECK: FieldDecl {{.*}} s6 '__layout.B'
128+
// CHECK-NOT: CXXRecordDecl {{.*}} class __layout_B definition
129+
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_4 definition
130+
// CHECK: FieldDecl {{.*}} s5 '__layout_E'
131+
// CHECK: FieldDecl {{.*}} s6 '__layout_B'
116132
}
133+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_E), "");
134+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layout_CB_4), "");
117135

118136
// check that this produces empty layout struct
119-
// CHECK: HLSLBufferDecl {{.*}} line:122:9 cbuffer CB
137+
// CHECK: HLSLBufferDecl {{.*}} line:140:9 cbuffer CB
120138
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
121139
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
122140
cbuffer CB {
@@ -130,25 +148,27 @@ cbuffer CB {
130148
RWBuffer<float> Buf;
131149
// CHECK: VarDecl {{.*}} ea 'EmptyArrayTypedef':'float[10][0]'
132150
EmptyArrayTypedef ea;
133-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.CB.5 definition
151+
// CHECK: CXXRecordDecl {{.*}} implicit class __layout_CB_5 definition
134152
// CHECK-NOT: FieldDecl
135153
}
136154

137155
// check host layout struct with compatible base struct
138-
// CHECK: HLSLBufferDecl {{.*}} line:141:9 cbuffer CB
156+
// CHECK: HLSLBufferDecl {{.*}} line:159:9 cbuffer CB
139157
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
140158
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
141159
cbuffer CB {
142160
// CHECK: VarDecl {{.*}} s8 'F'
143161
F s8;
144-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.F definition
162+
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_F definition
145163
// CHECK: public 'A'
146-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.CB.6 definition
147-
// CHECK: FieldDecl {{.*}} s8 '__layout.F'
164+
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_6 definition
165+
// CHECK: FieldDecl {{.*}} s8 '__layout_F'
148166
}
167+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_F), "");
168+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_CB_6), "");
149169

150170
// anonymous structs
151-
// CHECK: HLSLBufferDecl {{.*}} line:154:9 cbuffer CB
171+
// CHECK: HLSLBufferDecl {{.*}} line:174:9 cbuffer CB
152172
// CHECK: HLSLResourceClassAttr {{.*}} Implicit CBuffer
153173
// CHECK: HLSLResourceAttr {{.*}} Implicit CBuffer
154174
cbuffer CB {
@@ -161,23 +181,26 @@ cbuffer CB {
161181
// CHECK: FieldDecl {{.*}} f 'RWBuffer<float>':'hlsl::RWBuffer<float>'
162182
RWBuffer<float> f;
163183
} s9;
164-
// CHECK: VarDecl {{.*}} s9 'struct (unnamed struct at {{.*}}cbuffer.hlsl:156:3
184+
// CHECK: VarDecl {{.*}} s9 'struct (unnamed struct at {{.*}}cbuffer.hlsl:176:3
165185
// CHECK: CXXRecordDecl {{.*}} struct definition
166186
struct {
167-
// CHECK: FieldDecl {{.*}} g 'int'
168-
int g;
187+
// CHECK: FieldDecl {{.*}} g 'float'
188+
float g;
169189
// CHECK: FieldDecl {{.*}} f 'RWBuffer<float>':'hlsl::RWBuffer<float>'
170190
RWBuffer<float> f;
171191
} s10;
172-
// CHECK: VarDecl {{.*}} s10 'struct (unnamed struct at {{.*}}cbuffer.hlsl:166:3
173-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.anon definition
192+
// CHECK: VarDecl {{.*}} s10 'struct (unnamed struct at {{.*}}cbuffer.hlsl:186:3
193+
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_anon definition
174194
// CHECK: FieldDecl {{.*}} e 'float'
175-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.anon.1 definition
176-
// CHECK: FieldDecl {{.*}} g 'int'
177-
// CHECK: CXXRecordDecl {{.*}} implicit class __layout.CB.7 definition
178-
// CHECK: FieldDecl {{.*}} s9 '__layout.anon'
179-
// CHECK: FieldDecl {{.*}} s10 '__layout.anon.1'
195+
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_anon_1 definition
196+
// CHECK: FieldDecl {{.*}} g 'float'
197+
// CHECK: CXXRecordDecl {{.*}} implicit referenced class __layout_CB_7 definition
198+
// CHECK: FieldDecl {{.*}} s9 '__layout_anon'
199+
// CHECK: FieldDecl {{.*}} s10 '__layout_anon_1'
180200
}
201+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_anon), "");
202+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(OneFloat, __layout_anon_1), "");
203+
_Static_assert(__builtin_hlsl_is_scalarized_layout_compatible(TwoFloats, __layout_CB_7), "");
181204

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

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ float foo() {
2121
// CHECK-NEXT: HLSLResourceClassAttr {{.*}} Implicit CBuffer
2222
// CHECK-NEXT: HLSLResourceAttr {{.*}} Implicit CBuffer
2323
// CHECK-NEXT: VarDecl 0x[[A:[0-9a-f]+]] {{.*}} imported used a 'float'
24-
// CHECK-NEXT: CXXRecordDecl {{.*}} imported implicit <undeserialized declarations> class __layout.A definition
24+
// CHECK-NEXT: CXXRecordDecl {{.*}} imported implicit <undeserialized declarations> class __layout_A definition
2525
// CHECK: FieldDecl {{.*}} imported a 'float'
2626

2727
// CHECK: HLSLBufferDecl {{.*}} line:11:9 imported <undeserialized declarations> tbuffer B
2828
// CHECK-NEXT: HLSLResourceClassAttr {{.*}} Implicit SRV
2929
// CHECK-NEXT: HLSLResourceAttr {{.*}} Implicit TBuffer
3030
// CHECK-NEXT: VarDecl 0x[[B:[0-9a-f]+]] {{.*}} imported used b 'float'
31-
// CHECK-NEXT: CXXRecordDecl 0x{{[0-9a-f]+}} {{.*}} imported implicit <undeserialized declarations> class __layout.B definition
31+
// CHECK-NEXT: CXXRecordDecl 0x{{[0-9a-f]+}} {{.*}} imported implicit <undeserialized declarations> class __layout_B definition
3232
// CHECK: FieldDecl 0x{{[0-9a-f]+}} {{.*}} imported b 'float'
3333

3434
// CHECK-NEXT: FunctionDecl {{.*}} line:15:7 imported foo 'float ()'

0 commit comments

Comments
 (0)