[Mesa-dev] [PATCH] nir: Handle double-precision in fabs, frsq, fsqrt, fexp2 and flog2

[Iago Toral]

On Thu, 2016-05-19 at 13:13 -0700, Matt Turner wrote:
> On Thu, May 19, 2016 at 3:36 AM, Iago Toral Quiroga <itoral at igalia.com> wrote:
> > We agreed in the list that it would be better to have these if they were
> > easy to implement.
> > ---
> >  src/compiler/nir/nir_opcodes.py | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_opcodes.py
> > index 8a3a80f..6dc0c90 100644
> > --- a/src/compiler/nir/nir_opcodes.py
> > +++ b/src/compiler/nir/nir_opcodes.py
> > @@ -153,13 +153,13 @@ unop("fnot", tfloat, "(src0 == 0.0f) ? 1.0f : 0.0f")
> >  unop("fsign", tfloat, "(src0 == 0.0f) ? 0.0f : ((src0 > 0.0f) ? 1.0f : -1.0f)")
> >  unop("isign", tint, "(src0 == 0) ? 0 : ((src0 > 0) ? 1 : -1)")
> >  unop("iabs", tint, "(src0 < 0) ? -src0 : src0")
> > -unop("fabs", tfloat, "fabsf(src0)")
> > +unop("fabs", tfloat, "bit_size == 64 ? fabs(src0) : fabsf(src0)")
> 
> Seems like we should have had this anyway, since the extension
> requires abs(double)?
> 
> As idr said in that thread, fnot, fsign, and fsat should have explicit
> double support as well.

I thought the conclusion to that was that these were fine since floats
would be promoted to doubles anyway. I can add explicit support though.

> >  unop("fsat", tfloat, "(src0 > 1.0f) ? 1.0f : ((src0 <= 0.0f) ? 0.0f : src0)")
> >  unop("frcp", tfloat, "1.0f / src0")
> > -unop("frsq", tfloat, "1.0f / sqrtf(src0)")
> > -unop("fsqrt", tfloat, "sqrtf(src0)")
> > -unop("fexp2", tfloat, "exp2f(src0)")
> > -unop("flog2", tfloat, "log2f(src0)")
> > +unop("frsq", tfloat, "bit_size == 64 ? 1.0 / sqrt(src0) : 1.0f / sqrtf(src0)")
> > +unop("fsqrt", tfloat, "bit_size == 64 ? sqrt(src0) : sqrtf(src0)")
> 
> And these?
> 
> > +unop("fexp2", tfloat, "bit_size == 64 ? exp2(src0) : exp2f(src0)")
> > +unop("flog2", tfloat, "bit_size == 64 ? log2(src0) : log2f(src0)")
> 
> I don't know why we'd add these. I think this is just confusing the
> issue. I'd separate functions that are actually required by the spec
> into a separate patch at least.

In the original thread Jason and Ian seemed to agree that we should
probably just add all of these if they were easy to implement. FWIW, the
original patch by Connor already added a few that are not in the spec.
In any case, I am fine with leaving these out for now.

Iago