[Mesa-dev] [PATCH] nir/gather_info: Handle double inputs in set_io_mask

[Jason Ekstrand]

Ugh... This patch, though obviously correct if taken on its own, doesn't
wok.  The double_inputs_read field is more subtle and bonkers than I
thought.

On Sat, Dec 2, 2017 at 9:32 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:

> It's a little sketchy to put the logic in set_io_mask but due to a
> detail of the GLSL spec, it's safe to check for 64-bit without knowing
> the full access chain.  This fixes a bug where try_mask_partial_io
> succeeded at marking a portion of the input as used but then we came
> along and marked the whole thing in double_inputs_read.  By putting it
> in set_io_mask, we get the correct flagging even for partial reads.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103241
> Cc: mesa-stable at lists.freedesktop.org
> Cc: Timothy Arceri <tarceri at itsqueeze.com>
> Cc: Alejandro Piñeiro <apinheiro at igalia.com>
> ---
>  src/compiler/nir/nir_gather_info.c | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/src/compiler/nir/nir_gather_info.c
> b/src/compiler/nir/nir_gather_info.c
> index 9469396..bab37f3 100644
> --- a/src/compiler/nir/nir_gather_info.c
> +++ b/src/compiler/nir/nir_gather_info.c
> @@ -54,6 +54,23 @@ set_io_mask(nir_shader *shader, nir_variable *var, int
> offset, int len,
>           else
>              shader->info.inputs_read |= bitfield;
>
> +         /* From the GLSL 4.60 spec, section 4.3.4 "Input Variables":
> +          *
> +          *    It is a compile-time error to declare a vertex shader input
> +          *    containing any of the  following:
> +          *
> +          *     - A Boolean type (bool, bvec2, bvec3, bvec4)
> +          *     - An opaque type
> +          *     - A structure
> +          *
> +          * Therefore, when looking for 64-bit attributes, we need only
> +          * consider arrays of vectors.
> +          */
> +         if (shader->info.stage == MESA_SHADER_VERTEX &&
> +             glsl_type_is_dual_slot(glsl_without_array(var->type))) {
> +            shader->info.double_inputs_read |= bitfield;
> +         }
> +
>           if (shader->info.stage == MESA_SHADER_FRAGMENT) {
>              shader->info.fs.uses_sample_qualifier |= var->data.sample;
>           }
> @@ -226,17 +243,6 @@ gather_intrinsic_info(nir_intrinsic_instr *instr,
> nir_shader *shader)
>
>           if (!try_mask_partial_io(shader, instr->variables[0],
> is_output_read))
>              mark_whole_variable(shader, var, is_output_read);
> -
> -         /* We need to track which input_reads bits correspond to a
> -          * dvec3/dvec4 input attribute */
> -         if (shader->info.stage == MESA_SHADER_VERTEX &&
> -             var->data.mode == nir_var_shader_in &&
> -             glsl_type_is_dual_slot(glsl_without_array(var->type))) {
> -            for (uint i = 0; i < glsl_count_attribute_slots(var->type,
> false); i++) {
> -               int idx = var->data.location + i;
> -               shader->info.double_inputs_read |= BITFIELD64_BIT(idx);
> -            }
> -         }
>        }
>        break;
>     }
> --
> 2.5.0.400.gff86faf
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171202/a6520415/attachment.html>