Skip to content

Commit 1ee7afc

Browse files
spencer-lunargspencer-lunarg
committed
gpuav: Fix ShaderObject hitting descriptor set limit
1 parent 7596e62 commit 1ee7afc

2 files changed

Lines changed: 134 additions & 28 deletions

File tree

‎layers/gpuav/instrumentation/gpuav_shader_instrumentor.cpp‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -239,9 +239,7 @@ void GpuShaderInstrumentor::PreCallRecordCreatePipelineLayout(VkDevice device, c
239239
strm << "pCreateInfo::setLayoutCount (" << chassis_state.modified_create_info.setLayoutCount
240240
<< ") will conflicts with validation's descriptor set at slot " << instrumentation_desc_set_bind_index_ << ". "
241241
<< "This Pipeline Layout has too many descriptor sets that will not allow GPU shader instrumentation to be setup "
242-
"for "
243-
"pipelines created with it, therefor no validation error will be repored for them by GPU-AV at "
244-
"runtime.";
242+
"for pipelines created with it, therefore no validation error will be repored for them by GPU-AV at runtime.";
245243
InternalWarning(device, record_obj.location, strm.str().c_str());
246244
} else {
247245
// Modify the pipeline layout by:
@@ -378,7 +376,7 @@ void GpuShaderInstrumentor::PreCallRecordCreateShadersEXT(VkDevice device, uint3
378376
strm << "pCreateInfos[" << i << "]::setLayoutCount (" << new_create_info.setLayoutCount
379377
<< ") will conflicts with validation's descriptor set at slot " << instrumentation_desc_set_bind_index_ << ". "
380378
<< "This Shader Object has too many descriptor sets that will not allow GPU shader instrumentation to be setup "
381-
"for VkShaderEXT created with it, therefor no validation error will be repored for them by GPU-AV at "
379+
"for VkShaderEXT created with it, therefore no validation error will be repored for them by GPU-AV at "
382380
"runtime.";
383381
InternalWarning(device, record_obj.location, strm.str().c_str());
384382
} else {
@@ -402,10 +400,10 @@ void GpuShaderInstrumentor::PreCallRecordCreateShadersEXT(VkDevice device, uint3
402400
new_create_info.pSetLayouts[k] = dummy_desc_layout_;
403401
}
404402
new_create_info.pSetLayouts[instrumentation_desc_set_bind_index_] = instrumentation_desc_layout_;
405-
}
406403

407-
chassis_state.is_modified |=
408-
PreCallRecordShaderObjectInstrumentation(new_create_info, create_info_loc, instrumentation_data);
404+
chassis_state.is_modified |=
405+
PreCallRecordShaderObjectInstrumentation(new_create_info, create_info_loc, instrumentation_data);
406+
}
409407
}
410408

411409
chassis_state.pCreateInfos = reinterpret_cast<VkShaderCreateInfoEXT *>(chassis_state.modified_create_infos.data());

‎tests/unit/debug_printf.cpp‎

Lines changed: 129 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2853,8 +2853,7 @@ TEST_F(NegativeDebugPrintf, UseAllDescriptorSlotsPipelineGPL) {
28532853
}
28542854
}
28552855

