Re: [Mesa-dev] [PATCH 0/2] nir: Move the si vote lowering to common code

[Jason Ekstrand]

On Tue, Mar 13, 2018 at 11:39 AM, Bas Nieuwenhuizen  wrote:

> Thanks a lot!
>

You're welcome.  Classic case of writing code is easier than reviewing it.
:-)


> Reviewed-by: Bas Nieuwenhuizen 
>

Thanks!  Pushed.


> for the series.
>
> On Sat, Mar 10, 2018 at 7:42 PM, Jason Ekstrand 
> wrote:
> > On Sat, Mar 10, 2018 at 10:18 AM, Jason Ekstrand 
> > wrote:
> >>
> >> This tiny series moves the pass that Bas wrote for lowering vote
> >> instructions to ballots into common code.  As you can see from the
> >> diffstat
> >> below, it cuts the size of the pass in half by removing boilerplate.
> The
> >> new version (in nir_lower_subgroups) also contains two bugfixes:
> >>
> >>  1) The old pass wasn't actually inserting the RFI instruction
> >>
> >>  2) The old pass wouldn't handle NaN correctly in feq because it
> switched
> >> to effectively using ineg(fne(a, b)) which is not the same as feq(a,
> >> b)
> >> when NaN is present.
> >>
> >> This has not been tested at all beyond compile-testing (and I didn't
> even
> >> compile-test radv).
> >
> >
> > I hacked up anv to use it and ran the subgroups tests from the 1.1 CTS.
> > This pointed out some bugs which I've now fixed and I sent a v2 which
> works
> > at least for anv.
> >
> >>
> >> Cc: Bas Nieuwenhuizen 
> >> Cc: Timothy Arceri 
> >>
> >> Jason Ekstrand (2):
> >>   nir/subgroups: Add lowering for vote_ieq/vote_feq to a ballot
> >>   ac/nir: Use lower_vote_eq_to_ballot instead of ac_nir_lower_subgroups
> >>
> >>  src/amd/Makefile.sources |  1 -
> >>  src/amd/common/ac_lower_subgroups.c  | 92
> >> 
> >>  src/amd/common/ac_nir_to_llvm.c  |  3 -
> >>  src/amd/common/ac_nir_to_llvm.h  |  2 -
> >>  src/amd/common/meson.build   |  1 -
> >>  src/compiler/nir/nir.h   |  1 +
> >>  src/compiler/nir/nir_lower_subgroups.c   | 48 +++
> >>  src/gallium/drivers/radeonsi/si_shader_nir.c |  1 +
> >>  8 files changed, 50 insertions(+), 99 deletions(-)
> >>  delete mode 100644 src/amd/common/ac_lower_subgroups.c
> >>
> >> --
> >> 2.5.0.400.gff86faf
> >>
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/2] nir: Move the si vote lowering to common code

[Bas Nieuwenhuizen]

Thanks a lot!

Reviewed-by: Bas Nieuwenhuizen 

for the series.

On Sat, Mar 10, 2018 at 7:42 PM, Jason Ekstrand  wrote:
> On Sat, Mar 10, 2018 at 10:18 AM, Jason Ekstrand 
> wrote:
>>
>> This tiny series moves the pass that Bas wrote for lowering vote
>> instructions to ballots into common code.  As you can see from the
>> diffstat
>> below, it cuts the size of the pass in half by removing boilerplate.  The
>> new version (in nir_lower_subgroups) also contains two bugfixes:
>>
>>  1) The old pass wasn't actually inserting the RFI instruction
>>
>>  2) The old pass wouldn't handle NaN correctly in feq because it switched
>> to effectively using ineg(fne(a, b)) which is not the same as feq(a,
>> b)
>> when NaN is present.
>>
>> This has not been tested at all beyond compile-testing (and I didn't even
>> compile-test radv).
>
>
> I hacked up anv to use it and ran the subgroups tests from the 1.1 CTS.
> This pointed out some bugs which I've now fixed and I sent a v2 which works
> at least for anv.
>
>>
>> Cc: Bas Nieuwenhuizen 
>> Cc: Timothy Arceri 
>>
>> Jason Ekstrand (2):
>>   nir/subgroups: Add lowering for vote_ieq/vote_feq to a ballot
>>   ac/nir: Use lower_vote_eq_to_ballot instead of ac_nir_lower_subgroups
>>
>>  src/amd/Makefile.sources |  1 -
>>  src/amd/common/ac_lower_subgroups.c  | 92
>> 
>>  src/amd/common/ac_nir_to_llvm.c  |  3 -
>>  src/amd/common/ac_nir_to_llvm.h  |  2 -
>>  src/amd/common/meson.build   |  1 -
>>  src/compiler/nir/nir.h   |  1 +
>>  src/compiler/nir/nir_lower_subgroups.c   | 48 +++
>>  src/gallium/drivers/radeonsi/si_shader_nir.c |  1 +
>>  8 files changed, 50 insertions(+), 99 deletions(-)
>>  delete mode 100644 src/amd/common/ac_lower_subgroups.c
>>
>> --
>> 2.5.0.400.gff86faf
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/2] nir: Move the si vote lowering to common code

