[Mesa-dev] [PATCH 6/6] radeonsi: print LLVM IRs to ddebug logs

[Marek Olšák]

On Sat, Jul 2, 2016 at 11:44 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:
> On 01.07.2016 01:21, Marek Olšák wrote:
>>
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> Getting LLVM IRs of hanging shaders have never been easier.
>> ---
>>   src/gallium/drivers/radeon/r600_pipe_common.c   |  1 +
>>   src/gallium/drivers/radeon/r600_pipe_common.h   |  1 +
>>   src/gallium/drivers/radeonsi/si_pipe.c          |  3 +++
>>   src/gallium/drivers/radeonsi/si_pipe.h          |  1 +
>>   src/gallium/drivers/radeonsi/si_shader.c        | 12 ++++++++++++
>>   src/gallium/drivers/radeonsi/si_state_shaders.c |  9 ++++++++-
>>   6 files changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/drivers/radeon/r600_pipe_common.c
>> b/src/gallium/drivers/radeon/r600_pipe_common.c
>> index 5e981d6..74674e7 100644
>> --- a/src/gallium/drivers/radeon/r600_pipe_common.c
>> +++ b/src/gallium/drivers/radeon/r600_pipe_common.c
>> @@ -67,6 +67,7 @@ void radeon_shader_binary_clean(struct
>> radeon_shader_binary *b)
>>         FREE(b->global_symbol_offsets);
>>         FREE(b->relocs);
>>         FREE(b->disasm_string);
>> +       FREE(b->llvm_ir_string);
>>   }
>>
>>   /*
>> diff --git a/src/gallium/drivers/radeon/r600_pipe_common.h
>> b/src/gallium/drivers/radeon/r600_pipe_common.h
>> index c145dc3..1ad69f8 100644
>> --- a/src/gallium/drivers/radeon/r600_pipe_common.h
>> +++ b/src/gallium/drivers/radeon/r600_pipe_common.h
>> @@ -155,6 +155,7 @@ struct radeon_shader_binary {
>>
>>         /** Disassembled shader in a string. */
>>         char *disasm_string;
>> +       char *llvm_ir_string;
>>   };
>>
>>   void radeon_shader_binary_init(struct radeon_shader_binary *b);
>> diff --git a/src/gallium/drivers/radeonsi/si_pipe.c
>> b/src/gallium/drivers/radeonsi/si_pipe.c
>> index f15e589..ad2a86a 100644
>> --- a/src/gallium/drivers/radeonsi/si_pipe.c
>> +++ b/src/gallium/drivers/radeonsi/si_pipe.c
>> @@ -139,6 +139,9 @@ static struct pipe_context *si_create_context(struct
>> pipe_screen *screen,
>>         if (sscreen->b.debug_flags & DBG_CHECK_VM)
>>                 flags |= PIPE_CONTEXT_DEBUG;
>>
>> +       if (flags & PIPE_CONTEXT_DEBUG)
>> +               sscreen->record_llvm_ir = true; /* racy but not critical
>> */
>> +
>>         sctx->b.b.screen = screen; /* this must be set first */
>>         sctx->b.b.priv = priv;
>>         sctx->b.b.destroy = si_destroy_context;
>> diff --git a/src/gallium/drivers/radeonsi/si_pipe.h
>> b/src/gallium/drivers/radeonsi/si_pipe.h
>> index 9d15cbf..fc7e73e 100644
>> --- a/src/gallium/drivers/radeonsi/si_pipe.h
>> +++ b/src/gallium/drivers/radeonsi/si_pipe.h
>> @@ -87,6 +87,7 @@ struct si_screen {
>>
>>         /* Whether shaders are monolithic (1-part) or separate (3-part).
>> */
>>         bool                            use_monolithic_shaders;
>> +       bool                            record_llvm_ir;
>>
>>         pipe_mutex                      shader_parts_mutex;
>>         struct si_shader_part           *vs_prologs;
>> diff --git a/src/gallium/drivers/radeonsi/si_shader.c
>> b/src/gallium/drivers/radeonsi/si_shader.c
>> index bba6a55..e2aae85 100644
>> --- a/src/gallium/drivers/radeonsi/si_shader.c
>> +++ b/src/gallium/drivers/radeonsi/si_shader.c
>> @@ -6159,6 +6159,12 @@ void si_shader_dump(struct si_screen *sscreen,
>> struct si_shader *shader,
>>                     struct pipe_debug_callback *debug, unsigned processor,
>>                     FILE *file)
>>   {
>> +       if (file != stderr && shader->binary.llvm_ir_string) {
>> +               fprintf(file, "\n%s - main shader part - LLVM IR:\n\n",
>> +                       si_get_shader_name(shader, processor));
>> +               fprintf(file, "%s\n", shader->binary.llvm_ir_string);
>> +       }
>> +
>>         if (file != stderr ||
>>             (r600_can_dump_shader(&sscreen->b, processor) &&
>>              !(sscreen->b.debug_flags & DBG_NO_ASM))) {
>> @@ -6204,6 +6210,12 @@ int si_compile_llvm(struct si_screen *sscreen,
>>                 }
>>         }
>>
>> +       if (sscreen->record_llvm_ir) {
>> +               char *ir = LLVMPrintModuleToString(mod);
>> +               binary->llvm_ir_string = strdup(ir);
>> +               LLVMDisposeMessage(ir);
>> +       }
>> +
>>         if (!si_replace_shader(count, binary)) {
>>                 r = radeon_llvm_compile(mod, binary, tm, debug);
>>                 if (r)
>> diff --git a/src/gallium/drivers/radeonsi/si_state_shaders.c
>> b/src/gallium/drivers/radeonsi/si_state_shaders.c
>> index 4bcdeb6..ed14288 100644
>> --- a/src/gallium/drivers/radeonsi/si_state_shaders.c
>> +++ b/src/gallium/drivers/radeonsi/si_state_shaders.c
>> @@ -96,6 +96,8 @@ static uint32_t *read_chunk(uint32_t *ptr, void **data,
>> unsigned *size)
>>   {
>>         *size = *ptr++;
>>         assert(*data == NULL);
>> +       if (!*size)
>> +               return ptr;
>>         *data = malloc(*size);
>>         return read_data(ptr, *data, *size);
>>   }
>> @@ -110,6 +112,8 @@ static void *si_get_shader_binary(struct si_shader
>> *shader)
>>         unsigned relocs_size = shader->binary.reloc_count *
>>                                sizeof(shader->binary.relocs[0]);
>>         unsigned disasm_size = strlen(shader->binary.disasm_string) + 1;
>> +       unsigned llvm_ir_size = shader->binary.llvm_ir_string ?
>> +                               strlen(shader->binary.llvm_ir_string) + 1
>> : 0;
>
>
> I don't believe it's crashing right now, but this is a bit fragile: in the
> case where llvm_ir_string is originally NULL, when the chunk is read back
> one allocates a 0-sized buffer, which may be a non-NULL pointer at garbage.
>
> I'd be fine with fixing that by changing read_chunk accordingly.

but read_chunk is already fixed above, isn't it?

write_chunk writes size=0 and no data, read_chunk reads size=0,
asserts (data == NULL) and doesn't allocate anything.

Marek