[Mesa-dev] [PATCH] radv: remove dead variables after splitting per member structs

[Timothy Arceri]

On 23/08/18 09:57, Timothy Arceri wrote:
> On 22/08/18 20:34, Samuel Pitoiset wrote:
>> Otherwise, nir_lower_clip_cull_distance_arrays might report
>> wrong number of output clips/culls because it relies on
>> shader output variables and some of them might be dead.
>>
>> This fixes a rendering issue with Dolphin and Super Mario
>> Sunshine.
>>
>> Fixes: b0c643d8f5 ("spirv: Use NIR per-member splitting")
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107610
>> C: 18.2 <mesa-stable at lists.freedesktop.org>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/amd/vulkan/radv_shader.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
>> index 7d4265cfda..207e5b050e 100644
>> --- a/src/amd/vulkan/radv_shader.c
>> +++ b/src/amd/vulkan/radv_shader.c
>> @@ -259,9 +259,6 @@ radv_shader_compile_to_nir(struct radv_device 
>> *device,
>>            */
>>           NIR_PASS_V(nir, nir_lower_constant_initializers, 
>> nir_var_shader_out);
>> -        NIR_PASS_V(nir, nir_remove_dead_variables,
>> -                   nir_var_shader_in | nir_var_shader_out | 
>> nir_var_system_value);
>> -
>>           /* Now that we've deleted all but the main function, we can 
>> go ahead and
>>            * lower the rest of the constant initializers.
>>            */
>> @@ -273,6 +270,9 @@ radv_shader_compile_to_nir(struct radv_device 
>> *device,
>>           NIR_PASS_V(nir, nir_split_var_copies);
>>           NIR_PASS_V(nir, nir_split_per_member_structs);
> 
> I see you already pushed but I would have suggested adding a comment 
> here saying this must be called after nir_split_per_member_structs.

And obviously including the 'why' we need to call it after 
nir_split_per_member_structs too.

  So
> this didn't get accidentally moved again in future. Seems like a subtle 
> bug to track down. Any patch to add the comment would have my r-b.
> 
>> +        NIR_PASS_V(nir, nir_remove_dead_variables,
>> +                   nir_var_shader_in | nir_var_shader_out | 
>> nir_var_system_value);
>> +
>>           NIR_PASS_V(nir, nir_lower_system_values);
>>           NIR_PASS_V(nir, nir_lower_clip_cull_distance_arrays);
>>       }
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev