[Mesa-dev] [PATCH 2/2] radv: copy indirect lowering settings from radeonsi

[Timothy Arceri]

On 19/10/17 19:43, Samuel Pitoiset wrote:
> 
> 
> On 10/19/2017 12:27 AM, Timothy Arceri wrote:
>> It looks the original indirect mask was probably copied from
>> ANV.
>>
>> Sascha Willems demo results:
>>
>> tessellation ~4000 -> ~4200 fps
>>
>> V2: continue lowering local indirect due to llvm deficiencies.
>>
>> Cc: Alex Smith <asmith at feralinteractive.com>
>> ---
>>   src/amd/vulkan/radv_shader.c | 27 ++++++++++++++++++++++++++-
>>   1 file changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
>> index 055787a705..faba0c50e9 100644
>> --- a/src/amd/vulkan/radv_shader.c
>> +++ b/src/amd/vulkan/radv_shader.c
>> @@ -238,22 +238,47 @@ radv_shader_compile_to_nir(struct radv_device 
>> *device,
>>           NIR_PASS_V(nir, nir_lower_constant_initializers, ~0);
>>           NIR_PASS_V(nir, nir_lower_system_values);
>>           NIR_PASS_V(nir, nir_lower_clip_cull_distance_arrays);
>>       }
>>       /* Vulkan uses the separate-shader linking model */
>>       nir->info.separate_shader = true;
>>       nir_shader_gather_info(nir, entry_point->impl);
>> +    /* While it would be nice not to have this flag, we are constrained
>> +     * by the reality that LLVM 5.0 doesn't have working VGPR indexing
>> +     * on GFX9.
>> +     */
>> +    bool llvm_has_working_vgpr_indexing =
>> +        device->physical_device->rad_info.chip_class <= VI;
> 
> I would prefer to see a boolean in radv_device instead. That way, all 
> chip-dependent capabilities are defined and grouped there.

This is an llvm limitation rather than a chip limitation if it makes any 
difference.

> 
> For now, patch 1 is:
> 
> Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> 
>> +
>> +    /* TODO: Indirect indexing of GS inputs is unimplemented.
>> +     *
>> +     * TCS and TES load inputs directly from LDS or offchip memory, so
>> +     * indirect indexing is trivial.
>> +     */
>>       nir_variable_mode indirect_mask = 0;
>> -    indirect_mask |= nir_var_shader_in;
>> +    if (nir->stage == MESA_SHADER_GEOMETRY ||
>> +        (nir->stage != MESA_SHADER_TESS_CTRL &&
>> +         nir->stage != MESA_SHADER_TESS_EVAL &&
>> +         !llvm_has_working_vgpr_indexing)) {
>> +        indirect_mask |= nir_var_shader_in;
>> +    }
>> +
>> +    /* TODO: We shouldn't need to do this, however LLVM isn't currently
>> +     * smart enough to handle indirects without causing excess spilling
>> +     * causing the gpu to hang.
>> +     *
>> +     * See the following thread for more details of the problem:
>> +     * 
>> https://lists.freedesktop.org/archives/mesa-dev/2017-July/162106.html
>> +     */
>>       indirect_mask |= nir_var_local;
>>       nir_lower_indirect_derefs(nir, indirect_mask);
>>       static const nir_lower_tex_options tex_options = {
>>         .lower_txp = ~0,
>>       };
>>       nir_lower_tex(nir, &tex_options);
>>