<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Nov 12, 2018 at 6:10 PM Karol Herbst <<a href="mailto:kherbst@redhat.com">kherbst@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">it shouldn't make a difference. This pass lowers load_derefs into<br>
whatever we want here. If we handle the system value explicitly<br>
"sysval" gets set. If not, we fetch the op through<br>
nir_intrinsic_from_system_value and do the load based on that. We just<br>
take a different path, but fundamentally we do the same either way.<br></blockquote><div><br></div><div>Drp... You're right.</div><div><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Tue, Nov 13, 2018 at 12:17 AM Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote:<br>
><br>
> 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.<br>
><br>
> --Jason<br>
><br>
> On Mon, Nov 12, 2018 at 4:02 PM Plamena Manolova <<a href="mailto:plamena.n.manolova@gmail.com" target="_blank">plamena.n.manolova@gmail.com</a>> wrote:<br>
>><br>
>> If the local work group size is variable it won't be available<br>
>> at compile time so we can't lower it in nir_lower_system_values().<br>
>><br>
>> Signed-off-by: Plamena Manolova <<a href="mailto:plamena.n.manolova@gmail.com" target="_blank">plamena.n.manolova@gmail.com</a>><br>
>> ---<br>
>>  src/compiler/nir/nir_lower_system_values.c | 24 ++++++++++++++++++------<br>
>>  1 file changed, 18 insertions(+), 6 deletions(-)<br>
>><br>
>> diff --git a/src/compiler/nir/nir_lower_system_values.c b/src/compiler/nir/nir_lower_system_values.c<br>
>> index bde7eb1180..fbc4057357 100644<br>
>> --- a/src/compiler/nir/nir_lower_system_values.c<br>
>> +++ b/src/compiler/nir/nir_lower_system_values.c<br>
>> @@ -31,12 +31,24 @@<br>
>>  static nir_ssa_def*<br>
>>  build_local_group_size(nir_builder *b)<br>
>>  {<br>
>> -   nir_const_value local_size;<br>
>> -   memset(&local_size, 0, sizeof(local_size));<br>
>> -   local_size.u32[0] = b->shader->info.cs.local_size[0];<br>
>> -   local_size.u32[1] = b->shader->info.cs.local_size[1];<br>
>> -   local_size.u32[2] = b->shader->info.cs.local_size[2];<br>
>> -   return nir_build_imm(b, 3, 32, local_size);<br>
>> +   nir_ssa_def *local_size;<br>
>> +<br>
>> +   /*<br>
>> +    * If the local work group size is variable it can't be lowered at this<br>
>> +    * point, but its intrinsic can still be used.<br>
>> +    */<br>
>> +   if (b->shader->info.cs.local_size_variable) {<br>
>> +      local_size = nir_load_local_group_size(b);<br>
>> +   } else {<br>
>> +      nir_const_value local_size_const;<br>
>> +      memset(&local_size_const, 0, sizeof(local_size_const));<br>
>> +      local_size_const.u32[0] = b->shader->info.cs.local_size[0];<br>
>> +      local_size_const.u32[1] = b->shader->info.cs.local_size[1];<br>
>> +      local_size_const.u32[2] = b->shader->info.cs.local_size[2];<br>
>> +      local_size = nir_build_imm(b, 3, 32, local_size_const);<br>
>> +   }<br>
>> +<br>
>> +   return local_size;<br>
>>  }<br>
>><br>
>>  static bool<br>
>> --<br>
>> 2.11.0<br>
>><br>
</blockquote></div></div>