[Mesa-dev] [PATCH 26/26] nir/dead_variables: Configurably work with any variable mode

[Jason Ekstrand]

On Sat, Mar 26, 2016 at 3:00 PM, Rob Clark <robdclark at gmail.com> wrote:

> On Sat, Mar 26, 2016 at 5:43 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> > On Sat, Mar 26, 2016 at 8:22 AM, Rob Clark <robdclark at gmail.com> wrote:
> >> btw, I do remember for lower_io I wanted the mode to be a bitmask so
> >> we could more easily deal w/ multiple modes in the same pass, but that
> >> exceeded # of bits somewhere or other.  I suppose maybe nir_var_all is
> >> good enough..  offhand I can't think of a scenario where we want to
> >> lower some but not all..
> >
> >
> > I'd rather do that too.  I don't remember why we didn't before (too lazy
> to
> > search the mailing list).  It shouldn't overflow a uint32_t, there are
> only
> > 9 modes last I checked.  We may add more, but it'll be a while before we
> add
> > 22 more.
>
> I dug it up, and the issue is in nir_variable_data:
>
>       nir_variable_mode mode:5;
>
> ofc we could just give more bits to that and be done...
>

Adding connor to the Cc in case he has an opinion...

I think that specifying multiple modes is something that will happen often
enough that we should just standardize it.  I see three different
possibilities here:

 1) nir_lower_io takes a nir_var_all enum value which is -1 and indicates
"everything"
 2) nir_lower_indirect_derefs takes a uint32_t mode_mask which is expected
to be a bunch of (1 << mode) ORd together
 3) We could just convert each of the mode enum values to be a single bit
and support ORing them together

(1) is pretty terrible since it only allows one or all and doesn't allow
you to specify a non-trivial subset.  (2) allows for enum confusion but is
a fairly standard pattern in mesa today so maybe not too bad.  (3) requires
more bits when you only want to specify one enum (meh), requires that we
validate that only one bit is set on a variable, and will be confused with
the common pattern of (2) in mesa.  On the other hand (3) prints nicely in
gdb and looks nice when you write it.

All told, I don't like (1) at all and have a mild preference for (2) over
(3).  However, I could be convinced of (3) should others have a strong
inclination towards it.

Regardless of what we choose, I'll update this patch and submit patches to
fix up nir_lower_io and nir_lower_indirect_derefs to match.

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160410/287de99a/attachment.html>