Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[XCOFF] Display branch-absolute targets in hex. #72532

Merged
merged 8 commits into from
Dec 13, 2023

Conversation

stephenpeckham
Copy link
Contributor

Branch-absolute instructions are currently printed in decimal, and negative addresses are printed as positive numbers.

With this change, addresses are printed in hex and negative addresses are converted to an unsigned 32- or 64-bit address.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 16, 2023

@llvm/pr-subscribers-mc
@llvm/pr-subscribers-lld
@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-llvm-binary-utilities

Author: None (stephenpeckham)

Changes

Branch-absolute instructions are currently printed in decimal, and negative addresses are printed as positive numbers.

With this change, addresses are printed in hex and negative addresses are converted to an unsigned 32- or 64-bit address.


Full diff: https://github.com/llvm/llvm-project/pull/72532.diff

5 Files Affected:

  • (modified) llvm/lib/Target/PowerPC/MCTargetDesc/PPCInstPrinter.cpp (+4-1)
  • (modified) llvm/lib/Target/PowerPC/PPCRegisterInfo.td (+3)
  • (added) llvm/test/tools/llvm-objdump/XCOFF/Inputs/abs32.o ()
  • (added) llvm/test/tools/llvm-objdump/XCOFF/Inputs/abs64.o ()
  • (added) llvm/test/tools/llvm-objdump/XCOFF/disassemble-abs.test (+37)