[Jason Ekstrand]

On Sat, Mar 10, 2018 at 10:18 AM, Jason Ekstrand 
wrote:

> This tiny series moves the pass that Bas wrote for lowering vote
> instructions to ballots into common code.  As you can see from the diffstat
> below, it cuts the size of the pass in half by removing boilerplate.  The
> new version (in nir_lower_subgroups) also contains two bugfixes:
>
>  1) The old pass wasn't actually inserting the RFI instruction
>
>  2) The old pass wouldn't handle NaN correctly in feq because it switched
> to effectively using ineg(fne(a, b)) which is not the same as feq(a, b)
> when NaN is present.
>
> This has not been tested at all beyond compile-testing (and I didn't even
> compile-test radv).
>

I hacked up anv to use it and ran the subgroups tests from the 1.1 CTS.
This pointed out some bugs which I've now fixed and I sent a v2 which works
at least for anv.


> Cc: Bas Nieuwenhuizen 
> Cc: Timothy Arceri 
>
> Jason Ekstrand (2):
>   nir/subgroups: Add lowering for vote_ieq/vote_feq to a ballot
>   ac/nir: Use lower_vote_eq_to_ballot instead of ac_nir_lower_subgroups
>
>  src/amd/Makefile.sources |  1 -
>  src/amd/common/ac_lower_subgroups.c  | 92
> 
>  src/amd/common/ac_nir_to_llvm.c  |  3 -
>  src/amd/common/ac_nir_to_llvm.h  |  2 -
>  src/amd/common/meson.build   |  1 -
>  src/compiler/nir/nir.h   |  1 +
>  src/compiler/nir/nir_lower_subgroups.c   | 48 +++
>  src/gallium/drivers/radeonsi/si_shader_nir.c |  1 +
>  8 files changed, 50 insertions(+), 99 deletions(-)
>  delete mode 100644 src/amd/common/ac_lower_subgroups.c
>
> --
> 2.5.0.400.gff86faf
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 0/2] nir: Move the si vote lowering to common code

[Jason Ekstrand]

This tiny series moves the pass that Bas wrote for lowering vote
instructions to ballots into common code.  As you can see from the diffstat
below, it cuts the size of the pass in half by removing boilerplate.  The
new version (in nir_lower_subgroups) also contains two bugfixes:

 1) The old pass wasn't actually inserting the RFI instruction

 2) The old pass wouldn't handle NaN correctly in feq because it switched
to effectively using ineg(fne(a, b)) which is not the same as feq(a, b)
when NaN is present.

This has not been tested at all beyond compile-testing (and I didn't even
compile-test radv).

Cc: Bas Nieuwenhuizen 
Cc: Timothy Arceri 

Jason Ekstrand (2):
  nir/subgroups: Add lowering for vote_ieq/vote_feq to a ballot
  ac/nir: Use lower_vote_eq_to_ballot instead of ac_nir_lower_subgroups

 src/amd/Makefile.sources |  1 -
 src/amd/common/ac_lower_subgroups.c  | 92 
 src/amd/common/ac_nir_to_llvm.c  |  3 -
 src/amd/common/ac_nir_to_llvm.h  |  2 -
 src/amd/common/meson.build   |  1 -
 src/compiler/nir/nir.h   |  1 +
 src/compiler/nir/nir_lower_subgroups.c   | 48 +++
 src/gallium/drivers/radeonsi/si_shader_nir.c |  1 +
 8 files changed, 50 insertions(+), 99 deletions(-)
 delete mode 100644 src/amd/common/ac_lower_subgroups.c

-- 
2.5.0.400.gff86faf

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