From a548b727a147b4ead1accef10499c4cf81af3057 Mon Sep 17 00:00:00 2001 From: Bas Nieuwenhuizen Date: Mon, 23 Oct 2017 03:27:28 +0200 Subject: [PATCH] ac/nir: Only clamp shadow reference on radeonsi. Vulkan CTS does not expect the value to be clamped (at least for D32), and it makes a differences even though depth is in [0,1], due to strict inequalities. I couldn't find anything in the Vulkan spec about this, but the test seemed to be copied from GL tests and the GL spec only specifies clamping for fixed point formats. Hence I expect radeonsi to run into this at some point as well, but given that they still have a usecase with the Z16->Z32 promotion, I'll leave that for someone else to clean up. This at least fixes radv dEQP-VK.texture.shadow.* on VI. Fixes: 0f9e32519bb 'ac/nir: clamp shadow texture comparison value on VI' Reviewed-by: Dave Airlie --- src/amd/common/ac_nir_to_llvm.c | 5 +++-- src/amd/common/ac_nir_to_llvm.h | 1 + src/amd/common/ac_shader_abi.h | 4 ++++ src/gallium/drivers/radeonsi/si_shader_nir.c | 1 + 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c index 5e5a46a21f4..2ddc748b5be 100644 --- a/src/amd/common/ac_nir_to_llvm.c +++ b/src/amd/common/ac_nir_to_llvm.c @@ -4652,14 +4652,14 @@ static void visit_tex(struct ac_nir_context *ctx, nir_tex_instr *instr) LLVMValueRef z = ac_to_float(&ctx->ac, llvm_extract_elem(&ctx->ac, comparator, 0)); - /* TC-compatible HTILE promotes Z16 and Z24 to Z32_FLOAT, + /* TC-compatible HTILE on radeonsi promotes Z16 and Z24 to Z32_FLOAT, * so the depth comparison value isn't clamped for Z16 and * Z24 anymore. Do it manually here. * * It's unnecessary if the original texture format was * Z32_FLOAT, but we don't know that here. */ - if (ctx->ac.chip_class == VI) + if (ctx->ac.chip_class == VI && ctx->abi->clamp_shadow_reference) z = ac_build_clamp(&ctx->ac, z); address[count++] = z; @@ -6600,6 +6600,7 @@ LLVMModuleRef ac_translate_nir_to_llvm(LLVMTargetMachineRef tm, ctx.abi.emit_outputs = handle_shader_outputs_post; ctx.abi.load_ssbo = radv_load_ssbo; ctx.abi.load_sampler_desc = radv_get_sampler_desc; + ctx.abi.clamp_shadow_reference = false; if (shader_count >= 2) ac_init_exec_full_mask(&ctx.ac); diff --git a/src/amd/common/ac_nir_to_llvm.h b/src/amd/common/ac_nir_to_llvm.h index 9579aeeefef..1d9ec8ce8b0 100644 --- a/src/amd/common/ac_nir_to_llvm.h +++ b/src/amd/common/ac_nir_to_llvm.h @@ -80,6 +80,7 @@ struct ac_nir_compiler_options { struct ac_shader_variant_key key; bool unsafe_math; bool supports_spill; + bool clamp_shadow_reference; enum radeon_family family; enum chip_class chip_class; }; diff --git a/src/amd/common/ac_shader_abi.h b/src/amd/common/ac_shader_abi.h index 5f296be0c1f..14517d55700 100644 --- a/src/amd/common/ac_shader_abi.h +++ b/src/amd/common/ac_shader_abi.h @@ -88,6 +88,10 @@ struct ac_shader_abi { LLVMValueRef index, enum ac_descriptor_type desc_type, bool image, bool write); + + /* Whether to clamp the shadow reference value to [0,1]on VI. Radeonsi currently + * uses it due to promoting D16 to D32, but radv needs it off. */ + bool clamp_shadow_reference; }; #endif /* AC_SHADER_ABI_H */ diff --git a/src/gallium/drivers/radeonsi/si_shader_nir.c b/src/gallium/drivers/radeonsi/si_shader_nir.c index a2d175364f8..e186661caf3 100644 --- a/src/gallium/drivers/radeonsi/si_shader_nir.c +++ b/src/gallium/drivers/radeonsi/si_shader_nir.c @@ -498,6 +498,7 @@ bool si_nir_build_llvm(struct si_shader_context *ctx, struct nir_shader *nir) ctx->abi.inputs = &ctx->inputs[0]; ctx->abi.load_sampler_desc = si_nir_load_sampler_desc; + ctx->abi.clamp_shadow_reference = true; ctx->num_samplers = util_last_bit(info->samplers_declared); ctx->num_images = util_last_bit(info->images_declared); -- 2.30.2