From 92f553bcff4bd03dfa2f96f01ea62450c63c0d0b Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Tue, 6 May 2025 14:31:26 -0400 Subject: [PATCH] vtn: remove spurious texel buffer warning The spec text here is: Image Type must be an OpTypeImage. It is the type of the image in the combined sampler and image type. It must not have a Dim of SubpassData. Additionally, starting with version 1.6, it must not have a Dim of Buffer. For older SPIR-V versions, there is no analogous requirement. It is implicitly valid to use a Dim of Buffer (even though it doesn't make much sense). Should apps do it anyway? Probably not, but it doesn't matter and they do. glslang considers this requirement relevant only for 1.6+: if (glslangIntermediate->getSpv().spv >= glslang::EShTargetSpv_1_6 && texType.getSampler().isBuffer()) { // SamplerBuffer is not supported in spirv1.6 so // `samplerBuffer(textureBuffer, sampler)` is a no-op // and textureBuffer is the result going forward constructed = arguments[0]; } else constructed = builder.createOp(spv::OpSampledImage, resultType(), arguments); That means SPIR-V with an older declared version will warn even with a glslang new enough to know about the 1.6 requirement. That includes a *lot* of SPIR-V's built with the CTS. I see no compelling reason to keep the warning for older than 1.6. Removing the spurious warning silences a *huge* amount of noise from dEQP-VK (plus a bit from KHR-GL46). In exchange I see very little tradeoff, it's not really our job to lint for best practices not in the spec. I see two viable options: 1. Try to convince the whole ecosystem outside of Mesa to pivot to our pedantic reading of the spec and get them to update all the old SPIR-V binaries in the wild, in the case of CTS being changed at the glslang level and then trickling down into CTS. 2. Merge this patch, simplifying Mesa and immediately forget about this forever. I'm spending all my FOSS political capital on kernel upstreaming so I have a strong preference for #2, aka hitting Marge on this MR and then moving on with all of our lives. ("Ignore the problem and make deqp-runner annoying to use" is the secret 3rd option I'd rather not do.) Signed-off-by: Alyssa Rosenzweig Reviewed-by: Faith Ekstrand Reviewed-by: Mike Blumenkrantz Part-of: --- src/compiler/spirv/spirv_to_nir.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/compiler/spirv/spirv_to_nir.c b/src/compiler/spirv/spirv_to_nir.c index 96b929dbfa9..05ccce54db6 100644 --- a/src/compiler/spirv/spirv_to_nir.c +++ b/src/compiler/spirv/spirv_to_nir.c @@ -1837,14 +1837,9 @@ validate_image_type_for_sampled_image(struct vtn_builder *b, dim == GLSL_SAMPLER_DIM_SUBPASS_MS, "%s must not have a Dim of SubpassData.", operand); - if (dim == GLSL_SAMPLER_DIM_BUF) { - if (b->version >= 0x10600) { - vtn_fail("Starting with SPIR-V 1.6, %s " - "must not have a Dim of Buffer.", operand); - } else { - vtn_warn("%s should not have a Dim of Buffer.", operand); - } - } + vtn_fail_if(dim == GLSL_SAMPLER_DIM_BUF && b->version >= 0x10600, + "Starting with SPIR-V 1.6, %s must not have a Dim of Buffer.", + operand); } static void