[Mesa-dev] [PATCH] radv: include LLVM IR in the VK_AMD_shader_info "disassembly"

[Samuel Pitoiset]

On 11/9/18 2:53 PM, Haehnle, Nicolai wrote:
> On 08.11.18 20:39, Samuel Pitoiset wrote:
>>
>>
>> On 11/8/18 4:15 PM, Nicolai Hähnle wrote:
>>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>>
>>> Helpful for debugging compiler backend problems: this allows us to
>>> easily retrieve the LLVM IR from RenderDoc.
>>> -- 
>>> For the peanut gallery: AMD's official stance on radv hasn't changed.
>>> But we take regressions for radv caused by our changes in LLVM seriously.
>>> After all, they might just as well affect anything else using the same
>>> compiler backend...
>>
>> Thanks a lot for looking at those regressions. Your help is highly
>> appreciated.
>>
>> One comment below.
>>
>>> ---
>>>    src/amd/vulkan/radv_shader.c | 1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
>>> index f98ca6b4edd..1c51297d202 100644
>>> --- a/src/amd/vulkan/radv_shader.c
>>> +++ b/src/amd/vulkan/radv_shader.c
>>> @@ -855,20 +855,21 @@ radv_GetShaderInfoAMD(VkDevice _device,
>>>                    result = VK_INCOMPLETE;
>>>            }
>>>            break;
>>>        case VK_SHADER_INFO_TYPE_DISASSEMBLY_AMD:
>>>            buf = _mesa_string_buffer_create(NULL, 1024);
>>>            _mesa_string_buffer_printf(buf, "%s:\n",
>>> radv_get_shader_name(variant, stage));
>>>            _mesa_string_buffer_printf(buf, "%s\n\n",
>>> variant->disasm_string);
>>>            generate_shader_stats(device, variant, stage, buf);
>>> +        _mesa_string_buffer_printf(buf, "\n\n%s\n\n",
>>> variant->llvm_ir_string);
>>
>> I would prefer to put the LLVM IR just before the assembly to be
>> consistent with the output of RADV_DEBUG=shaders.
> 
> Sure, I can do that. I chose this order simply because I ran into this
> from the RenderDoc use case, where I'd expect people are more interested
> in the GCN assembly upfront. But it doesn't really matter, so I'll
> change it :)

Yeah, it doesn't really matter much. :)

> 
> Cheers,
> Nicolai
> 
> 
>> Other than that, patch is:
>>
>> Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>
>>>            /* Need to include the null terminator. */
>>>            size_t length = buf->length + 1;
>>>            if (!pInfo) {
>>>                *pInfoSize = length;
>>>            } else {
>>>                size_t size = *pInfoSize;
>>>                *pInfoSize = length;
>>>