Page MenuHomePhabricator

[MC][AMDGPU][llvm-objdump] Synthesized local labels in disassembly
ClosedPublic

Authored by tpr on Apr 23 2021, 1:05 AM.

Details

Summary
  1. Add an accessor function to MCSymbolizer to retrieve addresses referenced by a symbolizable operand, but not resolved to a symbol. That way, the caller can synthesize labels at those addresses and then retry disassembling the section.
  1. Implement that in AMDGPU -- a failed symbol lookup results in the address being added to a vector returned by the new function.
  1. Use that in llvm-objdump when using MCSymbolizer (which only happens on AMDGPU).

Change-Id: I19087c3bbfece64bad5a56ee88bcc9110d83989e

Diff Detail

Event Timeline

tpr created this revision.Apr 23 2021, 1:05 AM
tpr requested review of this revision.Apr 23 2021, 1:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 23 2021, 1:05 AM
rochauha added inline comments.Apr 23 2021, 3:34 AM
llvm/tools/llvm-objdump/llvm-objdump.cpp
1199

I think it would help if this logic is moved to a separate function in the tool, that is called conditionally. It would make the main loop shorter and perhaps also easier to read?

rochauha added inline comments.Apr 23 2021, 3:44 AM
llvm/tools/llvm-objdump/llvm-objdump.cpp
1200–1237

I meant this entire snippet in my previous comment, apologies for causing confusion.

The test in this patch runs with --symbolize-operands.

But we are symbolizing even when --symbolize-operands is not passed to llvm-objdump. Though this doesn't do any harm, I'd lean towards symbolizing only when the switch is passed.

foad added a subscriber: foad.Apr 23 2021, 7:52 AM
tpr updated this revision to Diff 340101.Apr 23 2021, 10:19 AM

V2: Extract AMDGPU-add-symbolizer code and my new logic into a separate
function. Only enable new functionality with SymbolizeOperands to bring
it into line with X86 (and stop existing tests failing).

tpr marked an inline comment as done.Apr 23 2021, 10:24 AM
tpr added inline comments.
llvm/tools/llvm-objdump/llvm-objdump.cpp
1199

I extracted the whole AMDGPU adding symbolizer stuff, including my new code, into a new func. Is that ok?

rochauha accepted this revision.Apr 25 2021, 10:12 AM

LGTM :)

This revision is now accepted and ready to land.Apr 25 2021, 10:12 AM
foad added inline comments.May 3 2021, 6:07 AM
llvm/tools/llvm-objdump/llvm-objdump.cpp
1010–1012

Can't you populate this on construction? std::vector<uint64_t> LabelAddrs(LabelAddrsRef.begin(), LabelAddrsRef.end());