This is an archive of the discontinued LLVM Phabricator instance.

[MCInstPrinter] Pass `Address` parameter to MCOI::OPERAND_PCREL typed operands. NFC
ClosedPublic

Authored by MaskRay on Mar 22 2020, 2:46 PM.

Details

Summary

Follow-up of D72172 and D72180

This patch passes uint64_t Address to print methods of PC-relative
operands so that subsequent target specific patches can change
*InstPrinter::print{Operand,PCRelImm,...} to customize the output.

Add MCInstPrinter::PrintBranchImmAsAddress which is set to true by
llvm-objdump.

// Current llvm-objdump -d output
aarch64: 20000: bl #0
ppc:     20000: bl .+4
x86:     20000: callq 0

// Ideal output
aarch64: 20000: bl 0x20000
ppc:     20000: bl 0x20004
x86:     20000: callq 0x20005

// GNU objdump -d. The lack of 0x is not ideal because the result cannot be re-assembled
aarch64: 20000: bl 20000
ppc:     20000: bl 0x20004
x86:     20000: callq 20005

In lib/Target/X86/X86GenAsmWriter1.inc (generated by llvm-tblgen -gen-asm-writer):

   case 12:
     // CALL64pcrel32, CALLpcrel16, CALLpcrel32, EH_SjLj_Setup, JCXZ, JECXZ, J...
-    printPCRelImm(MI, 0, O);
+    printPCRelImm(MI, Address, 0, O);
     return;

Some targets have 2 printOperand overloads, one without Address and
one with Address. They should annotate derived Operand properly with
let OperandType = "OPERAND_PCREL".

Diff Detail

Event Timeline

MaskRay created this revision.Mar 22 2020, 2:46 PM

What, if any, are the actual behaviour changes in this patch? I don't see any test updates, but the patch isn't labelled NFC.

llvm/lib/Target/AArch64/MCTargetDesc/AArch64InstPrinter.cpp
1350–1351

Address is unused. Is this going to cause an unused variable warning?

1365–1366

Address1 doesn't clearly distinguish to me why the passed-in address cannot be used instead. Perhaps you need to change the name of this variable here.

llvm/lib/Target/AVR/MCTargetDesc/AVRInstPrinter.h
41

Address is unused here.

llvm/lib/Target/SystemZ/MCTargetDesc/SystemZInstPrinter.cpp
159

Ditto.

MaskRay updated this revision to Diff 252339.Mar 24 2020, 8:49 AM
MaskRay marked 4 inline comments as done.
MaskRay retitled this revision from [MCInstPrinter] Pass `Address` parameter to MCOI::OPERAND_PCREL typed operands to [MCInstPrinter] Pass `Address` parameter to MCOI::OPERAND_PCREL typed operands. NFC.

Address comments. Add NFC tag

llvm/lib/Target/AArch64/MCTargetDesc/AArch64InstPrinter.cpp
1350–1351

LLVM CMake/gn enables -Wno-unused-parameter. This is somewhat common in LLVM.

/*Address*/ is not used to reduce diff when a future change adds the support.

llvm/lib/Target/SystemZ/MCTargetDesc/SystemZInstPrinter.cpp
159

Here /*Address*/ is used so that when the support is added, the code doesn't need to touch this line.

MaskRay updated this revision to Diff 252406.Mar 24 2020, 12:34 PM
MaskRay edited the summary of this revision. (Show Details)

Add PrintBranchImmAsAddress

This revision is now accepted and ready to land.Mar 26 2020, 1:46 AM
MaskRay updated this revision to Diff 252853.Mar 26 2020, 8:17 AM
MaskRay edited the summary of this revision. (Show Details)

Fix description. powerpc64-linux-gnu-objdump prints 0x prefix

This revision was automatically updated to reflect the committed changes.