On Sat, Jul 2, 2016 at 11:44 AM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
> On 01.07.2016 01:21, Marek Olšák wrote:
>>
>> From: Marek Olšák <marek.ol...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
