[Mesa-dev] [PATCH v2] radv: fix passing clip/cull distances from VS to PS
Bas Nieuwenhuizen
bas at basnieuwenhuizen.nl
Thu Aug 30 11:43:33 UTC 2018
okay, r-b with that.
On Thu, Aug 30, 2018 at 1:39 PM Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
>
>
>
> On 8/30/18 12:30 PM, Bas Nieuwenhuizen wrote:
> > 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.
>
> Good point, Fixed locally.
>
> >
> >> + 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