Skip to content

Commit

Permalink
layers: Sort stages for ValidateInterfaceBetweenStages
Browse files Browse the repository at this point in the history
  • Loading branch information
spencer-lunarg committed Aug 27, 2024
1 parent c3a7a71 commit 955dc2f
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 13 deletions.
71 changes: 58 additions & 13 deletions layers/core_checks/cc_shader_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -901,19 +901,64 @@ bool CoreChecks::ValidateGraphicsPipelineShaderState(const vvl::Pipeline &pipeli
create_info_loc);
}

for (size_t i = 1; i < pipeline.stage_states.size(); i++) {
const auto &producer = pipeline.stage_states[i - 1];
const auto &consumer = pipeline.stage_states[i];
const std::shared_ptr<const spirv::Module> &producer_spirv =
producer.spirv_state ? producer.spirv_state : producer.module_state->spirv;
const std::shared_ptr<const spirv::Module> &consumer_spirv =
consumer.spirv_state ? consumer.spirv_state : consumer.module_state->spirv;
if (&producer == fragment_stage) {
break;
}
if (consumer_spirv && producer_spirv && consumer.entrypoint && producer.entrypoint) {
skip |= ValidateInterfaceBetweenStages(*producer_spirv.get(), *producer.entrypoint, *consumer_spirv.get(),
*consumer.entrypoint, create_info_loc);
// We need to order the stages not how they are supplied in VkGraphicsPipelineCreateInfo::pStages but rather how they are
// chained together in the pipeline. Note, we could be in the PreRaster GPL path, so there may not be a Fragment Shader see
// https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8443
if (pipeline.stage_states.size() > 1) {
const size_t not_found = vvl::kU32Max;
auto get_stage = [&pipeline, not_found = not_found](VkShaderStageFlagBits stage) {
for (size_t i = 0; i < pipeline.stage_states.size(); i++) {
if (pipeline.stage_states[i].GetStage() == stage) {
return i;
}
}
return not_found;
};
// Two graphic pipeline paths will be
// Vert -> (Tess) -> (Geom) -> [Fragment]
// (Task) -> Mesh -> [Fragment]
const size_t total_stages = 8;
// Pack both paths in, works because fragment are the last stage always
const VkShaderStageFlagBits stage_list[total_stages] = {VK_SHADER_STAGE_VERTEX_BIT,
VK_SHADER_STAGE_TESSELLATION_CONTROL_BIT,
VK_SHADER_STAGE_TESSELLATION_EVALUATION_BIT,
VK_SHADER_STAGE_GEOMETRY_BIT,
VK_SHADER_STAGE_FRAGMENT_BIT,
VK_SHADER_STAGE_TASK_BIT_EXT,
VK_SHADER_STAGE_MESH_BIT_EXT,
VK_SHADER_STAGE_FRAGMENT_BIT};

// Use active_shaders because with GPL, you will need to check PreRaster and Fragment together at linking
const bool has_vertex_shader = pipeline.active_shaders & VK_SHADER_STAGE_VERTEX_BIT;
const bool has_task_shader = pipeline.active_shaders & VK_SHADER_STAGE_TASK_BIT_EXT;

size_t ordered_stages_index = has_vertex_shader ? 0 : has_task_shader ? 5 : 6;
size_t producer_index = get_stage(stage_list[ordered_stages_index++]);
assert(producer_index != not_found);

size_t consumer_index = not_found;
// start at 1 as we are always searching for the next consumer
for (size_t i = 1; i < pipeline.stage_states.size(); i++) {
// Find current producer's consumer
while (ordered_stages_index < total_stages) {
consumer_index = get_stage(stage_list[ordered_stages_index++]);
if (consumer_index != not_found) break;
}

const auto &producer = pipeline.stage_states[producer_index];
const auto &consumer = pipeline.stage_states[consumer_index];

const std::shared_ptr<const spirv::Module> &producer_spirv =
producer.spirv_state ? producer.spirv_state : producer.module_state->spirv;
const std::shared_ptr<const spirv::Module> &consumer_spirv =
consumer.spirv_state ? consumer.spirv_state : consumer.module_state->spirv;

if (consumer_spirv && producer_spirv && consumer.entrypoint && producer.entrypoint) {
skip |= ValidateInterfaceBetweenStages(*producer_spirv.get(), *producer.entrypoint, *consumer_spirv.get(),
*consumer.entrypoint, create_info_loc);
}

producer_index = consumer_index;
}
}

Expand Down
57 changes: 57 additions & 0 deletions tests/unit/geometry_tessellation_positive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,60 @@ TEST_F(PositiveGeometryTessellation, TessellationPointMode) {
pipe.tess_ci_ = tess_ci;
pipe.CreateGraphicsPipeline();
}

TEST_F(PositiveGeometryTessellation, InterfaceComponents) {
TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8443");
AddRequiredFeature(vkt::Feature::geometryShader);
RETURN_IF_SKIP(Init());
InitRenderTarget();

char const *vs_source = R"glsl(
#version 450
layout(location = 0) out ivec4 a;
void main() {
a = ivec4(1);
}
)glsl";

char const *geom_source = R"glsl(
#version 450
layout(triangles) in;
layout(triangle_strip) out;
layout(location = 0) in ivec4 a[3];
layout(location = 0) out vec4 b;
layout(max_vertices = 3) out;
void main() {
b = vec4(a[0]);
gl_Position = a[0];
EmitVertex();
b = vec4(a[1]);
gl_Position = a[1];
EmitVertex();
b = vec4(a[2]);
gl_Position = a[2];
EmitVertex();
EndPrimitive();
}
)glsl";

char const *fs_source = R"glsl(
#version 450
layout(location = 0) in vec4 b;
layout(location = 0) out vec4 c;
void main() {
c = b;
}
)glsl";

VkShaderObj vert(this, vs_source, VK_SHADER_STAGE_VERTEX_BIT);
VkShaderObj geom(this, geom_source, VK_SHADER_STAGE_GEOMETRY_BIT);
VkShaderObj frag(this, fs_source, VK_SHADER_STAGE_FRAGMENT_BIT);

CreatePipelineHelper pipe(*this);
pipe.shader_stages_ = {vert.GetStageCreateInfo(), frag.GetStageCreateInfo(), geom.GetStageCreateInfo()};
pipe.CreateGraphicsPipeline();
}
33 changes: 33 additions & 0 deletions tests/unit/shader_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,39 @@ TEST_F(NegativeShaderInterface, VsFsTypeMismatch) {
CreatePipelineHelper::OneshotTest(*this, set_info, kErrorBit, "VUID-RuntimeSpirv-OpEntryPoint-07754");
}

TEST_F(NegativeShaderInterface, VsFsTypeMismatch2) {
TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8443");

RETURN_IF_SKIP(Init());
InitRenderTarget();

char const *vsSource = R"glsl(
#version 450
layout(location=0) out int x;
void main(){
x = 0;
gl_Position = vec4(1);
}
)glsl";
char const *fsSource = R"glsl(
#version 450
layout(location=0) in float x; /* VS writes int */
layout(location=0) out vec4 color;
void main(){
color = vec4(x);
}
)glsl";

VkShaderObj vs(this, vsSource, VK_SHADER_STAGE_VERTEX_BIT);
VkShaderObj fs(this, fsSource, VK_SHADER_STAGE_FRAGMENT_BIT);

const auto set_info = [&](CreatePipelineHelper &helper) {
// Flipped here
helper.shader_stages_ = {fs.GetStageCreateInfo(), vs.GetStageCreateInfo()};
};
CreatePipelineHelper::OneshotTest(*this, set_info, kErrorBit, "VUID-RuntimeSpirv-OpEntryPoint-07754");
}

TEST_F(NegativeShaderInterface, VsFsTypeMismatchInBlock) {
TEST_DESCRIPTION(
"Test that an error is produced for mismatched types across the vertex->fragment shader interface, when the variable is "
Expand Down

0 comments on commit 955dc2f

Please sign in to comment.