From 0994aaaf51277598cfdc6fe8ea4e8f50f34bfd51 Mon Sep 17 00:00:00 2001 From: Danylo Piliaiev Date: Thu, 27 Jul 2023 14:38:43 +0200 Subject: [PATCH] radv: fix unused non-xfb shader outputs not being removed It was not taken into account that without Offset decoration the output is not written into XFB. Aside from eliminating more outputs this change prevents gl_PerVertex builtins generated by glslang from being kept alive in case when XFB is enabled. Keeping such outputs alive may upset a driver. VUID-StandaloneSpirv-Offset-04716: "Only variables or block members in the output interface decorated with Offset can be captured for transform feedback, and those variables or block members must also be decorated with XfbBuffer and XfbStride, or inherit XfbBuffer and XfbStride decorations from a block containing them" Additional info about glslang behavior could be found at: https://github.com/KhronosGroup/glslang/issues/1526 Fixes: e95531e101f0ba61d28195fe38414e411bf418b3 ("radv: fix gathering XFB info if there is dead outputs") Signed-off-by: Danylo Piliaiev Part-of: (cherry picked from commit 81407797b940dc97da808cde2f19fb59267d1f81) --- .pick_status.json | 2 +- src/amd/vulkan/radv_shader.c | 41 ++++++++++++++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index eecb507f80a..00d892cb0b6 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -32224,7 +32224,7 @@ "description": "radv: fix unused non-xfb shader outputs not being removed", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "e95531e101f0ba61d28195fe38414e411bf418b3" }, diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c index b874ee880d6..be711da0010 100644 --- a/src/amd/vulkan/radv_shader.c +++ b/src/amd/vulkan/radv_shader.c @@ -334,8 +334,45 @@ is_not_xfb_output(nir_variable *var, void *data) if (var->data.mode != nir_var_shader_out) return true; - return !var->data.explicit_xfb_buffer && - !var->data.explicit_xfb_stride; + /* From the Vulkan 1.3.259 spec: + * + * VUID-StandaloneSpirv-Offset-04716 + * + * "Only variables or block members in the output interface decorated + * with Offset can be captured for transform feedback, and those + * variables or block members must also be decorated with XfbBuffer + * and XfbStride, or inherit XfbBuffer and XfbStride decorations from + * a block containing them" + * + * glslang generates gl_PerVertex builtins when they are not declared, + * enabled XFB should not prevent them from being DCE'd. + * + * The logic should match nir_gather_xfb_info_with_varyings + */ + + if (!var->data.explicit_xfb_buffer) + return true; + + bool is_array_block = var->interface_type != NULL && + glsl_type_is_array(var->type) && + glsl_without_array(var->type) == var->interface_type; + + if (!is_array_block) { + return !var->data.explicit_offset; + } else { + /* For array of blocks we have to check each element */ + unsigned aoa_size = glsl_get_aoa_size(var->type); + const struct glsl_type *itype = var->interface_type; + unsigned nfields = glsl_get_length(itype); + for (unsigned b = 0; b < aoa_size; b++) { + for (unsigned f = 0; f < nfields; f++) { + if (glsl_get_struct_field_offset(itype, f) >= 0) + return false; + } + } + + return true; + } } nir_shader *