[Mesa-dev] [PATCH 6/9] i965/vec4: Use NIR remapping for VS attributes

Kenneth Graunke kenneth at whitecape.org
Fri May 5 23:30:29 UTC 2017


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 <apinheiro at 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.

> > 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170505/bf58ee84/attachment-0001.sig>


More information about the mesa-dev mailing list