[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