Skip to content

Commit 2e40a11

Browse files
spirv-val: Fix OffsetIdEXT not checking if in struct (KhronosGroup#6708)
Pulled the `OffsetIdEXT` part out of KhronosGroup#6701 until I resolve https://gitlab.khronos.org/spirv/SPIR-V/-/issues/937 This is mainly because people are bringing up `SPV_EXT_descriptor_heap` in dxc/slang and want to not have this waste time hitting the "driver bug" which is actually [not a driver bug](https://gitlab.khronos.org/Tracker/vk-gl-cts/-/issues/6505)
1 parent c2bafae commit 2e40a11

2 files changed

Lines changed: 50 additions & 7 deletions

File tree

source/val/validate_annotation.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,10 @@ bool IsMemberDecorationOnly(spv::Decoration dec) {
5353
case spv::Decoration::RowMajor:
5454
case spv::Decoration::ColMajor:
5555
case spv::Decoration::MatrixStride:
56-
// SPIR-V spec bug? Offset is generated on variables when dealing with
57-
// transform feedback.
58-
// case spv::Decoration::Offset:
56+
// Spec ambiguity where this is allowed or not currently
57+
// See https://gitlab.khronos.org/spirv/SPIR-V/-/issues/937
58+
// case spv::Decoration::Offset:
59+
case spv::Decoration::OffsetIdEXT:
5960
return true;
6061
default:
6162
break;
@@ -427,11 +428,12 @@ spv_result_t ValidateDecorateId(ValidationState_t& _, const Instruction* inst) {
427428
}
428429
}
429430

430-
// No member decorations take id parameters, so we don't bother checking if
431-
// we are using a member only decoration here.
431+
if (IsMemberDecorationOnly(decoration)) {
432+
return _.diag(SPV_ERROR_INVALID_ID, inst)
433+
<< _.SpvDecorationString(decoration)
434+
<< " can only be applied to structure members";
435+
}
432436

433-
// TODO: Add validations for these decorations.
434-
// UniformId is covered elsewhere.
435437
return SPV_SUCCESS;
436438
}
437439

test/val/val_extension_spv_ext_descriptor_heap.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2417,6 +2417,47 @@ TEST_F(ValidateSpvEXTDescriptorHeap, ArrayStrideSpecConstantZero) {
24172417
EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_4));
24182418
}
24192419

2420+
// https://github.com/KhronosGroup/SPIRV-Tools/issues/6696
2421+
TEST_F(ValidateSpvEXTDescriptorHeap, OffsetIdOnArray) {
2422+
const std::string str = R"(
2423+
OpCapability Shader
2424+
OpCapability UntypedPointersKHR
2425+
OpCapability DescriptorHeapEXT
2426+
OpCapability Sampled1D
2427+
OpExtension "SPV_KHR_untyped_pointers"
2428+
OpExtension "SPV_EXT_descriptor_heap"
2429+
OpMemoryModel Logical GLSL450
2430+
OpEntryPoint GLCompute %1 "main" %2
2431+
OpExecutionMode %1 LocalSize 1 1 1
2432+
OpDecorate %2 BuiltIn ResourceHeapEXT
2433+
OpDecorateId %7 OffsetIdEXT %4
2434+
%11 = OpTypeVoid
2435+
%12 = OpTypeFunction %11
2436+
%13 = OpTypeInt 32 0
2437+
%16 = OpConstant %13 0
2438+
%4 = OpConstant %13 0
2439+
%20 = OpTypeImage %13 1D 0 0 0 1 Unknown
2440+
%21 = OpTypeBufferEXT StorageBuffer
2441+
%22 = OpTypeSampler
2442+
%23 = OpTypeSampledImage %20
2443+
%7 = OpTypeRuntimeArray %20
2444+
%24 = OpTypeUntypedPointerKHR UniformConstant
2445+
%25 = OpTypeUntypedPointerKHR StorageBuffer
2446+
%2 = OpUntypedVariableKHR %24 UniformConstant
2447+
%1 = OpFunction %11 None %12
2448+
%26 = OpLabel
2449+
%27 = OpUntypedAccessChainKHR %24 %7 %2 %16
2450+
%28 = OpLoad %20 %27
2451+
OpReturn
2452+
OpFunctionEnd
2453+
)";
2454+
CompileSuccessfully(str.c_str(), SPV_ENV_VULKAN_1_4);
2455+
EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_4));
2456+
EXPECT_THAT(
2457+
getDiagnosticString(),
2458+
HasSubstr("OffsetIdEXT can only be applied to structure members"));
2459+
}
2460+
24202461
} // namespace
24212462
} // namespace val
24222463
} // namespace spvtools

0 commit comments

Comments
 (0)