Re: [Mesa-dev] [PATCH] nir: fix ir_binop_gequal glsl_to_nir conversion

2018-04-16 Thread Jason Ekstrand 

Pushed.  Thanks and welcome to Mesa!

On April 14, 2018 12:26:18 Jason Ekstrand  wrote:


Reviewed-by: Jason Ekstrand 

What driver is hitting this path?  The !supports_ints path isn't used to my
knowledge so if some driver has started using it, they're liable to find
more bugs than just this one. :-)

On April 14, 2018 12:16:48 Erico Nunes  wrote:

> ir_binop_gequal needs to be converted to nir_op_sge when native integers
> are not supported in the driver.
> Otherwise it becomes no different than ir_binop_less after the
> conversion.
>
> Signed-off-by: Erico Nunes 
> ---
> src/compiler/glsl/glsl_to_nir.cpp | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/compiler/glsl/glsl_to_nir.cpp
> b/src/compiler/glsl/glsl_to_nir.cpp
> index 17d58acc4c..8e5e9c3491 100644
> --- a/src/compiler/glsl/glsl_to_nir.cpp
> +++ b/src/compiler/glsl/glsl_to_nir.cpp
> @@ -1832,7 +1832,7 @@ nir_visitor::visit(ir_expression *ir)
>  else
> result = nir_uge(, srcs[0], srcs[1]);
>   } else {
> - result = nir_slt(, srcs[0], srcs[1]);
> + result = nir_sge(, srcs[0], srcs[1]);
>   }
>   break;
>case ir_binop_equal:
> --
> 2.14.3
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir: fix ir_binop_gequal glsl_to_nir conversion

2018-04-16 Thread Jason Ekstrand 
On Mon, Apr 16, 2018 at 6:45 AM, Erico Nunes  wrote:

> On Sun, Apr 15, 2018 at 2:30 AM, Jason Ekstrand 
> wrote:
> > On April 14, 2018 12:43:35 Connor Abbott  wrote:
> > I think that it's probably impractical to use this path, and we should
> > probably delete it. There are just too many optimizations, e.g. in
> > nir_opt_algebraic and lowering passes that assume you have ints. I
> > think a better plan would be to silently convert ints to floats in the
> > lima driver, and maybe inhibit any optimizations that use bit
> > twiddling tricks if real int support isn't indicated.
> >
> > I'm not sure.  For quite a while prog_to_nir used these comparison
> > operations so we know they more it less work.  For all I know, maybe it
> > still does (I didn't actually check).  The only thing we need to worry
> about
> > in terms of correctness is any optimizations in nir_opt_algebraic which
> > consume only floats but produce integers.  Also, all drivers need to
> handle
> > imov simply because it's easy.
> >
> > That being said, we've done a lot of work to optimize the integer
> supporting
> > paths so you may actually get better code if you can figure out a good
> way
> > to lower the integers away.
>
> I'm not really using ints in my sample program, just floats. But still
> I'm getting nir_op_slt and nir_op_sge for the float comparison
> operations.
> Should I be getting nir_op_flt and nir_op_fge instead even with
> .native_integers disabled in glsl_to_nir?
>

Nope.  That's kind of what the native_integers option is for.  I'm just
saying that it isn't incredibly well tested so be ware.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir: fix ir_binop_gequal glsl_to_nir conversion

2018-04-16 Thread Erico Nunes 
On Sun, Apr 15, 2018 at 2:30 AM, Jason Ekstrand  wrote:
> On April 14, 2018 12:43:35 Connor Abbott  wrote:
> I think that it's probably impractical to use this path, and we should
> probably delete it. There are just too many optimizations, e.g. in
> nir_opt_algebraic and lowering passes that assume you have ints. I
> think a better plan would be to silently convert ints to floats in the
> lima driver, and maybe inhibit any optimizations that use bit
> twiddling tricks if real int support isn't indicated.
>
> I'm not sure.  For quite a while prog_to_nir used these comparison
> operations so we know they more it less work.  For all I know, maybe it
> still does (I didn't actually check).  The only thing we need to worry about
> in terms of correctness is any optimizations in nir_opt_algebraic which
> consume only floats but produce integers.  Also, all drivers need to handle
> imov simply because it's easy.
>
> That being said, we've done a lot of work to optimize the integer supporting
> paths so you may actually get better code if you can figure out a good way
> to lower the integers away.

I'm not really using ints in my sample program, just floats. But still
I'm getting nir_op_slt and nir_op_sge for the float comparison
operations.
Should I be getting nir_op_flt and nir_op_fge instead even with
.native_integers disabled in glsl_to_nir?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir: fix ir_binop_gequal glsl_to_nir conversion

2018-04-14 Thread Jason Ekstrand 

On April 14, 2018 12:43:35 Connor Abbott  wrote:

On Sat, Apr 14, 2018 at 3:39 PM, Erico Nunes  wrote:
On Sat, Apr 14, 2018 at 9:26 PM, Jason Ekstrand  wrote:
Reviewed-by: Jason Ekstrand 

What driver is hitting this path?  The !supports_ints path isn't used to my
knowledge so if some driver has started using it, they're liable to find
more bugs than just this one. :-)

I'm doing some work on the lima vertex shader compiler and I hit this.

And yeah this is there since 2015 it seems, so I suppose no other
drivers are using this path, we'll see if there's more.

I think that it's probably impractical to use this path, and we should
probably delete it. There are just too many optimizations, e.g. in
nir_opt_algebraic and lowering passes that assume you have ints. I
think a better plan would be to silently convert ints to floats in the
lima driver, and maybe inhibit any optimizations that use bit
twiddling tricks if real int support isn't indicated.

I'm not sure.  For quite a while prog_to_nir used these comparison 
operations so we know they more it less work.  For all I know, maybe it 
still does (I didn't actually check).  The only thing we need to worry 
about in terms of correctness is any optimizations in nir_opt_algebraic 
which consume only floats but produce integers.  Also, all drivers need to 
handle imov simply because it's easy.


That being said, we've done a lot of work to optimize the integer 
supporting paths so you may actually get better code if you can figure out 
a good way to lower the integers away.




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir: fix ir_binop_gequal glsl_to_nir conversion

2018-04-14 Thread Connor Abbott 
On Sat, Apr 14, 2018 at 3:39 PM, Erico Nunes  wrote:
> On Sat, Apr 14, 2018 at 9:26 PM, Jason Ekstrand  wrote:
>> Reviewed-by: Jason Ekstrand 
>>
>> What driver is hitting this path?  The !supports_ints path isn't used to my
>> knowledge so if some driver has started using it, they're liable to find
>> more bugs than just this one. :-)
>
> I'm doing some work on the lima vertex shader compiler and I hit this.
>
> And yeah this is there since 2015 it seems, so I suppose no other
> drivers are using this path, we'll see if there's more.

I think that it's probably impractical to use this path, and we should
probably delete it. There are just too many optimizations, e.g. in
nir_opt_algebraic and lowering passes that assume you have ints. I
think a better plan would be to silently convert ints to floats in the
lima driver, and maybe inhibit any optimizations that use bit
twiddling tricks if real int support isn't indicated.

> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir: fix ir_binop_gequal glsl_to_nir conversion

2018-04-14 Thread Erico Nunes 
On Sat, Apr 14, 2018 at 9:26 PM, Jason Ekstrand  wrote:
> Reviewed-by: Jason Ekstrand 
>
> What driver is hitting this path?  The !supports_ints path isn't used to my
> knowledge so if some driver has started using it, they're liable to find
> more bugs than just this one. :-)

I'm doing some work on the lima vertex shader compiler and I hit this.

And yeah this is there since 2015 it seems, so I suppose no other
drivers are using this path, we'll see if there's more.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nir: fix ir_binop_gequal glsl_to_nir conversion

2018-04-14 Thread Jason Ekstrand 

Reviewed-by: Jason Ekstrand 

What driver is hitting this path?  The !supports_ints path isn't used to my 
knowledge so if some driver has started using it, they're liable to find 
more bugs than just this one. :-)


On April 14, 2018 12:16:48 Erico Nunes  wrote:


ir_binop_gequal needs to be converted to nir_op_sge when native integers
are not supported in the driver.
Otherwise it becomes no different than ir_binop_less after the
conversion.

Signed-off-by: Erico Nunes 
---
src/compiler/glsl/glsl_to_nir.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/glsl/glsl_to_nir.cpp 
b/src/compiler/glsl/glsl_to_nir.cpp

index 17d58acc4c..8e5e9c3491 100644
--- a/src/compiler/glsl/glsl_to_nir.cpp
+++ b/src/compiler/glsl/glsl_to_nir.cpp
@@ -1832,7 +1832,7 @@ nir_visitor::visit(ir_expression *ir)
 else
result = nir_uge(, srcs[0], srcs[1]);
  } else {
- result = nir_slt(, srcs[0], srcs[1]);
+ result = nir_sge(, srcs[0], srcs[1]);
  }
  break;
   case ir_binop_equal:
--
2.14.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev