[Mesa-dev] [PATCH 22/59] compiler/nir: add lowering for 16-bit ldexp

[Jason Ekstrand]

On Wed, Dec 5, 2018 at 5:32 AM Pohjolainen, Topi <topi.pohjolainen at gmail.com>
wrote:

> On Wed, Dec 05, 2018 at 12:26:06PM +0100, Iago Toral wrote:
> > On Wed, 2018-12-05 at 13:20 +0200, Pohjolainen, Topi wrote:
> > > On Wed, Dec 05, 2018 at 11:53:44AM +0100, Iago Toral wrote:
> > > > On Wed, 2018-12-05 at 11:39 +0200, Pohjolainen, Topi wrote:
> > > > > I remember people preferring to order things 16, 32, 64 before.
> > > > > Should
> > > > > we follow that here as well?
> > > >
> > > > Yes, it makes sense. I'll change that.
>

Agreed.


> > > >
> > > > > On Tue, Dec 04, 2018 at 08:16:46AM +0100, Iago Toral Quiroga
> > > > > wrote:
> > > > > > ---
> > > > > >  src/compiler/nir/nir_opt_algebraic.py | 5 +++++
> > > > > >  1 file changed, 5 insertions(+)
> > > > > >
> > > > > > diff --git a/src/compiler/nir/nir_opt_algebraic.py
> > > > > > b/src/compiler/nir/nir_opt_algebraic.py
> > > > > > index 6c3b77c9b6e..747f1751086 100644
> > > > > > --- a/src/compiler/nir/nir_opt_algebraic.py
> > > > > > +++ b/src/compiler/nir/nir_opt_algebraic.py
> > > > > > @@ -778,6 +778,8 @@ def fexp2i(exp, bits):
> > > > > >        return ('ishl', ('iadd', exp, 127), 23)
> > > > > >     elif bits == 64:
> > > > > >        return ('pack_64_2x32_split', 0, ('ishl', ('iadd', exp,
> > > > > > 1023), 20))
> > > > > > +   elif bits == 16:
> > > > > > +      return ('i2i16', ('ishl', ('iadd', exp, 15), 10))
> > > > > >     else:
> > > > > >        assert False
> > > > > >
> > > > > > @@ -796,6 +798,8 @@ def ldexp(f, exp, bits):
> > > > > >        exp = ('imin', ('imax', exp, -252), 254)
> > > > > >     elif bits == 64:
> > > > > >        exp = ('imin', ('imax', exp, -2044), 2046)
> > > > > > +   elif bits == 16:
> > > > > > +      exp = ('imin', ('imax', exp, -30), 30)
> > > > >
> > > > > I expected this to be:
> > > > >
> > > > >          exp = ('imin', ('imax', exp, -29), 30)
> > > >
> > > > Actually, I think this should be -28, since the minimum exponent
> > > > value
> > > > is -14.
> > >
> > > I kept wondering about. The offset is 15 and -14 - 15 yields -29. But
> > > -28
> > > in turn would be more in line with the 32- and 64-bit cases.
> >
> > I think the idea is to have this be 2x the minimum (and maximum)
> > exponents we can represent, since below we are dividing it by two and
> > emitting two exponentials, each with half that exponent. That way we
> > ensure that when we divide the exponent by 2 we still produce a
> > representable exponent for the bit-size.
>
> Ah, right. I should have checked the context, -28 makes sense now.
>
> Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>

With things in the right order and [-28, 30],

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181207/8799257f/attachment-0001.html>