This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Fix +DumpCode to print an entry label for the first function
ClosedPublic

Authored by foad on Jun 24 2019, 6:00 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.

It tries to print an entry label at the start of every function, but
that didn't work for the first function in the module because
DumpCodeInstEmitter wasn't initialised until EmitFunctionBodyStart
which is too late.

Change-Id: I790d73ddf4f51fd02ab32529380c7cb7c607c4ee

Diff Detail

Repository
rL LLVM

Event Timeline

foad created this revision.Jun 24 2019, 6:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 24 2019, 6:00 AM

I'm not a huge fan of new patches for DumpCode. Can we finally just delete it? It shouldn't be difficult for users to switch to using the proper assembler/disassembler

tpr added a comment.Jun 24 2019, 6:11 AM

I don't think anyone is a fan of dumpcode. But we're still in the position that the proper disassembler does not support gfx6 or gfx7, and we need to get this particular problem fixed in the short term.

arsenm added a comment.EditedJun 24 2019, 2:25 PM
In D63712#1555460, @tpr wrote:

I don't think anyone is a fan of dumpcode. But we're still in the position that the proper disassembler does not support gfx6 or gfx7, and we need to get this particular problem fixed in the short term.

You should go the other way, emit text asm and then assemble

tpr added a comment.Jun 24 2019, 2:31 PM

Possibly, but our majority use case is to not need the disassembly, and our thinking is that we don't want to change the tool flow between the majority case and the minority case. So can we get this fix in please?

arsenm accepted this revision.Jun 26 2019, 1:32 PM

LGTM, but I really don't like how this keeps getting put off

This revision is now accepted and ready to land.Jun 26 2019, 1:32 PM
This revision was automatically updated to reflect the committed changes.