2856-
// TODO - https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/7178
2857-
TEST_F(NegativeDebugPrintf, DISABLED_UseAllDescriptorSlotsShaderObjectReserved) {
2856+
TEST_F(NegativeDebugPrintf, UseAllDescriptorSlotsShaderObjectReserved) {
28582857
TEST_DESCRIPTION("Reserve a descriptor slot and proceed to use them all anyway so debug printf can't");
28592858
AddRequiredExtensions(VK_EXT_SHADER_OBJECT_EXTENSION_NAME);
28602859
AddRequiredFeature(vkt::Feature::shaderObject);
@@ -2877,7 +2876,7 @@ TEST_F(NegativeDebugPrintf, DISABLED_UseAllDescriptorSlotsShaderObjectReserved)
28772876
OneOffDescriptorSet descriptor_set(m_device, {{0, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr}});
28782877
std::vector<VkDescriptorSetLayout> layouts;
28792878
for (uint32_t i = 0; i < set_limit; i++) {
2880-
layouts.push_back(descriptor_set.layout_.handle());
2879+
layouts.push_back(descriptor_set.layout_);
28812880
}
28822881

28832882
// First try to use too many sets in the Shader Object
@@ -2892,7 +2891,7 @@ TEST_F(NegativeDebugPrintf, DISABLED_UseAllDescriptorSlotsShaderObjectReserved)
28922891

28932892
m_command_buffer.Begin();
28942893
m_command_buffer.BindCompShader(comp_shader);
2895-
vk::CmdDispatch(m_command_buffer.handle(), 1, 1, 1);
2894+
vk::CmdDispatch(m_command_buffer, 1, 1, 1);
28962895
m_command_buffer.End();
28972896

28982897
// Will not print out because no slot was possible to put output buffer
@@ -2901,12 +2900,21 @@ TEST_F(NegativeDebugPrintf, DISABLED_UseAllDescriptorSlotsShaderObjectReserved)
29012900

29022901
// Reduce by one (so there is room now) and print something
29032902
{
2903+
uint32_t under_set_limit = set_limit - 1;
2904+
std::vector<const vkt::DescriptorSetLayout *> vkt_layouts;
2905+
for (uint32_t i = 0; i < under_set_limit; i++) {
2906+
vkt_layouts.push_back(&descriptor_set.layout_);
2907+
}
2908+
vkt::PipelineLayout pipe_layout(*m_device, vkt_layouts);
2909+
29042910
const vkt::Shader comp_shader(*m_device,
2905-
ShaderCreateInfo(cs_spirv, VK_SHADER_STAGE_COMPUTE_BIT, set_limit - 1, layouts.data()));
2911+
ShaderCreateInfo(cs_spirv, VK_SHADER_STAGE_COMPUTE_BIT, under_set_limit, layouts.data()));
29062912

29072913
m_command_buffer.Begin();
29082914
m_command_buffer.BindCompShader(comp_shader);
2909-
vk::CmdDispatch(m_command_buffer.handle(), 1, 1, 1);
2915+
vk::CmdBindDescriptorSets(m_command_buffer, VK_PIPELINE_BIND_POINT_COMPUTE, pipe_layout, 0, 1, &descriptor_set.set_, 0,
2916+
nullptr);
2917+
vk::CmdDispatch(m_command_buffer, 1, 1, 1);
29102918
m_command_buffer.End();
29112919

29122920
m_errorMonitor->SetDesiredInfo("float == 3.141500");
@@ -2915,8 +2923,7 @@ TEST_F(NegativeDebugPrintf, DISABLED_UseAllDescriptorSlotsShaderObjectReserved)
29152923
}
29162924
}
29172925

