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

Submitted by Iago Toral Quiroga on May 19, 2016, 10:36 a.m.

Details

Message ID 1463654210-9880-1-git-send-email-itoral@igalia.com
State New
Headers show
Series "nir: Handle double-precision in fabs, frsq, fsqrt, fexp2 and flog2" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Iago Toral Quiroga May 19, 2016, 10:36 a.m.
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(-)

Patch hide | download patch | download mbox

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)")
 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)")
+unop("fexp2", tfloat, "bit_size == 64 ? exp2(src0) : exp2f(src0)")
+unop("flog2", tfloat, "bit_size == 64 ? log2(src0) : log2f(src0)")
 unop_convert("f2i", tint32, tfloat32, "src0") # Float-to-integer conversion.
 unop_convert("f2u", tuint32, tfloat32, "src0") # Float-to-unsigned conversion
 unop_convert("d2i", tint32, tfloat64, "src0") # Double-to-integer conversion.

Comments

On Thu, May 19, 2016 at 3:36 AM, Iago Toral Quiroga <itoral@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.

>  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.
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@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