diff --git a/llvm/lib/Target/PowerPC/MCTargetDesc/PPCInstPrinter.cpp b/llvm/lib/Target/PowerPC/MCTargetDesc/PPCInstPrinter.cpp
index ccbb650c65365b4..f0a5cfcd5a78878 100644
--- a/llvm/lib/Target/PowerPC/MCTargetDesc/PPCInstPrinter.cpp
+++ b/llvm/lib/Target/PowerPC/MCTargetDesc/PPCInstPrinter.cpp
@@ -484,7 +484,10 @@ void PPCInstPrinter::printAbsBranchOperand(const MCInst *MI, unsigned OpNo,
   if (!MI->getOperand(OpNo).isImm())
     return printOperand(MI, OpNo, STI, O);
 
-  O << SignExtend32<32>((unsigned)MI->getOperand(OpNo).getImm() << 2);
+  uint64_t Imm = MI->getOperand(OpNo).getImm() << 2;
+  if (!TT.isPPC64())
+    Imm &= 0xffffffff;
+  O << formatHex(Imm);
 }
 
 void PPCInstPrinter::printcrbitm(const MCInst *MI, unsigned OpNo,
diff --git a/llvm/lib/Target/PowerPC/PPCRegisterInfo.td b/llvm/lib/Target/PowerPC/PPCRegisterInfo.td
index 6151faf403aaaf1..375e63654db1184 100644
--- a/llvm/lib/Target/PowerPC/PPCRegisterInfo.td
+++ b/llvm/lib/Target/PowerPC/PPCRegisterInfo.td
@@ -798,6 +798,7 @@ def directbrtarget : Operand<OtherVT> {
 def absdirectbrtarget : Operand<OtherVT> {
   let PrintMethod = "printAbsBranchOperand";
   let EncoderMethod = "getAbsDirectBrEncoding";
+  let DecoderMethod = "decodeDirectBrTarget";
   let ParserMatchClass = PPCDirectBrAsmOperand;
 }
 def PPCCondBrAsmOperand : AsmOperandClass {
@@ -814,6 +815,7 @@ def condbrtarget : Operand<OtherVT> {
 def abscondbrtarget : Operand<OtherVT> {
   let PrintMethod = "printAbsBranchOperand";
   let EncoderMethod = "getAbsCondBrEncoding";
+  let DecoderMethod = "decodeCondBrTarget";
   let ParserMatchClass = PPCCondBrAsmOperand;
 }
 def calltarget : Operand<iPTR> {
@@ -826,6 +828,7 @@ def calltarget : Operand<iPTR> {
 def abscalltarget : Operand<iPTR> {
   let PrintMethod = "printAbsBranchOperand";
   let EncoderMethod = "getAbsDirectBrEncoding";
+  let DecoderMethod = "decodeDirectBrTarget";
   let ParserMatchClass = PPCDirectBrAsmOperand;
 }
 def PPCCRBitMaskOperand : AsmOperandClass {
diff --git a/llvm/test/tools/llvm-objdump/XCOFF/Inputs/abs32.o b/llvm/test/tools/llvm-objdump/XCOFF/Inputs/abs32.o
new file mode 100644
index 000000000000000..eda8e461736e033
Binary files /dev/null and b/llvm/test/tools/llvm-objdump/XCOFF/Inputs/abs32.o differ
diff --git a/llvm/test/tools/llvm-objdump/XCOFF/Inputs/abs64.o b/llvm/test/tools/llvm-objdump/XCOFF/Inputs/abs64.o
new file mode 100644
index 000000000000000..ae9e0e87d001730
Binary files /dev/null and b/llvm/test/tools/llvm-objdump/XCOFF/Inputs/abs64.o differ
diff --git a/llvm/test/tools/llvm-objdump/XCOFF/disassemble-abs.test b/llvm/test/tools/llvm-objdump/XCOFF/disassemble-abs.test
new file mode 100644
index 000000000000000..dc83b3540ca0a76
--- /dev/null
+++ b/llvm/test/tools/llvm-objdump/XCOFF/disassemble-abs.test
@@ -0,0 +1,37 @@
+# RUN: llvm-objdump -d %p/Inputs/abs32.o | \
+# RUN:   FileCheck %s
+
+# RUN: llvm-objdump -d %p/Inputs/abs64.o | \
+# RUN:   FileCheck --check-prefixes=CHECK64 %s
+
+## Object files assembled on AIX from the following source:
+##        .csect [PR]
+##.main:
+##        .globl .main
+##        .extern .function
+##        bla     .function
+##        btla    .function
+##        ba      0x1234
+##        ba      -32
+##        bta     0x2348
+##        bta     -256
+
+CHECK:        Inputs/abs32.o:	file format aixcoff-rs6000
+CHECK:        Disassembly of section .text:
+CHECK:        00000000 <.main>:
+CHECK:            0: 48 00 00 03   bla 0x0
+CHECK-NEXT:       4: 41 80 00 03   btla    0, 0x0
+CHECK-NEXT:       8: 48 00 12 36   ba 0x1234
+CHECK-NEXT:       c: 4b ff ff e2   ba 0xffffffe0
+CHECK-NEXT:      10: 41 80 23 4a   bta     0, 0x2348
+CHECK-NEXT:      14: 41 80 ff 02   bta     0, 0xffffff00
+
+CHECK64:      Inputs/abs64.o: file format aix5coff64-rs6000
+CHECK64:      Disassembly of section .text:
+CHECK64:      0000000000000000 <.main>:
+CHECK64-NEXT:       0: 48 00 00 03   bla 0x0
+CHECK64-NEXT:       4: 41 80 00 03   btla    0, 0x0
+CHECK64-NEXT:       8: 48 00 12 36   ba 0x1234
+CHECK64-NEXT:       c: 4b ff ff e2   ba 0xffffffffffffffe0
+CHECK64-NEXT:      10: 41 80 23 4a   bta     0, 0x2348
+CHECK64-NEXT:      14: 41 80 ff 02   bta     0, 0xffffffffffffff00

Copy link

github-actions bot commented Nov 22, 2023

✅ With the latest revision this PR passed the Python code formatter.

Copy link

github-actions bot commented Nov 22, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@llvmbot llvmbot added the mc Machine (object) code label Nov 22, 2023
@ldionne ldionne removed request for a team November 22, 2023 23:32
@nikic nikic removed their request for review November 24, 2023 09:47
@antiagainst antiagainst removed their request for review November 27, 2023 04:25
@@ -0,0 +1,37 @@
# RUN: llvm-objdump -d %p/Inputs/abs32.o | \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use yaml2obj to generate the object? Raw object input should always be avoided.

obj2yaml  abs32.o -o m.yaml
yaml2obj m.yaml -o t.o
$llvm-objdump -d t.o

t.o:	file format aixcoff-rs6000

Disassembly of section .text:

00000000 <.main>:
       0: 48 00 00 03  	bla 0
       4: 41 80 00 03  	btla	0, 0
       8: 48 00 12 36  	ba 4660
       c: 4b ff ff e2  	ba 67108832
      10: 41 80 23 4a  	bta	0, 9032
      14: 41 80 ff 02  	bta	0, 65280

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. It looks like obj2yaml has some bugs. The generated object files are not valid (I cannot run ld -r t.o). For the 64-bit testcase, llvm-objdump fails. I'll see what I can do.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to generate an object that can pass ld -r for this patch? I assume we only need to correctly dump the assembly code?

Below yaml for XCOFF64 can get an object which can be disassembled by llvm-objdump.

--- !XCOFF
FileHeader:
  MagicNumber:     0x1F7
  NumberOfSections: 1
  CreationTime:    1701859282
  AuxiliaryHeaderSize: 0
  Flags:           0x0
Sections:
  - Name:            .text
    Address:         0x0
    Size:            0x18
    FileOffsetToData: 0x60
    FileOffsetToRelocations: 0x78
    FileOffsetToLineNumbers: 0x0
    NumberOfRelocations: 0x2
    NumberOfLineNumbers: 0x0
    Flags:           [ STYP_TEXT ]
    SectionData:     4800000341800003480012364BFFFFE24180234A4180FF02
    Relocations:
      - Address:         0x0
        Symbol:          0x1
        Info:            0x99
        Type:            0x18
      - Address:         0x6
        Symbol:          0x1
        Info:            0x8F
        Type:            0x18
Symbols:
  - Name:            .file
    Section:         N_DEBUG
    StorageClass:    C_FILE
    NumberOfAuxEntries: 0
  - Name:            .function
    Section:         N_UNDEF
    StorageClass:    C_EXT
    NumberOfAuxEntries: 1
    AuxEntries:
      - Type:                   AUX_CSECT
        StorageMappingClass:    XMC_PR
        SymbolAlignmentAndType: 0
  - Name:            ''
    Section:         .text
    StorageClass:    C_HIDEXT
    NumberOfAuxEntries: 1
    AuxEntries:
      - Type:                   AUX_CSECT
        StorageMappingClass:    XMC_PR
        SymbolAlignmentAndType: 1
  - Name:            .main
    Section:         .text
    StorageClass:    C_EXT
    NumberOfAuxEntries: 1
    AuxEntries:
      - Type:                   AUX_CSECT
        StorageMappingClass:    XMC_PR
        SymbolAlignmentAndType: 2
StringTable:
  Strings:
    - .file
    - .function
    - .text
    - .main

and below yaml is for XCOFF32:

--- !XCOFF
FileHeader:
  MagicNumber:     0x1DF
  NumberOfSections: 1
  CreationTime:    1701859282
  AuxiliaryHeaderSize: 0
  Flags:           0x0
Sections:
  - Name:            .text
    Address:         0x0
    Size:            0x18
    FileOffsetToData: 0x3C
    FileOffsetToRelocations: 0x54
    FileOffsetToLineNumbers: 0x0
    NumberOfRelocations: 0x2
    NumberOfLineNumbers: 0x0
    Flags:           [ STYP_TEXT ]
    SectionData:     4800000341800003480012364BFFFFE24180234A4180FF02
    Relocations:
      - Address:         0x0
        Symbol:          0x4
        Info:            0x99
        Type:            0x18
      - Address:         0x6
        Symbol:          0x4
        Info:            0x8F
        Type:            0x18
Symbols:
  - Name:            ''
    Section:         .text
    StorageClass:    C_HIDEXT
    NumberOfAuxEntries: 1
    AuxEntries:
      - Type:                   AUX_CSECT
        StorageMappingClass:    XMC_PR
        SymbolAlignmentAndType: 1
  - Name:            .main
    Section:         .text
    StorageClass:    C_EXT
    NumberOfAuxEntries: 1
    AuxEntries:
      - Type:                   AUX_CSECT
        StorageMappingClass:    XMC_PR
        SymbolAlignmentAndType: 2
  - Name:            .function
    Section:         N_UNDEF
    StorageClass:    C_EXT
    NumberOfAuxEntries: 1
    AuxEntries:
      - Type:                   AUX_CSECT
        StorageMappingClass:    XMC_PR
        SymbolAlignmentAndType: 0

@stephenpeckham
Copy link
Contributor Author

Thanks for your help @chenzheng1030. I have deleted the object files and I'm using yaml2obj.

Copy link
Collaborator

@chenzheng1030 chenzheng1030 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for making this improvement.

@@ -798,6 +798,7 @@ def directbrtarget : Operand<OtherVT> {
def absdirectbrtarget : Operand<OtherVT> {
let PrintMethod = "printAbsBranchOperand";
let EncoderMethod = "getAbsDirectBrEncoding";
let DecoderMethod = "decodeDirectBrTarget";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a bug related to branch instruction decode?
Normally we need another patch to fix this as the fix here is not same with this patch's purpose.
Since this change is quite small, let's put them together.

@stephenpeckham stephenpeckham merged commit 2fd7657 into llvm:main Dec 13, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants