On 23 October 2017 at 11:32, Bas Nieuwenhuizen <[email protected]> wrote: > 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.
Yeah seems like the best plan for now. Reviewed-by: Dave Airlie <[email protected]> > > Fixes: 0f9e32519bb 'ac/nir: clamp shadow texture comparison value on VI' > --- > 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.14.2 > > _______________________________________________ > mesa-dev mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
