Skip to content

Commit b50e727

Browse files
spencer-lunargjuan-lunarg
authored andcommitted
layers: Fix crash with push constants using spec const
1 parent 1e6caca commit b50e727

File tree

3 files changed

+84
-1
lines changed

3 files changed

+84
-1
lines changed

layers/state_tracker/shader_module.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1823,7 +1823,17 @@ PushConstantVariable::PushConstantVariable(const SHADER_MODULE_STATE& module_sta
18231823
}
18241824
}
18251825
const auto& highest_member = type_struct_info->members[highest_element_index];
1826-
const uint32_t highest_element_size = module_state.GetTypeBytesSize(highest_member.insn);
1826+
uint32_t highest_element_size = 0;
1827+
if (highest_member.insn->Opcode() == spv::OpTypeArray &&
1828+
module_state.FindDef(highest_member.insn->Word(3))->Opcode() == spv::OpSpecConstant) {
1829+
// TODO - This is a work-around because currently we only apply SpecConstant for workgroup size
1830+
// The shader validation needs to be fixed so we handle all cases when spec constant are applied, while still being catious
1831+
// of the fact that information is not known until pipeline creation (not at shader module creation time)
1832+
// https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/5911
1833+
highest_element_size = module_state.FindDef(highest_member.insn->Word(3))->Word(3);
1834+
} else {
1835+
highest_element_size = module_state.GetTypeBytesSize(highest_member.insn);
1836+
}
18271837
size = (highest_element_size + highest_element_offset) - offset;
18281838
}
18291839

tests/negative/shader_push_constants.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,3 +456,49 @@ TEST_F(NegativeShaderPushConstants, MultipleEntryPoint) {
456456
pipe.InitState();
457457
pipe.CreateGraphicsPipeline();
458458
}
459+
460+
// This is not working because of a bug in the Spec Constant logic
461+
// https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/5911
462+
TEST_F(NegativeShaderPushConstants, DISABLED_SpecConstantSize) {
463+
TEST_DESCRIPTION("Use SpecConstant to adjust size of Push Constant Block");
464+
ASSERT_NO_FATAL_FAILURE(Init());
465+
466+
const char *cs_source = R"glsl(
467+
#version 460
468+
layout (constant_id = 0) const int my_array_size = 1;
469+
layout (push_constant) uniform my_buf {
470+
float my_array[my_array_size];
471+
} pc;
472+
473+
void main() {
474+
float a = pc.my_array[0];
475+
}
476+
)glsl";
477+
478+
uint32_t data = 32;
479+
480+
VkSpecializationMapEntry entry;
481+
entry.constantID = 0;
482+
entry.offset = 0;
483+
entry.size = sizeof(uint32_t);
484+
485+
VkSpecializationInfo specialization_info = {};
486+
specialization_info.mapEntryCount = 1;
487+
specialization_info.pMapEntries = &entry;
488+
specialization_info.dataSize = sizeof(uint32_t);
489+
specialization_info.pData = &data;
490+
491+
// With spec constant set, this should be 32, not 16
492+
VkPushConstantRange push_constant_range = {VK_SHADER_STAGE_COMPUTE_BIT, 0, 16};
493+
const VkPipelineLayoutObj pipeline_layout(m_device, {}, {push_constant_range});
494+
495+
CreateComputePipelineHelper pipe(*this);
496+
pipe.InitInfo();
497+
pipe.cs_.reset(
498+
new VkShaderObj(this, cs_source, VK_SHADER_STAGE_COMPUTE_BIT, SPV_ENV_VULKAN_1_0, SPV_SOURCE_GLSL, &specialization_info));
499+
pipe.InitState();
500+
pipe.pipeline_layout_ = VkPipelineLayoutObj(m_device, {}, {push_constant_range});
501+
m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, "VUID-VkComputePipelineCreateInfo-layout-07987");
502+
pipe.CreateComputePipeline();
503+
m_errorMonitor->VerifyFound();
504+
}

tests/positive/shader_push_constants.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,3 +745,30 @@ TEST_F(PositiveShaderPushConstants, MultipleStructs) {
745745
m_commandBuffer->EndRenderPass();
746746
m_commandBuffer->end();
747747
}
748+
749+
TEST_F(PositiveShaderPushConstants, SpecConstantSize) {
750+
TEST_DESCRIPTION("Use SpecConstant to adjust size of Push Constant Block");
751+
ASSERT_NO_FATAL_FAILURE(Init());
752+
753+
const char *cs_source = R"glsl(
754+
#version 460
755+
layout (constant_id = 2) const int my_array_size = 1;
756+
layout (push_constant) uniform my_buf {
757+
float my_array[my_array_size];
758+
} pc;
759+
760+
void main() {
761+
float a = pc.my_array[0];
762+
}
763+
)glsl";
764+
765+
VkPushConstantRange push_constant_range = {VK_SHADER_STAGE_COMPUTE_BIT, 0, 32};
766+
const VkPipelineLayoutObj pipeline_layout(m_device, {}, {push_constant_range});
767+
768+
CreateComputePipelineHelper pipe(*this);
769+
pipe.InitInfo();
770+
pipe.cs_.reset(new VkShaderObj(this, cs_source, VK_SHADER_STAGE_COMPUTE_BIT));
771+
pipe.InitState();
772+
pipe.pipeline_layout_ = VkPipelineLayoutObj(m_device, {}, {push_constant_range});
773+
pipe.CreateComputePipeline();
774+
}

0 commit comments

Comments
 (0)