[Mesa-dev] [PATCH v2] radv: fix passing clip/cull distances from VS to PS

Bas Nieuwenhuizen bas at basnieuwenhuizen.nl
Thu Aug 30 10:30:01 UTC 2018


On Wed, Aug 29, 2018 at 10:55 PM Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
>
> CTS doesn't test input clip/cull distances for the fragment
> shader stage, which explains why this was totally broken. I
> wrote a simple test locally that works now.
>
> This fixes a crash with GTA V and DXVK.
>
> Note that we are exporting unused parameters from the vertex
> shader now, but this can't be optimized easily because we don't
> keep the fragment shader info...
>
> Cc: mesa-stable at lists.freedesktop.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107477
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>  src/amd/vulkan/radv_nir_to_llvm.c | 30 +++++++++++++++++++++++++++++-
>  src/amd/vulkan/radv_pipeline.c    | 16 ++++++++++++++++
>  src/amd/vulkan/radv_shader.h      |  1 +
>  src/amd/vulkan/radv_shader_info.c |  4 ++++
>  4 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/src/amd/vulkan/radv_nir_to_llvm.c b/src/amd/vulkan/radv_nir_to_llvm.c
> index 4940e3230f..d7cd8cc069 100644
> --- a/src/amd/vulkan/radv_nir_to_llvm.c
> +++ b/src/amd/vulkan/radv_nir_to_llvm.c
> @@ -2098,9 +2098,10 @@ handle_fs_input_decl(struct radv_shader_context *ctx,
>         int idx = variable->data.location;
>         unsigned attrib_count = glsl_count_attribute_slots(variable->type, false);
>         LLVMValueRef interp = NULL;
> +       uint64_t mask;
>
>         variable->data.driver_location = idx * 4;
> -       ctx->input_mask |= ((1ull << attrib_count) - 1) << variable->data.location;
> +       mask = ((1ull << attrib_count) - 1) << variable->data.location;
>
>         if (glsl_get_base_type(glsl_without_array(variable->type)) == GLSL_TYPE_FLOAT) {
>                 unsigned interp_type;
> @@ -2121,6 +2122,15 @@ handle_fs_input_decl(struct radv_shader_context *ctx,
>         for (unsigned i = 0; i < attrib_count; ++i)
>                 ctx->inputs[ac_llvm_reg_index_soa(idx + i, 0)] = interp;
>
> +       if (idx == VARYING_SLOT_CLIP_DIST0) {
> +               /* Do not account for the number of components inside the array
> +                * of clip/cull distances because this might wrongly set other
> +                * bits like primitive ID or layer.
> +                */
> +               mask = 1ull << VARYING_SLOT_CLIP_DIST0;
> +       }
> +
> +       ctx->input_mask |= mask;
>  }
>
>  static void
> @@ -2187,6 +2197,17 @@ handle_fs_inputs(struct radv_shader_context *ctx,
>                         if (LLVMIsUndef(interp_param))
>                                 ctx->shader_info->fs.flat_shaded_mask |= 1u << index;
>                         ++index;
> +               } else if (i == VARYING_SLOT_CLIP_DIST0) {
> +                       int length = ctx->shader_info->info.ps.num_input_clips_culls;
> +
> +                       for (unsigned j = 0; j < length; j += 4) {
> +                               inputs = ctx->inputs + ac_llvm_reg_index_soa(i, j);
> +
> +                               interp_param = *inputs;
> +                               interp_fs_input(ctx, index, interp_param,
> +                                               ctx->abi.prim_mask, inputs);
> +                               ++index;
> +                       }
>                 } else if (i == VARYING_SLOT_POS) {
>                         for(int i = 0; i < 3; ++i)
>                                 inputs[i] = ctx->abi.frag_pos[i];
> @@ -2482,6 +2503,13 @@ handle_vs_outputs_post(struct radv_shader_context *ctx,
>                 memcpy(&pos_args[target - V_008DFC_SQ_EXP_POS],
>                        &args, sizeof(args));
>
> +               /* Export the clip/cull distances values to the next stage. */
> +               radv_export_param(ctx, param_count, &slots[0], 0xf);
> +               outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST0] = param_count++;
> +               if (ctx->num_output_clips + ctx->num_output_culls > 4) {
> +                       radv_export_param(ctx, param_count, &slots[4], 0xf);
> +                       outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST1] = param_count++;
> +               }
>         }
>
>         LLVMValueRef pos_values[4] = {ctx->ac.f32_0, ctx->ac.f32_0, ctx->ac.f32_0, ctx->ac.f32_1};
> diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
> index e63c481d1e..0303642d7e 100644
> --- a/src/amd/vulkan/radv_pipeline.c
> +++ b/src/amd/vulkan/radv_pipeline.c
> @@ -3052,6 +3052,22 @@ radv_pipeline_generate_ps_inputs(struct radeon_cmdbuf *cs,
>                 ps_offset++;
>         }
>
> +       if (ps->info.info.ps.num_input_clips_culls) {
> +               unsigned vs_offset;
> +
> +               vs_offset = outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST0];
> +               if (vs_offset != AC_EXP_PARAM_UNDEFINED) {
> +                       ps_input_cntl[ps_offset] = offset_to_ps_input(vs_offset, true);
> +                       ++ps_offset;
> +               }
> +
> +               vs_offset = outinfo->vs_output_param_offset[VARYING_SLOT_CLIP_DIST1];
> +               if (vs_offset != AC_EXP_PARAM_UNDEFINED) {

 &&  ps->info.info.ps.num_input_clips_culls > 4 ? Otherwise ps_offset
can be wrong when the PS only reads the first few.

> +                       ps_input_cntl[ps_offset] = offset_to_ps_input(vs_offset, true);
> +                       ++ps_offset;
> +               }
> +       }
> +
>         for (unsigned i = 0; i < 32 && (1u << i) <= ps->info.fs.input_mask; ++i) {
>                 unsigned vs_offset;
>                 bool flat_shade;
> diff --git a/src/amd/vulkan/radv_shader.h b/src/amd/vulkan/radv_shader.h
> index 03760b689c..897e2fc5e1 100644
> --- a/src/amd/vulkan/radv_shader.h
> +++ b/src/amd/vulkan/radv_shader.h
> @@ -174,6 +174,7 @@ struct radv_shader_info {
>                 bool has_pcoord;
>                 bool prim_id_input;
>                 bool layer_input;
> +               uint8_t num_input_clips_culls;
>         } ps;
>         struct {
>                 bool uses_grid_size;
> diff --git a/src/amd/vulkan/radv_shader_info.c b/src/amd/vulkan/radv_shader_info.c
> index 8026cca46c..a45c847c46 100644
> --- a/src/amd/vulkan/radv_shader_info.c
> +++ b/src/amd/vulkan/radv_shader_info.c
> @@ -341,6 +341,7 @@ static void
>  gather_info_input_decl_ps(const nir_shader *nir, const nir_variable *var,
>                           struct radv_shader_info *info)
>  {
> +       unsigned attrib_count = glsl_count_attribute_slots(var->type, false);
>         const struct glsl_type *type = glsl_without_array(var->type);
>         int idx = var->data.location;
>
> @@ -354,6 +355,9 @@ gather_info_input_decl_ps(const nir_shader *nir, const nir_variable *var,
>         case VARYING_SLOT_LAYER:
>                 info->ps.layer_input = true;
>                 break;
> +       case VARYING_SLOT_CLIP_DIST0:
> +               info->ps.num_input_clips_culls = attrib_count;
> +               break;
>         default:
>                 break;
>         }
> --
> 2.18.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list