[Mesa-dev] [RFC PATCH 1/4] radv: gather more info about push constants

[Samuel Pitoiset]

On 1/28/19 5:58 PM, Bas Nieuwenhuizen wrote:
> On Fri, Jan 25, 2019 at 5:27 PM Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>> This is needed in order to inline some push constants when possible.
>> This also adds a new helper for initializing the pass.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   src/amd/vulkan/radv_nir_to_llvm.c |  2 ++
>>   src/amd/vulkan/radv_private.h     |  2 ++
>>   src/amd/vulkan/radv_shader.h      |  4 ++++
>>   src/amd/vulkan/radv_shader_info.c | 32 ++++++++++++++++++++++++++++++-
>>   4 files changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/amd/vulkan/radv_nir_to_llvm.c b/src/amd/vulkan/radv_nir_to_llvm.c
>> index 5048d9d2493..b655e2c2e2c 100644
>> --- a/src/amd/vulkan/radv_nir_to_llvm.c
>> +++ b/src/amd/vulkan/radv_nir_to_llvm.c
>> @@ -3439,6 +3439,8 @@ LLVMModuleRef ac_translate_nir_to_llvm(struct ac_llvm_compiler *ac_llvm,
>>
>>          memset(shader_info, 0, sizeof(*shader_info));
>>
>> +       radv_nir_shader_info_init(&shader_info->info);
>> +
>>          for(int i = 0; i < shader_count; ++i)
>>                  radv_nir_shader_info_pass(shaders[i], options, &shader_info->info);
>>
>> diff --git a/src/amd/vulkan/radv_private.h b/src/amd/vulkan/radv_private.h
>> index 85c18906f84..4c76521a045 100644
>> --- a/src/amd/vulkan/radv_private.h
>> +++ b/src/amd/vulkan/radv_private.h
>> @@ -1941,6 +1941,8 @@ void radv_nir_shader_info_pass(const struct nir_shader *nir,
>>                                 const struct radv_nir_compiler_options *options,
>>                                 struct radv_shader_info *info);
>>
>> +void radv_nir_shader_info_init(struct radv_shader_info *info);
>> +
>>   struct radeon_winsys_sem;
>>
>>   #define RADV_DEFINE_HANDLE_CASTS(__radv_type, __VkType)                \
>> diff --git a/src/amd/vulkan/radv_shader.h b/src/amd/vulkan/radv_shader.h
>> index 3652a811e80..0f049f9a528 100644
>> --- a/src/amd/vulkan/radv_shader.h
>> +++ b/src/amd/vulkan/radv_shader.h
>> @@ -162,6 +162,10 @@ struct radv_streamout_info {
>>
>>   struct radv_shader_info {
>>          bool loads_push_constants;
>> +       uint8_t min_push_constant_used;
>> +       uint8_t max_push_constant_used;
> can be 0-256 + an "invalid" value, so need to be uint16_t?
>
>> +       bool has_32bit_push_constants;
>> +       bool has_indirect_push_constants;
>>          uint32_t desc_set_used_mask;
>>          bool needs_multiview_view_index;
>>          bool uses_invocation_id;
>> diff --git a/src/amd/vulkan/radv_shader_info.c b/src/amd/vulkan/radv_shader_info.c
>> index 7e5a3789af2..c9cd5fddc53 100644
>> --- a/src/amd/vulkan/radv_shader_info.c
>> +++ b/src/amd/vulkan/radv_shader_info.c
>> @@ -190,6 +190,30 @@ gather_intrinsic_store_deref_info(const nir_shader *nir,
>>          }
>>   }
>>
>> +static void
>> +gather_push_constant_info(const nir_shader *nir,
>> +                         const nir_intrinsic_instr *instr,
>> +                         struct radv_shader_info *info)
>> +{
>> +       nir_const_value *cval = nir_src_as_const_value(instr->src[0]);
>> +
>> +       if (!cval)
>> +               info->has_indirect_push_constants = true;
>> +
>> +       if (instr->dest.ssa.bit_size == 32)
>> +               info->has_32bit_push_constants = true;
>> +
>> +       int base = nir_intrinsic_base(instr);
>> +       int range = nir_intrinsic_range(instr);
>> +
>> +       if (base + range > info->max_push_constant_used)
>> +               info->max_push_constant_used = base + range;
>> +       if (base < info->min_push_constant_used)
>> +               info->min_push_constant_used = base;
> Use MIN2 and MAX2?
>
>> +
>> +       info->loads_push_constants = true;
>> +}
>> +
>>   static void
>>   gather_intrinsic_info(const nir_shader *nir, const nir_intrinsic_instr *instr,
>>                        struct radv_shader_info *info)
>> @@ -243,7 +267,7 @@ gather_intrinsic_info(const nir_shader *nir, const nir_intrinsic_instr *instr,
>>                  info->uses_prim_id = true;
>>                  break;
>>          case nir_intrinsic_load_push_constant:
>> -               info->loads_push_constants = true;
>> +               gather_push_constant_info(nir, instr, info);
>>                  break;
>>          case nir_intrinsic_vulkan_resource_index:
>>                  info->desc_set_used_mask |= (1 << nir_intrinsic_desc_set(instr));
>> @@ -504,6 +528,12 @@ gather_xfb_info(const nir_shader *nir, struct radv_shader_info *info)
>>          ralloc_free(xfb);
>>   }
>>
>> +void
>> +radv_nir_shader_info_init(struct radv_shader_info *info)
>> +{
>> +       info->min_push_constant_used = -1;
> Can you use one of the appropriate MAX macros from IIRC stddef.h?

Thanks for the review Bas, I will improve and re-send if necessary.

I guess you are okay with the overall idea?

>> +}
>> +
>>   void
>>   radv_nir_shader_info_pass(const struct nir_shader *nir,
>>                            const struct radv_nir_compiler_options *options,
>> --
>> 2.20.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev