Skip to content

Commit

Permalink
Add warning when vk::offset is not correctly aligned (#6694)
Browse files Browse the repository at this point in the history
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 #6171
  • Loading branch information
s-perron authored Jun 17, 2024
1 parent 206b7c2 commit 8c3f40c
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 3 deletions.
13 changes: 10 additions & 3 deletions tools/clang/lib/SPIRV/AlignmentSizeCalculator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,18 @@ std::pair<uint32_t, uint32_t> AlignmentSizeCalculator::getAlignmentAndSize(
}

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

// The base alignment of the structure is N, where N is the largest
Expand Down
9 changes: 9 additions & 0 deletions tools/clang/lib/SPIRV/AlignmentSizeCalculator.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ class AlignmentSizeCalculator {
return astContext.getDiagnostics().Report(srcLoc, diagId);
}

/// Emits warning to the diagnostic engine associated with this visitor.
template <unsigned N>
DiagnosticBuilder emitWarning(const char (&message)[N],
SourceLocation srcLoc = {}) const {
const auto diagId = astContext.getDiagnostics().getCustomDiagID(
clang::DiagnosticsEngine::Warning, message);
return astContext.getDiagnostics().Report(srcLoc, diagId);
}

// Returns the alignment and size in bytes for the given struct
// according to the given LayoutRule.
std::pair<uint32_t, uint32_t>
Expand Down
33 changes: 33 additions & 0 deletions tools/clang/test/CodeGenSPIRV/bad.alignment.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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
// 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

struct complex
{
float r;
float i;
};

// When using the default layout, the member should be aligned at a multiple of 16.
// We expect an error
// WARNING: :20:31: warning: The offset provided in the attribute should be 16-byte aligned.

// When using scalar layout, the member should be placed at offset 8.
// SCALAR: OpMemberDecorate %Error 1 Offset 8

struct Error
{
float2 a;
[[vk::offset(8)]] complex b;
};

cbuffer Stuff
{
Error b[10];
};

[numthreads(1, 1, 1)]
void main()
{
Error testValue = b[0];
}

0 comments on commit 8c3f40c

Please sign in to comment.