Skip to content

Commit 9b5f5c9

Browse files
authored
[SPIRV] Use OpCopyLogical to reconstruct values (#7530)
When DXC needs to change the layout of a value, it currently has to extract each individual scalar, and then reconstruct using the type with the different layout. If you have a large array or struct with many member, this generates a lot of extra code. Starting in SPIR-V 1.4, the OpCopyLogical instruction is available to do the reconstruction. This should help generate less code, which will lead to improved compile time and maybe smaller binary sizes. Fixes #7493
1 parent 2084643 commit 9b5f5c9

File tree

5 files changed

+133
-0
lines changed

5 files changed

+133
-0
lines changed

tools/clang/include/clang/SPIRV/AstTypeProbe.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,10 @@ bool isOrContainsNonFpColMajorMatrix(const ASTContext &,
337337
const SpirvCodeGenOptions &, QualType type,
338338
const Decl *decl);
339339

340+
/// brief Returns true if the type is a boolean type or an aggragate type that
341+
/// contains a boolean type.
342+
bool isOrContainsBoolType(QualType type);
343+
340344
/// \brief Returns true if the given type is `vk::ext_result_id<T>`.
341345
bool isExtResultIdType(QualType type);
342346

tools/clang/lib/SPIRV/AstTypeProbe.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,6 +1353,27 @@ bool isOrContainsNonFpColMajorMatrix(const ASTContext &astContext,
13531353
return false;
13541354
}
13551355

1356+
bool isOrContainsBoolType(QualType type) {
1357+
if (isBoolOrVecMatOfBoolType(type)) {
1358+
return true;
1359+
}
1360+
1361+
if (const auto *arrayType = type->getAsArrayTypeUnsafe()) {
1362+
return isOrContainsBoolType(arrayType->getElementType());
1363+
}
1364+
1365+
if (const auto *recordType = type->getAs<RecordType>()) {
1366+
for (auto field : recordType->getDecl()->fields()) {
1367+
if (isOrContainsBoolType(field->getType())) {
1368+
return true;
1369+
}
1370+
}
1371+
return false;
1372+
}
1373+
1374+
return false;
1375+
}
1376+
13561377
bool isTypeInVkNamespace(const RecordType *type) {
13571378
if (const auto *nameSpaceDecl =
13581379
dyn_cast<NamespaceDecl>(type->getDecl()->getDeclContext())) {

tools/clang/lib/SPIRV/SpirvEmitter.cpp

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7108,6 +7108,38 @@ void SpirvEmitter::storeValue(SpirvInstruction *lhsPtr,
71087108
}
71097109
}
71107110

7111+
bool SpirvEmitter::canUseOpCopyLogical(QualType type) const {
7112+
if (featureManager.getSpirvVersion(featureManager.getTargetEnv()) <
7113+
VersionTuple(1, 4)) {
7114+
return false;
7115+
}
7116+
7117+
if (!type->isArrayType() && !type->isRecordType()) {
7118+
return false;
7119+
}
7120+
7121+
if (const auto *recordType = type->getAs<RecordType>()) {
7122+
if (isTypeInVkNamespace(recordType) &&
7123+
(recordType->getDecl()->getName().equals("BufferPointer") ||
7124+
recordType->getDecl()->getName().equals("SpirvType") ||
7125+
recordType->getDecl()->getName().equals("SpirvOpaqueType"))) {
7126+
// vk::BufferPointer<T> lowers to a pointer type. No need to reconstruct
7127+
// the value. The vk::Spirv*Type should be treated an opaque type. All we
7128+
// can do is leave it the same.
7129+
return false;
7130+
}
7131+
}
7132+
7133+
if (hlsl::IsHLSLVecMatType(type) || hlsl::IsHLSLResourceType(type)) {
7134+
return false;
7135+
}
7136+
7137+
// If the type contains a bool it is possible that one type represents it with
7138+
// a bool and the other with an int. If that happens, OpCopyLogical is not
7139+
// valid.
7140+
return !isOrContainsBoolType(type);
7141+
}
7142+
71117143
SpirvInstruction *SpirvEmitter::reconstructValue(SpirvInstruction *srcVal,
71127144
const QualType valType,
71137145
SpirvLayoutRule dstLR,
@@ -7171,6 +7203,13 @@ SpirvInstruction *SpirvEmitter::reconstructValue(SpirvInstruction *srcVal,
71717203
return result;
71727204
};
71737205

7206+
if (canUseOpCopyLogical(valType)) {
7207+
SpirvInstruction *copy = spvBuilder.createUnaryOp(
7208+
spv::Op::OpCopyLogical, valType, srcVal, srcVal->getSourceLocation());
7209+
copy->setLayoutRule(dstLR);
7210+
return copy;
7211+
}
7212+
71747213
// Constant arrays
71757214
if (const auto *arrayType = astContext.getAsConstantArrayType(valType)) {
71767215
const auto elemType = arrayType->getElementType();

tools/clang/lib/SPIRV/SpirvEmitter.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,8 @@ class SpirvEmitter : public ASTConsumer {
228228
QualType lhsValType, SourceLocation loc,
229229
SourceRange range = {});
230230

231+
bool canUseOpCopyLogical(QualType type) const;
232+
231233
/// Decomposes and reconstructs the given srcVal of the given valType to meet
232234
/// the requirements of the dstLR layout rule.
233235
SpirvInstruction *reconstructValue(SpirvInstruction *srcVal, QualType valType,
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// RUN: %dxc %s -fcgl -spirv -T ps_6_8 -fspv-target-env=vulkan1.1spirv1.4 | FileCheck %s
2+
3+
4+
5+
struct WithBool {
6+
bool b;
7+
};
8+
9+
struct StructWithBool {
10+
WithBool wb;
11+
};
12+
13+
struct StructWithoutBool {
14+
int a;
15+
};
16+
17+
struct OuterStruct {
18+
StructWithBool a[2];
19+
WithBool b;
20+
StructWithoutBool c;
21+
StructWithoutBool d[2];
22+
} S;
23+
24+
25+
// CHECK: %GetStruct = OpFunction %OuterStruct_0 None %34
26+
// CHECK: %bb_entry_0 = OpLabel
27+
// CHECK: [[ld:%[0-9]+]] = OpLoad %OuterStruct %39
28+
29+
// The array `a` must be split up because it contains a bool that needs a
30+
// conversion from int to bool.
31+
// CHECK: [[arr_with_bool:%[0-9]+]] = OpCompositeExtract %_arr_StructWithBool_uint_2 [[ld]] 0
32+
// CHECK: [[struct_with_bool:%[0-9]+]] = OpCompositeExtract %StructWithBool [[arr_with_bool]] 0
33+
// CHECK: [[with_bool:%[0-9]+]] = OpCompositeExtract %WithBool [[struct_with_bool]] 0
34+
// CHECK: [[int:%[0-9]+]] = OpCompositeExtract %uint [[with_bool]] 0
35+
// CHECK: [[bool:%[0-9]+]] = OpINotEqual %bool [[int]] %uint_0
36+
// CHECK: [[with_bool:%[0-9]+]] = OpCompositeConstruct %WithBool_0 [[bool]]
37+
// CHECK: [[struct_with_bool:%[0-9]+]] = OpCompositeConstruct %StructWithBool_0 [[with_bool]]
38+
39+
// Skip second element of the array. It is more of the same.
40+
// CHECK: [[a:%[0-9]+]] = OpCompositeConstruct %_arr_StructWithBool_0_uint_2 [[struct_with_bool]] {{%.*}}
41+
42+
// The struct `b` must be split up for the same reason.
43+
// CHECK: [[with_bool:%[0-9]+]] = OpCompositeExtract %WithBool [[ld]] 1
44+
// CHECK: [[int:%[0-9]+]] = OpCompositeExtract %uint [[with_bool]] 0
45+
// CHECK: [[bool:%[0-9]+]] = OpINotEqual %bool [[int]] %uint_0
46+
// CHECK: [[b:%[0-9]+]] = OpCompositeConstruct %WithBool_0 [[bool]]
47+
48+
// The struct `c` can use OpCopyLogical.
49+
// CHECK: %59 = OpCompositeExtract %StructWithoutBool [[ld]] 2
50+
// CHECK: [[c:%[0-9]+]] = OpCopyLogical %StructWithoutBool_0 %59
51+
52+
// The array `d` can use OpCopyLogical.
53+
// CHECK: %61 = OpCompositeExtract %_arr_StructWithoutBool_uint_2 [[ld]] 3
54+
// CHECK: [[d:%[0-9]+]] = OpCopyLogical %_arr_StructWithoutBool_0_uint_2 %61
55+
56+
// CHECK: [[r:%[0-9]+]] = OpCompositeConstruct %OuterStruct_0 [[a]] [[b]] [[c]] [[d]]
57+
// CHECK: OpStore {{%.*}} [[r]]
58+
// CHECK: OpFunctionEnd
59+
60+
OuterStruct GetStruct() { return S; }
61+
62+
uint main() : SV_TARGET
63+
{
64+
GetStruct();
65+
return 0;
66+
}
67+

0 commit comments

Comments
 (0)