Skip to content

Commit 8c3f40c

Browse files
authored
Add warning when vk::offset is not correctly aligned (microsoft#6694)
We will start issues a warning when `vk::offset` is not correctly aligned to make it easier for users to understand why their spir-v will not validate. Note that we do not treat this as an error because we want to allow someone to have the flexibility to do other things. For example, they could be targeting an API that does not follow any of the existing rules, which is why they are using `vk::offset`. Fixes microsoft#6171
1 parent 206b7c2 commit 8c3f40c

File tree

3 files changed

+52
-3
lines changed

3 files changed

+52
-3
lines changed

tools/clang/lib/SPIRV/AlignmentSizeCalculator.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,18 @@ std::pair<uint32_t, uint32_t> AlignmentSizeCalculator::getAlignmentAndSize(
133133
}
134134

135135
// Reset the current offset to the one specified in the source code
136-
// if exists. It's debatable whether we should do sanity check here.
137-
// If the developers want manually control the layout, we leave
138-
// everything to them.
136+
// if exists. We issues a warning instead of an error if the offset is not
137+
// correctly aligned. This allows uses to disable validation, and use the
138+
// alignment specified in the source code if they are sure that is what they
139+
// want.
139140
if (const auto *offsetAttr = field->getAttr<VKOffsetAttr>()) {
140141
structSize = offsetAttr->getOffset();
142+
if (structSize % memberAlignment != 0) {
143+
emitWarning(
144+
"The offset provided in the attribute should be %0-byte aligned.",
145+
field->getLocation())
146+
<< memberAlignment;
147+
}
141148
}
142149

143150
// The base alignment of the structure is N, where N is the largest

tools/clang/lib/SPIRV/AlignmentSizeCalculator.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,15 @@ class AlignmentSizeCalculator {
6767
return astContext.getDiagnostics().Report(srcLoc, diagId);
6868
}
6969

70+
/// Emits warning to the diagnostic engine associated with this visitor.
71+
template <unsigned N>
72+
DiagnosticBuilder emitWarning(const char (&message)[N],
73+
SourceLocation srcLoc = {}) const {
74+
const auto diagId = astContext.getDiagnostics().getCustomDiagID(
75+
clang::DiagnosticsEngine::Warning, message);
76+
return astContext.getDiagnostics().Report(srcLoc, diagId);
77+
}
78+
7079
// Returns the alignment and size in bytes for the given struct
7180
// according to the given LayoutRule.
7281
std::pair<uint32_t, uint32_t>
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// RUN: not %dxc %s -HV 2021 -T cs_6_6 -E main -fspv-target-env=vulkan1.3 -fcgl -spirv 2>&1 | FileCheck %s -check-prefix=WARNING
2+
// RUN: %dxc %s -HV 2021 -T cs_6_6 -E main -fspv-target-env=vulkan1.3 -fcgl -spirv -fvk-use-scalar-layout 2>&1 | FileCheck %s -check-prefix=SCALAR
3+
4+
struct complex
5+
{
6+
float r;
7+
float i;
8+
};
9+
10+
// When using the default layout, the member should be aligned at a multiple of 16.
11+
// We expect an error
12+
// WARNING: :20:31: warning: The offset provided in the attribute should be 16-byte aligned.
13+
14+
// When using scalar layout, the member should be placed at offset 8.
15+
// SCALAR: OpMemberDecorate %Error 1 Offset 8
16+
17+
struct Error
18+
{
19+
float2 a;
20+
[[vk::offset(8)]] complex b;
21+
};
22+
23+
cbuffer Stuff
24+
{
25+
Error b[10];
26+
};
27+
28+
[numthreads(1, 1, 1)]
29+
void main()
30+
{
31+
Error testValue = b[0];
32+
}
33+

0 commit comments

Comments
 (0)