On May 5, 2017 4:30:31 PM Kenneth Graunke <kenn...@whitecape.org> wrote:
On Friday, May 5, 2017 12:31:40 PM PDT Jason Ekstrand wrote:
On Fri, May 5, 2017 at 12:38 AM, Alejandro Piñeiro <apinhe...@igalia.com>
wrote:
> On 05/05/17 04:11, Jason Ekstrand wrote:
> > We have to pass inputs_read through from prog_data because we may add an
> > edge flag on old platforms.
>
> Well, the previous code was using nir->info->inputs_read. So perhaps
> this explanation should explicitly point that prog_data->inputs_read and
> nir->info->inputs_read they are not the same at that point (perhaps2,
> and why?)
>
Sure. How about:
We also change nir_lower_vs_inputs to take an explicit inputs_read bitmask
and pass in the inputs_read from prog_data instead from pulling it out of
NIR. This is because the version in prog_data may get EDGEFLAG added to it
on some old platforms.
Why don't you just make them the same? We're operating on a clone of
the nir_shader anyway, so we can whack nir->info.inputs_read however we
want. Then we can just copy it to the prog_data structure, and you
won't accidentally read the wrong one and screw up.
Because we don't clone the shader_info in nir_clone. :-( I've been
meaning to make nir_shader::info not a pointer. I guess i should just go
do that.
> Another nitpick and one question below.
>
> > ---
> > src/intel/compiler/brw_nir.c | 12 +++----
> > src/intel/compiler/brw_nir.h | 2 +-
> > src/intel/compiler/brw_vec4.cpp | 54
> +++++++++++-------------------
> > src/intel/compiler/brw_vec4_nir.cpp | 49
> ++-------------------------
> > src/intel/compiler/brw_vec4_visitor.cpp | 7 ++--
> > src/intel/compiler/brw_vec4_vs_visitor.cpp | 31 ++---------------
> > 6 files changed, 33 insertions(+), 122 deletions(-)
> >
> > diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
> > index 556782e..5ca532f 100644
> > --- a/src/intel/compiler/brw_nir.c
> > +++ b/src/intel/compiler/brw_nir.c
> > @@ -230,7 +230,7 @@ remap_patch_urb_offsets(nir_block *block,
> nir_builder *b,
> >
> > void
> > brw_nir_lower_vs_inputs(nir_shader *nir,
> > - bool is_scalar,
> > + uint64_t inputs_read,
> > bool use_legacy_snorm_formula,
> > const uint8_t *vs_attrib_wa_flags)
> > {
> > @@ -253,11 +253,7 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
> > brw_nir_apply_attribute_workarounds(nir, use_legacy_snorm_formula,
> > vs_attrib_wa_flags);
> >
> > - /* The last step is to remap VERT_ATTRIB_* to actual registers and
> we only
> > - * do that for scalar shaders at the moment.
> > - */
> > - if (!is_scalar)
> > - return;
> > + /* The last step is to remap VERT_ATTRIB_* to actual registers */
> >
> > const bool has_svgs =
> > nir->info->system_values_read &
> > @@ -266,7 +262,7 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
> > BITFIELD64_BIT(SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) |
> > BITFIELD64_BIT(SYSTEM_VALUE_INSTANCE_ID));
> >
> > - const unsigned num_inputs = _mesa_bitcount_64(nir->info->
> inputs_read);
> > + const unsigned num_inputs = _mesa_bitcount_64(inputs_read);
> >
> > nir_foreach_function(function, nir) {
> > if (!function->impl)
> > @@ -340,7 +336,7 @@ brw_nir_lower_vs_inputs(nir_shader *nir,
> > * before it and counting the bits.
> > */
> > int attr = nir_intrinsic_base(intrin);
> > - int slot = _mesa_bitcount_64(nir->info->inputs_read &
> > + int slot = _mesa_bitcount_64(inputs_read &
> > BITFIELD64_MASK(attr));
> > nir_intrinsic_set_base(intrin, slot);
> > break;
> > diff --git a/src/intel/compiler/brw_nir.h b/src/intel/compiler/brw_nir.h
> > index b96072c..3bba68d 100644
> > --- a/src/intel/compiler/brw_nir.h
> > +++ b/src/intel/compiler/brw_nir.h
> > @@ -98,7 +98,7 @@ nir_shader *brw_preprocess_nir(const struct
> brw_compiler *compiler,
> > bool brw_nir_lower_intrinsics(nir_shader *nir,
> > struct brw_stage_prog_data *prog_data);
> > void brw_nir_lower_vs_inputs(nir_shader *nir,
> > - bool is_scalar,
> > + uint64_t inputs_read,
> > bool use_legacy_snorm_formula,
> > const uint8_t *vs_attrib_wa_flags);
> > void brw_nir_lower_vue_inputs(nir_shader *nir, bool is_scalar,
With that suggestion, you can drop all of the above changes.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
