[Mesa-dev] [PATCH v4] nir: Don't lower the local work group size if it's variable.

[Karol Herbst]

Reviewed-by: Karol Herbst <kherbst at redhat.com>
On Tue, Nov 13, 2018 at 3:51 AM Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> On Mon, Nov 12, 2018 at 6:10 PM Karol Herbst <kherbst at redhat.com> wrote:
>>
>> it shouldn't make a difference. This pass lowers load_derefs into
>> whatever we want here. If we handle the system value explicitly
>> "sysval" gets set. If not, we fetch the op through
>> nir_intrinsic_from_system_value and do the load based on that. We just
>> take a different path, but fundamentally we do the same either way.
>
>
> Drp... You're right.
>
> Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>
>
>>
>> On Tue, Nov 13, 2018 at 12:17 AM Jason Ekstrand <jason at jlekstrand.net> wrote:
>> >
>> > I think we still want to skip the lowering of SYSTEM_VALUE_LOCAL_GROUP_SIZE when that flag is set.  I think this works, but we'll end up deleting one load_local_group_size intrinsic and replacing it with another which is pointless.
>> >
>> > --Jason
>> >
>> > On Mon, Nov 12, 2018 at 4:02 PM Plamena Manolova <plamena.n.manolova at gmail.com> wrote:
>> >>
>> >> If the local work group size is variable it won't be available
>> >> at compile time so we can't lower it in nir_lower_system_values().
>> >>
>> >> Signed-off-by: Plamena Manolova <plamena.n.manolova at gmail.com>
>> >> ---
>> >>  src/compiler/nir/nir_lower_system_values.c | 24 ++++++++++++++++++------
>> >>  1 file changed, 18 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/src/compiler/nir/nir_lower_system_values.c b/src/compiler/nir/nir_lower_system_values.c
>> >> index bde7eb1180..fbc4057357 100644
>> >> --- a/src/compiler/nir/nir_lower_system_values.c
>> >> +++ b/src/compiler/nir/nir_lower_system_values.c
>> >> @@ -31,12 +31,24 @@
>> >>  static nir_ssa_def*
>> >>  build_local_group_size(nir_builder *b)
>> >>  {
>> >> -   nir_const_value local_size;
>> >> -   memset(&local_size, 0, sizeof(local_size));
>> >> -   local_size.u32[0] = b->shader->info.cs.local_size[0];
>> >> -   local_size.u32[1] = b->shader->info.cs.local_size[1];
>> >> -   local_size.u32[2] = b->shader->info.cs.local_size[2];
>> >> -   return nir_build_imm(b, 3, 32, local_size);
>> >> +   nir_ssa_def *local_size;
>> >> +
>> >> +   /*
>> >> +    * If the local work group size is variable it can't be lowered at this
>> >> +    * point, but its intrinsic can still be used.
>> >> +    */
>> >> +   if (b->shader->info.cs.local_size_variable) {
>> >> +      local_size = nir_load_local_group_size(b);
>> >> +   } else {
>> >> +      nir_const_value local_size_const;
>> >> +      memset(&local_size_const, 0, sizeof(local_size_const));
>> >> +      local_size_const.u32[0] = b->shader->info.cs.local_size[0];
>> >> +      local_size_const.u32[1] = b->shader->info.cs.local_size[1];
>> >> +      local_size_const.u32[2] = b->shader->info.cs.local_size[2];
>> >> +      local_size = nir_build_imm(b, 3, 32, local_size_const);
>> >> +   }
>> >> +
>> >> +   return local_size;
>> >>  }
>> >>
>> >>  static bool
>> >> --
>> >> 2.11.0
>> >>