2918-
// TODO - https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/7178
2919-
TEST_F(NegativeDebugPrintf, DISABLED_UseAllDescriptorSlotsShaderObjectNotReserved) {
2926+
TEST_F(NegativeDebugPrintf, UseAllDescriptorSlotsShaderObjectNotReserved) {
29202927
TEST_DESCRIPTION("Dont reserve a descriptor slot and proceed to use them all anyway so debug printf can't");
29212928
AddRequiredExtensions(VK_EXT_SHADER_OBJECT_EXTENSION_NAME);
29222929
AddRequiredFeature(vkt::Feature::shaderObject);
@@ -2938,7 +2945,7 @@ TEST_F(NegativeDebugPrintf, DISABLED_UseAllDescriptorSlotsShaderObjectNotReserve
29382945
OneOffDescriptorSet descriptor_set(m_device, {{0, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr}});
29392946
std::vector<VkDescriptorSetLayout> layouts;
29402947
for (uint32_t i = 0; i < set_limit; i++) {
2941-
layouts.push_back(descriptor_set.layout_.handle());
2948+
layouts.push_back(descriptor_set.layout_);
29422949
}
29432950

29442951
// First try to use too many sets in the Shader Object
@@ -2953,7 +2960,7 @@ TEST_F(NegativeDebugPrintf, DISABLED_UseAllDescriptorSlotsShaderObjectNotReserve
29532960

29542961
m_command_buffer.Begin();
29552962
m_command_buffer.BindCompShader(comp_shader);
2956-
vk::CmdDispatch(m_command_buffer.handle(), 1, 1, 1);
2963+
vk::CmdDispatch(m_command_buffer, 1, 1, 1);
29572964
m_command_buffer.End();
29582965

29592966
// Will not print out because no slot was possible to put output buffer
@@ -2962,12 +2969,21 @@ TEST_F(NegativeDebugPrintf, DISABLED_UseAllDescriptorSlotsShaderObjectNotReserve
29622969

29632970
// Reduce by one (so there is room now) and print something
29642971
{
2972+
uint32_t under_set_limit = set_limit - 1;
2973+
std::vector<const vkt::DescriptorSetLayout *> vkt_layouts;
2974+
for (uint32_t i = 0; i < under_set_limit; i++) {
2975+
vkt_layouts.push_back(&descriptor_set.layout_);
2976+
}
2977+
vkt::PipelineLayout pipe_layout(*m_device, vkt_layouts);
2978+
29652979
const vkt::Shader comp_shader(*m_device,
29662980
ShaderCreateInfo(cs_spirv, VK_SHADER_STAGE_COMPUTE_BIT, set_limit - 1, layouts.data()));
29672981

29682982
m_command_buffer.Begin();
29692983
m_command_buffer.BindCompShader(comp_shader);
2970-
vk::CmdDispatch(m_command_buffer.handle(), 1, 1, 1);
2984+
vk::CmdBindDescriptorSets(m_command_buffer, VK_PIPELINE_BIND_POINT_COMPUTE, pipe_layout, 0, 1, &descriptor_set.set_, 0,
2985+
nullptr);
2986+
vk::CmdDispatch(m_command_buffer, 1, 1, 1);
29712987
m_command_buffer.End();
29722988

29732989
m_errorMonitor->SetDesiredInfo("float == 3.141500");
@@ -2976,8 +2992,7 @@ TEST_F(NegativeDebugPrintf, DISABLED_UseAllDescriptorSlotsShaderObjectNotReserve
29762992
}
29772993
}
29782994

2979-
// TODO - https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/7178
2980-
TEST_F(NegativeDebugPrintf, DISABLED_ShaderObjectMultiCreate) {
2995+
TEST_F(NegativeDebugPrintf, ShaderObjectMultiCreate) {
29812996
TEST_DESCRIPTION("Make sure we instrument every index of VkShaderCreateInfoEXT");
29822997
AddRequiredExtensions(VK_EXT_SHADER_OBJECT_EXTENSION_NAME);
29832998
AddRequiredExtensions(VK_KHR_DYNAMIC_RENDERING_EXTENSION_NAME);
@@ -2987,6 +3002,19 @@ TEST_F(NegativeDebugPrintf, DISABLED_ShaderObjectMultiCreate) {
29873002
RETURN_IF_SKIP(InitState());
29883003
InitDynamicRenderTarget();
29893004

3005+
char const *vs_source = R"glsl(
3006+
#version 450
3007+
#extension GL_EXT_debug_printf : enable
3008+
vec2 vertices[3];
3009+
void main(){
3010+
vertices[0] = vec2(-1.0, -1.0);
3011+
vertices[1] = vec2( 1.0, -1.0);
3012+
vertices[2] = vec2( 0.0, 1.0);
3013+
debugPrintfEXT("vertex == %u", gl_VertexIndex);
3014+
gl_Position = vec4(vertices[gl_VertexIndex % 3], 0.0, 1.0);
3015+
}
3016+
)glsl";
3017+
29903018
char const *fs_source = R"glsl(
29913019
#version 450
29923020
#extension GL_EXT_debug_printf : enable
@@ -2996,7 +3024,7 @@ TEST_F(NegativeDebugPrintf, DISABLED_ShaderObjectMultiCreate) {
29963024
}
29973025
)glsl";
29983026

2999-
const auto vert_spv = GLSLToSPV(VK_SHADER_STAGE_VERTEX_BIT, kVertexDrawPassthroughGlsl);
3027+
const auto vert_spv = GLSLToSPV(VK_SHADER_STAGE_VERTEX_BIT, vs_source);
30003028
const auto frag_spv = GLSLToSPV(VK_SHADER_STAGE_FRAGMENT_BIT, fs_source);
30013029

30023030
VkShaderCreateInfoEXT shader_create_infos[2];
@@ -3006,20 +3034,23 @@ TEST_F(NegativeDebugPrintf, DISABLED_ShaderObjectMultiCreate) {
30063034
VkShaderEXT shaders[2];
30073035
vk::CreateShadersEXT(m_device->handle(), 2, shader_create_infos, nullptr, shaders);
30083036

3009-
VkRenderingInfo renderingInfo = vku::InitStructHelper();
3010-
renderingInfo.colorAttachmentCount = 0;
3011-
renderingInfo.layerCount = 1;
3012-
renderingInfo.renderArea = {{0, 0}, {1, 1}};
3037+
VkRenderingInfo rendering_info = vku::InitStructHelper();
3038+
rendering_info.colorAttachmentCount = 0;
3039+
rendering_info.layerCount = 1;
3040+
rendering_info.renderArea = {{0, 0}, {1, 1}};
30133041

30143042
m_command_buffer.Begin();
3015-
m_command_buffer.BeginRendering(renderingInfo);
3043+
m_command_buffer.BeginRendering(rendering_info);
30163044
SetDefaultDynamicStatesExclude({VK_DYNAMIC_STATE_COLOR_BLEND_ENABLE_EXT});
30173045
const VkShaderStageFlagBits stages[] = {VK_SHADER_STAGE_VERTEX_BIT, VK_SHADER_STAGE_FRAGMENT_BIT};
3018-
vk::CmdBindShadersEXT(m_command_buffer.handle(), 2, stages, shaders);
3019-
vk::CmdDraw(m_command_buffer.handle(), 3, 1, 0, 0);
3046+
vk::CmdBindShadersEXT(m_command_buffer, 2, stages, shaders);
3047+
vk::CmdDraw(m_command_buffer, 3, 1, 0, 0);
30203048
m_command_buffer.EndRendering();
30213049
m_command_buffer.End();
30223050

3051+
m_errorMonitor->SetDesiredInfo("vertex == 0");
3052+
m_errorMonitor->SetDesiredInfo("vertex == 1");
3053+
m_errorMonitor->SetDesiredInfo("vertex == 2");
30233054
m_errorMonitor->SetDesiredInfo("float == 3.141500");
30243055
m_default_queue->SubmitAndWait(m_command_buffer);
30253056
m_errorMonitor->VerifyFound();
@@ -3029,6 +3060,83 @@ TEST_F(NegativeDebugPrintf, DISABLED_ShaderObjectMultiCreate) {
30293060
}
30303061
}
30313062

3063+
TEST_F(NegativeDebugPrintf, ShaderObjectBoundDescriptor) {
3064+
AddRequiredExtensions(VK_EXT_SHADER_OBJECT_EXTENSION_NAME);
3065+
AddRequiredFeature(vkt::Feature::shaderObject);
3066+
RETURN_IF_SKIP(InitDebugPrintfFramework());
3067+
RETURN_IF_SKIP(InitState());
3068+
m_errorMonitor->ExpectSuccess(kErrorBit | kWarningBit | kInformationBit);
3069+
3070+
char const *shader_source = R"glsl(
3071+
#version 450
3072+
#extension GL_EXT_debug_printf : enable
3073+
layout(set = 0, binding = 0) buffer SSBO { uint x; };
3074+
void main() {
3075+
debugPrintfEXT("x is undefined %u", x);
3076+
}
3077+
)glsl";
3078+
auto cs_spirv = GLSLToSPV(VK_SHADER_STAGE_COMPUTE_BIT, shader_source);
3079+
3080+
vkt::Buffer storage_buffer(*m_device, 4, VK_BUFFER_USAGE_STORAGE_BUFFER_BIT);
3081+
OneOffDescriptorSet descriptor_set(m_device, {{0, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr}});
3082+
const vkt::PipelineLayout pipeline_layout(*m_device, {&descriptor_set.layout_});
3083+
descriptor_set.WriteDescriptorBufferInfo(0, storage_buffer, 0, VK_WHOLE_SIZE, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
3084+
descriptor_set.UpdateDescriptorSets();
3085+
3086+
const vkt::Shader comp_shader(*m_device,
3087+
ShaderCreateInfo(cs_spirv, VK_SHADER_STAGE_COMPUTE_BIT, 1, &descriptor_set.layout_.handle()));
3088+
m_command_buffer.Begin();
3089+
m_command_buffer.BindCompShader(comp_shader);
3090+
vk::CmdBindDescriptorSets(m_command_buffer, VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout, 0, 1, &descriptor_set.set_, 0,
3091+
nullptr);
3092+
vk::CmdDispatch(m_command_buffer, 1, 1, 1);
3093+
m_command_buffer.End();
3094+
3095+
m_errorMonitor->SetDesiredInfo("x is undefined");
3096+
m_default_queue->SubmitAndWait(m_command_buffer);
3097+
m_errorMonitor->VerifyFound();
3098+
}
3099+
3100+
TEST_F(NegativeDebugPrintf, ShaderObjectUnusedBoundDescriptor) {
3101+
AddRequiredExtensions(VK_EXT_SHADER_OBJECT_EXTENSION_NAME);
3102+
AddRequiredFeature(vkt::Feature::shaderObject);
3103+
RETURN_IF_SKIP(InitDebugPrintfFramework());
3104+
RETURN_IF_SKIP(InitState());
3105+
m_errorMonitor->ExpectSuccess(kErrorBit | kWarningBit | kInformationBit);
3106+
3107+
char const *shader_source = R"glsl(
3108+
#version 450
3109+
#extension GL_EXT_debug_printf : enable
3110+
layout(set = 0, binding = 0) buffer SSBO { uint x; };
3111+
void main() {
3112+
debugPrintfEXT("x is undefined %u", x);
3113+
}
3114+
)glsl";
3115+
auto cs_spirv = GLSLToSPV(VK_SHADER_STAGE_COMPUTE_BIT, shader_source);
3116+
3117+
vkt::Buffer storage_buffer(*m_device, 4, VK_BUFFER_USAGE_STORAGE_BUFFER_BIT);
3118+
OneOffDescriptorSet descriptor_set0(m_device, {{0, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr}});
3119+
OneOffDescriptorSet descriptor_set1(m_device, {{0, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr}});
3120+
const vkt::PipelineLayout pipeline_layout(*m_device, {&descriptor_set0.layout_, &descriptor_set1.layout_});
3121+
descriptor_set0.WriteDescriptorBufferInfo(0, storage_buffer, 0, VK_WHOLE_SIZE, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
3122+
descriptor_set0.UpdateDescriptorSets();
3123+
descriptor_set1.WriteDescriptorBufferInfo(0, storage_buffer, 0, VK_WHOLE_SIZE, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
3124+
descriptor_set1.UpdateDescriptorSets();
3125+
3126+
VkDescriptorSetLayout layouts[2] = {descriptor_set0.layout_, descriptor_set1.layout_};
3127+
const vkt::Shader comp_shader(*m_device, ShaderCreateInfo(cs_spirv, VK_SHADER_STAGE_COMPUTE_BIT, 2, layouts));
3128+
m_command_buffer.Begin();
3129+
m_command_buffer.BindCompShader(comp_shader);
3130+
vk::CmdBindDescriptorSets(m_command_buffer, VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout, 0, 1, &descriptor_set0.set_, 0,
3131+
nullptr);
3132+
vk::CmdDispatch(m_command_buffer, 1, 1, 1);
3133+
m_command_buffer.End();
3134+
3135+
m_errorMonitor->SetDesiredInfo("x is undefined");
3136+
m_default_queue->SubmitAndWait(m_command_buffer);
3137+
m_errorMonitor->VerifyFound();
3138+
}
3139+
30323140
TEST_F(NegativeDebugPrintf, OverflowBuffer) {
30333141
TEST_DESCRIPTION("go over the default VK_LAYER_PRINTF_BUFFER_SIZE limit");
30343142
RETURN_IF_SKIP(InitDebugPrintfFramework());

0 commit comments

Comments
 (0)