Page MenuHomePhabricator

[AMDGPU] Fixed +DumpCode
ClosedPublic

Authored by tpr on Apr 15 2019, 3:16 AM.

Details

Summary

The +DumpCode attribute is a horrible hack in AMDGPU to embed the
disassembly of the generated code into the elf file. It is used by LLPC
to implement an extension that allows the application to read back the
disassembly of the code. Longer term, we should re-implement that by
using the LLVM disassembler from the Vulkan driver.

Recent LLVM changes broke +DumpCode. With -filetype=asm it crashed, and
with -filetype=obj I think it did not include any instructions, only the
labels. Fixed with this commit: now it has no effect with -filetype=asm,
and works as intended with -filetype=obj.

Change-Id: I6436d86fe2ea220d74a643a85e64753747c9366b

Diff Detail

Repository
rL LLVM

Event Timeline

tpr created this revision.Apr 15 2019, 3:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 15 2019, 3:16 AM

Do we really still need DumpCode? I've long wanted to remove it. Is there any real obstacle remaining to users switching to some combination of the assembler and disassembler?

tpr added a comment.Apr 15 2019, 6:13 AM

I think the only obstacle is getting round to doing the LLPC changes. So this is hopefully a short term fix until we can get around to it.

tpr added a comment.Apr 15 2019, 6:14 AM

PS I'm only speaking for LLPC. I don't know if Mesa uses it.

Mesa uses +DumpCode as well. Do we have a good solution for generation both ELF and textual output?

Mesa uses +DumpCode as well. Do we have a good solution for generation both ELF and textual output?

Yes, depending on what you mean by "good": just codegen to ASM, then assemble that to the ELF? One codegen gets you both the text and object without the need for the hack.

As far as the review goes, it looks like a hack to setUseAssemblerInfoForParsing here, but as long as the code behaves correctly in the cases where it is unset (seems to be just inline assembly?) then it is just a continuation of the +DumpCode hack, so it seems fine to me.

tpr added a comment.May 8 2019, 2:36 AM

Ping: Could someone approve this please? Thanks.

scott.linder accepted this revision.May 8 2019, 11:38 AM
This revision is now accepted and ready to land.May 8 2019, 11:38 AM
Closed by commit rL360688: [AMDGPU] Fixed +DumpCode (authored by tpr, committed by ). · Explain WhyMay 14 2019, 9:14 AM
This revision was automatically updated to reflect the committed changes.