[PATCH] D72180: [MC] Add parameter `Address` to MCInstrPrinter::printInstruction

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 3 14:55:16 PST 2020


MaskRay added a comment.

In D72180#1803701 <https://reviews.llvm.org/D72180#1803701>, @rnk wrote:

> So, as is, this is NFC, it just adds a dead parameter to a virtual method, but eventually it will make its way through the layers to at the very least X86InstPrinterCommon::printPCRelImm, which will change llvm-objdump's behavior.
>
> Do you think this will break any users? Should we add a flag to request the old behavior (print only the PC-relative offset)? I guess I suspect users are relying on our current behavior, but will be able to adapt.
>
> I think we should do this, but let's wait for at least one other person to agree.


I ended up creating the two patches after I reviewed D71453 <https://reviews.llvm.org/D71453>. (The llvm-objdump -d `b offset` problem has bother me (and likely many others) a long time.)

My plan is:

- Teach MCInstPrinter::printInst and MCInstrPrinter::printInstruction to take an extra parameter Address (D72172 <https://reviews.llvm.org/D72172> D72180 <https://reviews.llvm.org/D72180>)
- For target ABC, implement `printBranchOperand`, which is similar to `printOperand`, but takes `Address` into account.
- Migrate `ABCInstrInfo.td` direct branch/call instructions to use `PrintMethod = "printBranchOperand"`, instead of the default `"printOperand"`.

For users who want to retain the old behavior (offset instead of address), they can simply pass 0 to `uint64_t Address`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72180/new/

https://reviews.llvm.org/D72180





More information about the llvm-commits mailing list