[Mesa-dev] [PATCH] radeonsi: use the correct LLVMTargetMachineRef in si_build_shader_variant

[Samuel Pitoiset]

I have just tested this patch with Hero Siege, it still crashes.

As you said, it's definitely a multithreading issue because it crashes 
differently all the time.

Let me know if you need more information.

On 06/19/2017 01:40 PM, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
> 
> si_build_shader_variant can actually be called directly from one of
> normal-priority compiler threads. In that case, the thread_index is
> only valid for the normal tm array.
> 
> v2:
> - use the correct sel/shader->compiler_ctx_state
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101384 (maybe)
> Fixes: 86cc8097266c ("radeonsi: use a compiler queue with a low priority for optimized shaders")
> ---
>   src/gallium/drivers/radeonsi/si_state_shaders.c | 28 +++++++++++++++++++------
>   1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c b/src/gallium/drivers/radeonsi/si_state_shaders.c
> index 677a6de..6619d96 100644
> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
> @@ -1438,35 +1438,42 @@ static inline void si_shader_selector_key(struct pipe_context *ctx,
>   		break;
>   	}
>   	default:
>   		assert(0);
>   	}
>   
>   	if (unlikely(sctx->screen->b.debug_flags & DBG_NO_OPT_VARIANT))
>   		memset(&key->opt, 0, sizeof(key->opt));
>   }
>   
> -static void si_build_shader_variant(void *job, int thread_index)
> +static void si_build_shader_variant(struct si_shader *shader,
> +				    int thread_index,
> +				    bool low_priority)
>   {
> -	struct si_shader *shader = (struct si_shader *)job;
>   	struct si_shader_selector *sel = shader->selector;
>   	struct si_screen *sscreen = sel->screen;
>   	LLVMTargetMachineRef tm;
>   	struct pipe_debug_callback *debug = &shader->compiler_ctx_state.debug;
>   	int r;
>   
>   	if (thread_index >= 0) {
> -		assert(thread_index < ARRAY_SIZE(sscreen->tm_low_priority));
> -		tm = sscreen->tm_low_priority[thread_index];
> +		if (low_priority) {
> +			assert(thread_index < ARRAY_SIZE(sscreen->tm_low_priority));
> +			tm = sscreen->tm_low_priority[thread_index];
> +		} else {
> +			assert(thread_index < ARRAY_SIZE(sscreen->tm));
> +			tm = sscreen->tm[thread_index];
> +		}
>   		if (!debug->async)
>   			debug = NULL;
>   	} else {
> +		assert(!low_priority);
>   		tm = shader->compiler_ctx_state.tm;
>   	}
>   
>   	r = si_shader_create(sscreen, tm, shader, debug);
>   	if (unlikely(r)) {
>   		R600_ERR("Failed to build shader variant (type=%u) %d\n",
>   			 sel->type, r);
>   		shader->compilation_failed = true;
>   		return;
>   	}
> @@ -1476,20 +1483,29 @@ static void si_build_shader_variant(void *job, int thread_index)
>   					 &shader->shader_log_size);
>   		if (f) {
>   			si_shader_dump(sscreen, shader, NULL, sel->type, f, false);
>   			fclose(f);
>   		}
>   	}
>   
>   	si_shader_init_pm4_state(sscreen, shader);
>   }
>   
> +static void si_build_shader_variant_low_priority(void *job, int thread_index)
> +{
> +	struct si_shader *shader = (struct si_shader *)job;
> +
> +	assert(thread_index >= 0);
> +
> +	si_build_shader_variant(shader, thread_index, true);
> +}
> +
>   static const struct si_shader_key zeroed;
>   
>   static bool si_check_missing_main_part(struct si_screen *sscreen,
>   				       struct si_shader_selector *sel,
>   				       struct si_compiler_ctx_state *compiler_state,
>   				       struct si_shader_key *key)
>   {
>   	struct si_shader **mainp = si_get_main_shader_part(sel, key);
>   
>   	if (!*mainp) {
> @@ -1681,30 +1697,30 @@ again:
>   		sel->last_variant = shader;
>   	}
>   
>   	/* If it's an optimized shader, compile it asynchronously. */
>   	if (shader->is_optimized &&
>   	    !is_pure_monolithic &&
>   	    thread_index < 0) {
>   		/* Compile it asynchronously. */
>   		util_queue_add_job(&sscreen->shader_compiler_queue_low_priority,
>   				   shader, &shader->optimized_ready,
> -				   si_build_shader_variant, NULL);
> +				   si_build_shader_variant_low_priority, NULL);
>   
>   		/* Use the default (unoptimized) shader for now. */
>   		memset(&key->opt, 0, sizeof(key->opt));
>   		mtx_unlock(&sel->mutex);
>   		goto again;
>   	}
>   
>   	assert(!shader->is_optimized);
> -	si_build_shader_variant(shader, thread_index);
> +	si_build_shader_variant(shader, thread_index, false);
>   
>   	if (!shader->compilation_failed)
>   		state->current = shader;
>   
>   	mtx_unlock(&sel->mutex);
>   	return shader->compilation_failed ? -1 : 0;
>   }
>   
>   static int si_shader_select(struct pipe_context *ctx,
>   			    struct si_shader_ctx_state *state,
>