[Mesa-dev] [PATCH 1/2] nir: add lowering for ffloor

Tue Nov 13 21:34:05 UTC 2018

On Tue, Nov 13, 2018 at 3:21 PM Roland Scheidegger <sroland at vmware.com>
wrote:

> Am 12.11.18 um 20:40 schrieb Jason Ekstrand:
> > On Mon, Nov 12, 2018 at 1:29 PM Christian Gmeiner
> > <christian.gmeiner at gmail.com <mailto:christian.gmeiner at gmail.com>>
> wrote:
> >
> >     Hi Jason
> >
> >     Am Sa., 1. Sep. 2018 um 21:23 Uhr schrieb Jason Ekstrand
> >     <jason at jlekstrand.net <mailto:jason at jlekstrand.net>>:
> >     >
> >     > I don't think either of these work for negative numbers
> >     >
> >
> >     I would like to land this patch - can you provide some details why
> >     this does not work for
> >     negative numbers?
> >
> >
> > No, this is correct.  It's GLSL's definition of fract(x) that's dumb.
> > GLSL defines fract(x) as x - floor(x) (exactly what your formula says.
> > This means that
> >
> > fract(-1.4) = -1.4 - floor(-1.4) = -1.4 - (-2.0) = -1.4 + 2.0 = 0.6
> >
> > so GLSL fract() doesn't give you the fractional part at all.  *sigh*
> Can you elaborate why you think this result to be wrong? In fact it's
> the only definition I've ever heard of. But yes, wikipedia says fract
> for negative numbers is open for debate (with 3 possible solutions). In
> the context of graphics shading languages, it is however most definitely
> the agreed-upon formula for fractional parts (and certainly the same
> formula was used pre-glsl or hlsl).
>

It falls into exactly the same category as all the different ways to define
modulus/remainder.  You just sort-of end up having to pick one.  I guess my
main gripe is that, when someone says "returns the fractional part", I
would expect x = trunc(x) + fract(x) or x = trunc(x) + sign(x) * fract(x).
Of the possible conventions, they chose the one that I find to be the
strangest.  That's all.

--Jason

> Roland
>
>
> >
> > Reviewed-by: Jason Ekstrand <jason at jlekstrand.net
> > <mailto:jason at jlekstrand.net>>
> >
> >
> >     > On September 1, 2018 14:16:11 Christian Gmeiner
> >     > <christian.gmeiner at gmail.com <mailto:christian.gmeiner at gmail.com>>
> >     wrote:
> >     >
> >     > > Signed-off-by: Christian Gmeiner <christian.gmeiner at gmail.com
> >     <mailto:christian.gmeiner at gmail.com>>
> >     > > ---
> >     > > src/compiler/nir/nir.h                | 3 +++
> >     > > src/compiler/nir/nir_opt_algebraic.py | 1 +
> >     > > 2 files changed, 4 insertions(+)
> >     > >
> >     > > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> >     > > index 169fa1fa20..d81eefc032 100644
> >     > > --- a/src/compiler/nir/nir.h
> >     > > +++ b/src/compiler/nir/nir.h
> >     > > @@ -2054,6 +2054,9 @@ typedef struct nir_shader_compiler_options
> {
> >     > >     */
> >     > >    bool fdot_replicates;
> >     > >
> >     > > +   /** lowers ffloor to fsub+ffract: */
> >     > > +   bool lower_ffloor;
> >     > > +
> >     > >    /** lowers ffract to fsub+ffloor: */
> >     > >    bool lower_ffract;
> >     > >
> >     > > diff --git a/src/compiler/nir/nir_opt_algebraic.py
> >     > > b/src/compiler/nir/nir_opt_algebraic.py
> >     > > index ae1261f874..3d2b861a42 100644
> >     > > --- a/src/compiler/nir/nir_opt_algebraic.py
> >     > > +++ b/src/compiler/nir/nir_opt_algebraic.py
> >     > > @@ -118,6 +118,7 @@ optimizations = [
> >     > >    (('~flrp', a, 0.0, c), ('fadd', ('fmul', ('fneg', a), c), a)),
> >     > >    (('flrp at 32', a, b, c), ('fadd', ('fmul', c, ('fsub', b, a)),
> a),
> >     > >    'options->lower_flrp32'),
> >     > >    (('flrp at 64', a, b, c), ('fadd', ('fmul', c, ('fsub', b, a)),
> a),
> >     > >    'options->lower_flrp64'),
> >     > > +   (('ffloor', a), ('fsub', a, ('ffract', a)),
> >     'options->lower_ffloor'),
> >     > >    (('ffract', a), ('fsub', a, ('ffloor', a)),
> >     'options->lower_ffract'),
> >     > >    (('~fadd', ('fmul', a, ('fadd', 1.0, ('fneg', ('b2f', c)))),
> >     ('fmul', b,
> >     > >    ('b2f', c))), ('bcsel', c, b, a), 'options->lower_flrp32'),
> >     > >    (('~fadd at 32', ('fmul', a, ('fadd', 1.0, ('fneg',         c
> >     ))), ('fmul', b,
> >     > >            c )), ('flrp', a, b, c), '!options->lower_flrp32'),
> >     > > --
> >     > > 2.17.1
> >     > >
> >     > > _______________________________________________
> >     > > mesa-dev mailing list
> >     > > mesa-dev at lists.freedesktop.org
> >     <mailto:mesa-dev at lists.freedesktop.org>
> >     > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >     <
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-dev&data=02%7C01%7Csroland%40vmware.com%7Ce0fe2529778847ca3ef208d648d6bb83%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636776484451503870&sdata=vkUGCDEE3AeUCdZaibTXfuIwQS0FU39C3V%2FnJMraRlM%3D&reserved=0
> >
> >     >
> >     >
> >     >
> >
> >     --
> >     greets
> >     --
> >     Christian Gmeiner, MSc
> >
> >     https://christian-gmeiner.info
> >     <
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fchristian-gmeiner.info&data=02%7C01%7Csroland%40vmware.com%7Ce0fe2529778847ca3ef208d648d6bb83%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636776484451513879&sdata=IPMIs3gEGPgunAgWAoMjjR%2Bev%2FEBADnfJItCHtFuyYc%3D&reserved=0
> >
> >
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> >
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-dev&data=02%7C01%7Csroland%40vmware.com%7Ce0fe2529778847ca3ef208d648d6bb83%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636776484451523879&sdata=uioIRixNYHv8b%2FKTa%2BXqkzkDVZCnlm4FYdzsnxsDo38%3D&reserved=0
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181113/a6725acd/attachment-0001.html>