[Mesa-dev] [PATCH] radv: Always lower indirect derefs after nir_lower_global_vars_to_local.

[Timothy Arceri]

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

On 16/02/18 07:40, Bas Nieuwenhuizen wrote:
> Otherwise new local variables can cause hangs on vega.
> 
> CC: <mesa-stable at lists.freedesktop.org>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105098
> ---
>   src/amd/vulkan/radv_pipeline.c | 11 +++++--
>   src/amd/vulkan/radv_shader.c   | 74 +++++++++++++++++++++++-------------------
>   src/amd/vulkan/radv_shader.h   |  4 +++
>   3 files changed, 53 insertions(+), 36 deletions(-)
> 
> diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
> index 8f872e7c14..0d1958fc93 100644
> --- a/src/amd/vulkan/radv_pipeline.c
> +++ b/src/amd/vulkan/radv_pipeline.c
> @@ -1525,9 +1525,16 @@ radv_link_shaders(struct radv_pipeline *pipeline, nir_shader **shaders)
>   							   ordered_shaders[i - 1]);
>   
>   		if (progress) {
> -			nir_lower_global_vars_to_local(ordered_shaders[i]);
> +			if (nir_lower_global_vars_to_local(ordered_shaders[i])) {
> +				radv_lower_indirect_derefs(ordered_shaders[i],
> +				                           pipeline->device->physical_device);
> +			}
>   			radv_optimize_nir(ordered_shaders[i]);
> -			nir_lower_global_vars_to_local(ordered_shaders[i - 1]);
> +
> +			if (nir_lower_global_vars_to_local(ordered_shaders[i - 1])) {
> +				radv_lower_indirect_derefs(ordered_shaders[i - 1],
> +				                           pipeline->device->physical_device);
> +			}
>   			radv_optimize_nir(ordered_shaders[i - 1]);
>   		}
>   	}
> diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
> index 42f83bb335..d9b8e209a9 100644
> --- a/src/amd/vulkan/radv_shader.c
> +++ b/src/amd/vulkan/radv_shader.c
> @@ -114,6 +114,45 @@ void radv_DestroyShaderModule(
>   	vk_free2(&device->alloc, pAllocator, module);
>   }
>   
> +bool
> +radv_lower_indirect_derefs(struct nir_shader *nir,
> +                           struct radv_physical_device *device)
> +{
> +	/* 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->rad_info.chip_class <= VI;
> +
> +	/* 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;
> +	if (nir->info.stage == MESA_SHADER_GEOMETRY ||
> +	    (nir->info.stage != MESA_SHADER_TESS_CTRL &&
> +	     nir->info.stage != MESA_SHADER_TESS_EVAL &&
> +	     !llvm_has_working_vgpr_indexing)) {
> +		indirect_mask |= nir_var_shader_in;
> +	}
> +	if (!llvm_has_working_vgpr_indexing &&
> +	    nir->info.stage != MESA_SHADER_TESS_CTRL)
> +		indirect_mask |= nir_var_shader_out;
> +
> +	/* 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;
> +
> +	return nir_lower_indirect_derefs(nir, indirect_mask);
> +}
> +
>   void
>   radv_optimize_nir(struct nir_shader *shader)
>   {
> @@ -254,40 +293,6 @@ radv_shader_compile_to_nir(struct radv_device *device,
>   
>   	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;
> -
> -	/* 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;
> -	if (nir->info.stage == MESA_SHADER_GEOMETRY ||
> -	    (nir->info.stage != MESA_SHADER_TESS_CTRL &&
> -	     nir->info.stage != MESA_SHADER_TESS_EVAL &&
> -	     !llvm_has_working_vgpr_indexing)) {
> -		indirect_mask |= nir_var_shader_in;
> -	}
> -	if (!llvm_has_working_vgpr_indexing &&
> -	    nir->info.stage != MESA_SHADER_TESS_CTRL)
> -		indirect_mask |= nir_var_shader_out;
> -
> -	/* 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,
>   	};
> @@ -298,6 +303,7 @@ radv_shader_compile_to_nir(struct radv_device *device,
>   	nir_lower_var_copies(nir);
>   	nir_lower_global_vars_to_local(nir);
>   	nir_remove_dead_variables(nir, nir_var_local);
> +	radv_lower_indirect_derefs(nir, device->physical_device);
>   	radv_optimize_nir(nir);
>   
>   	return nir;
> diff --git a/src/amd/vulkan/radv_shader.h b/src/amd/vulkan/radv_shader.h
> index b07f8a89e7..31fe739771 100644
> --- a/src/amd/vulkan/radv_shader.h
> +++ b/src/amd/vulkan/radv_shader.h
> @@ -104,6 +104,10 @@ void
>   radv_shader_variant_destroy(struct radv_device *device,
>   			    struct radv_shader_variant *variant);
>   
> +bool
> +radv_lower_indirect_derefs(struct nir_shader *nir,
> +                           struct radv_physical_device *device);
> +
>   const char *
>   radv_get_shader_name(struct radv_shader_variant *var, gl_shader_stage stage);
>   
>