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

[Samuel Pitoiset]

On 06/20/2017 10:53 PM, Samuel Pitoiset wrote:
> 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.

FWIW, "Peace, Death!" and "Riptale" [1] are also affected by the issue 
(all three games use the same engine).

[1] https://www.gamingonlinux.com/wiki/Games_broken_on_Mesa

> 
> 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,
>>