[Mesa-dev] [PATCH 2/7] nir/int64: Add some more lowering helpers

[Jason Ekstrand]

On Mon, Oct 15, 2018 at 3:13 PM Connor Abbott <cwabbott0 at gmail.com> wrote:

> On Mon, Oct 15, 2018 at 8:41 PM Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> >
> > On Mon, Oct 15, 2018 at 1:39 PM Ian Romanick <idr at freedesktop.org>
> wrote:
> >>
> >> On 10/14/2018 03:58 PM, Jason Ekstrand wrote:
> >> > On October 14, 2018 17:12:34 Matt Turner <mattst88 at gmail.com> wrote:
> >> >> +static nir_ssa_def *
> >> >> +lower_iabs64(nir_builder *b, nir_ssa_def *x)
> >> >> +{
> >> >> +   nir_ssa_def *x_hi = nir_unpack_64_2x32_split_y(b, x);
> >> >> +   nir_ssa_def *x_is_neg = nir_ilt(b, x_hi, nir_imm_int(b, 0));
> >> >> +   return nir_bcsel(b, x_is_neg, lower_ineg64(b, x), x);
> >> >
> >> > lower_bcsel?  Or, since we're depending on this running multiple
> times,
> >> > just nir_ineg?  I go back and forth on whether a pass like this should
> >> > run in a loop or be smart enough to lower intermediate bits on the
> fly.
> >> > We should probably pick one.
> >>
> >> In principle, I agree.  I've been bitten a couple times by lowering
> >> passes that generate other things that need to be lowered on some
> >> platforms (that I didn't test).  In this case, I think the loop is the
> >> right answer since each operation is lowered by a separate flag.
> >
> >
> > That's the easy answer, certainly.  The other option is to have every
> lowered thing builder check the flag and conditionally do the lowering.
> That's annoying and hard to get right so a loop is probably best for now.
>
> Couldn't you just have the builder be right after the instruction,
> instead of before it, and make the outer loop use a non-safe iterator
> so that it will immediately run over the instructions generated? Doing
> another pass over the whole shader is usually a little expensive.
>

That's sneaky and also a really good idea!  We should totally just do that.

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