[XCOFF][AIX] Relocation support for SymB

[jasonliu]

Summary

This patch intends to provide relocation support for the expression contains two unpaired relocatable terms with opposite signs.

[DiggerLin]

llvm/lib/MC/XCOFFObjectWriter.cpp

421

what about use const auto ?

[jasonliu]

llvm/lib/MC/XCOFFObjectWriter.cpp

421

I'm guessing the reason you mention about "const auto" is that maybe it would help when we have 64 bit.
However, I don't think we made any decision on how to implement 64 bit yet. So I would like to only focus on how this code would work on 32 bit and worry about 64 bit later. So in that case, I don't think we should use auto here, as it would make it very unclear what type we are getting here.

[DiggerLin]

llvm/lib/MC/XCOFFObjectWriter.cpp

462

I have question on the "SymbolA - SymbolB + imm64"

for example: in the assembly of the test case

       .csect .text[PR] 
.foo:
# %bb.0:                                # %entry
        li 4, 0
        cmplwi  3, 3
        stw 3, -8(1)
        stw 4, -12(1)
        bgt     0, LBB0_5
# %bb.1:                                # %entry
        lwz 4, LC0(2)
        slwi 3, 3, 2
        lwzx 3, 3, 4
        add 3, 3, 4
        mtctr 3
        bctr
LBB0_2:                                 # %sw.bb1
        li 3, 1
        b LBB0_6

.......
........
        .csect .rodata[RO]
        .align  2
.LJTI0_0: (symb)
        .long   LBB0_2-.LJTI0_0

the address of LBB0_2 is 44 (0x2C) .

the value in the
00000058 <.rodata>:

58:   ff ff ff d4     .long 0xffffffd4   (-0x2C)
              58: R_POS       .text
              58: R_NEG       .rodata-0x58

the jump address
.text+imm64 (-0x2C) - (0x58) =0x84 + LC0(R2). it not correct

I think the content of in the address of the first bytes .csect .rodata[RO] after link in should be
.text - imm64(-0X2C)-(0X58) = -0X2C + LC0(R2) ?

463

what about use XCOFF::RelocationType type instead of uint8_t ?

[DiggerLin]

llvm/lib/MC/XCOFFObjectWriter.cpp

462

please ignore above question. I got it . the value in the address 0x58 is .text+imm64-.rodata.
So imm64 = value in the address 0x58 + .rodata -.text
when relocation.

[jasonliu]

llvm/lib/MC/XCOFFObjectWriter.cpp

463

At this stage, we are already lowering the high level representation to the actual binary level representation. XCOFFRelocation uses uint8_t to store the XCOFF::RelocationType, so I prefer to matches them here.

[hubert.reinterpretcast]

llvm/lib/MC/XCOFFObjectWriter.cpp

400

On the assembly path, I don't see a label for function descriptor definitions or for common definitions.

401

Typo: s/containting/containing/;

406

If this is not being changed, then—given the size of the function—using const MCSymbol *const SymA could help readability.

[jasonliu]

llvm/lib/MC/XCOFFObjectWriter.cpp

400

We don't have a label for function descriptor definitions or common definitions. So those symbols won't even get here. Could you elaborate your concern?

[jasonliu]

Address comments.

[jasonliu]

llvm/lib/MC/XCOFFObjectWriter.cpp

400

Discussed offline, updated the misleading comments.

[hubert.reinterpretcast]

LGTM with a request for more input.

llvm/test/CodeGen/PowerPC/aix-xcoff-reloc-symb.ll

38 ↗

(On Diff #256666)

I'd like a second opinion over whether this is too high-level.

[jasonliu]

llvm/test/CodeGen/PowerPC/aix-xcoff-reloc-symb.ll

38 ↗

(On Diff #256666)

I see your concern. The better way to write the test is through assembly. As you can easly write assembly like
.csect A[PR]
A:
...
.csect B[RO]
B:
.long A - B
to trigger the relocation emition.
But I doubt the asm parser for AIX works right now.
And I'm not sure if there are other way to get a better test case for it.

[jasonliu]

Updated test case to use mir in order to be as low level as possible.

[jasonliu]

llvm/test/CodeGen/PowerPC/aix-xcoff-reloc-symb.mir

51

I will subsitute index in this line and line 68, so that we don't use hardcoded value.

[hubert.reinterpretcast]

llvm/test/CodeGen/PowerPC/aix-xcoff-reloc-symb.mir

25

I'm a bit disturbed by the weirdness of this function. I suggest the following.

---
name:            foo
alignment:       16
tracksRegLiveness: true
jumpTable:
  kind:            label-difference32
  entries:
    - id:              0
      blocks:          [ '%bb.0' ]
body:             |
  bb.0:
    successors: %bb.0(0x20000000)
    liveins: $r2
    renamable $r3 = LWZtoc %jump-table.0, $r2 :: (load 4 from got)
    BLR implicit $lr, implicit $rm, implicit killed $r3
...

[jasonliu]

Updated test case to a much readable version.

[jasonliu]

llvm/test/CodeGen/PowerPC/aix-xcoff-reloc-symb.mir

25

Thanks. It is much better.

[hubert.reinterpretcast]

LGTM. Thanks.

llvm/lib/MC/XCOFFObjectWriter.cpp

void XCOFFObjectWriter::executePostLayoutBinding(MCAssembler &Asm,
   assignAddressesAndIndices(Layout);
 }
 
 void XCOFFObjectWriter::recordRelocation(MCAssembler &Asm,
                                          const MCAsmLayout &Layout,
                                          const MCFragment *Fragment,
                                          const MCFixup &Fixup, MCValue Target,
                                          uint64_t &FixedValue) {
+  auto getIndex = [this](const MCSymbol *Sym,
+                         const MCSectionXCOFF *ContainingCsect) {
+    // If we could not find the symbol directly in SymbolIndexMap, this symbol
+    // could either be a temporary symbol or an undefined symbol. In this case,
+    // we would need to have the relocation reference its csect instead.
+    return SymbolIndexMap.find(Sym) != SymbolIndexMap.end()
+               ? SymbolIndexMap[Sym]
+               : SymbolIndexMap[ContainingCsect->getQualNameSymbol()];
+  };
 
-  if (Target.getSymB())
-    report_fatal_error("Handling Target.SymB for relocation is unimplemented.");
+  auto getVirtualAddress = [this,
+                            &Layout](const MCSymbol *Sym,
+                                     const MCSectionXCOFF *ContainingCsect) {
+    // If Sym is a csect, return csect's address.
+    // If Sym is a label, return csect's address + label's offset from the csect.
+    return SectionMap[ContainingCsect]->Address +
+           (Sym->isDefined() ? Layout.getSymbolOffset(*Sym) : 0);
+  };
 
-  const MCSymbol &SymA = Target.getSymA()->getSymbol();
+  const MCSymbol *const SymA = &Target.getSymA()->getSymbol();
 
   MCAsmBackend &Backend = Asm.getBackend();
   bool IsPCRel = Backend.getFixupKindInfo(Fixup.getKind()).Flags &
                  MCFixupKindInfo::FKF_IsPCRel;
 
   uint8_t Type;
   uint8_t SignAndSize;
   std::tie(Type, SignAndSize) =
       TargetObjectWriter->getRelocTypeAndSignSize(Target, Fixup, IsPCRel);
 
-  const MCSectionXCOFF *SymASec =
-      getContainingCsect(cast<MCSymbolXCOFF>(&SymA));
+  const MCSectionXCOFF *SymASec = getContainingCsect(cast<MCSymbolXCOFF>(SymA));
   assert(SectionMap.find(SymASec) != SectionMap.end() &&
          "Expected containing csect to exist in map.");
 
-  // If we could not find SymA directly in SymbolIndexMap, this symbol could
-  // either be a temporary symbol or an undefined symbol. In this case, we
-  // would need to have the relocation reference its csect instead.
-  uint32_t Index = SymbolIndexMap.find(&SymA) != SymbolIndexMap.end()
-                       ? SymbolIndexMap[&SymA]
-                       : SymbolIndexMap[SymASec->getQualNameSymbol()];
-
+  const uint32_t Index = getIndex(SymA, SymASec);
   if (Type == XCOFF::RelocationType::R_POS)
     // The FixedValue should be symbol's virtual address in this object file
     // plus any constant value that we might get.
-    // Notice that SymA.isDefined() could return false, but SymASec could still
-    // be a defined csect. One of the example is the TOC-base symbol.
-    FixedValue = SectionMap[SymASec]->Address +
-                 (SymA.isDefined() ? Layout.getSymbolOffset(SymA) : 0) +
-                 Target.getConstant();
+    FixedValue = getVirtualAddress(SymA, SymASec) + Target.getConstant();
   else if (Type == XCOFF::RelocationType::R_TOC)
     // The FixedValue should be the TC entry offset from TOC-base.
     FixedValue = SectionMap[SymASec]->Address - TOCCsects.front().Address;
 
   assert(
       (TargetObjectWriter->is64Bit() ||
        Fixup.getOffset() <= UINT32_MAX - Layout.getFragmentOffset(Fragment)) &&
       "Fragment offset + fixup offset is overflowed in 32-bit mode.");
   uint32_t FixupOffsetInCsect =
       Layout.getFragmentOffset(Fragment) + Fixup.getOffset();
 
   XCOFFRelocation Reloc = {Index, FixupOffsetInCsect, SignAndSize, Type};
   MCSectionXCOFF *RelocationSec = cast<MCSectionXCOFF>(Fragment->getParent());
   assert(SectionMap.find(RelocationSec) != SectionMap.end() &&
          "Expected containing csect to exist in map.");
   SectionMap[RelocationSec]->Relocations.push_back(Reloc);
+
+  if (!Target.getSymB())
+    return;
+
+  const MCSymbol *const SymB = &Target.getSymB()->getSymbol();
+  if (SymA == SymB)
+    report_fatal_error("relocation for opposite term is not yet supported");
+
+  const MCSectionXCOFF *SymBSec = getContainingCsect(cast<MCSymbolXCOFF>(SymB));
+  assert(SectionMap.find(SymBSec) != SectionMap.end() &&
+         "Expected containing csect to exist in map.");
+  if (SymASec == SymBSec)
+    report_fatal_error(
+        "relocation for paired relocatable term is not yet supported");
+
+  assert(Type == XCOFF::RelocationType::R_POS &&
+         "SymA must be R_POS here if it's not opposite term or paired "
+         "relocatable term.");
+  const uint32_t IndexB = getIndex(SymB, SymBSec);
+  // SymB must be R_NEG here, given the general form of Target(MCValue) is
+  // "SymbolA - SymbolB + imm64".
+  const uint8_t TypeB = XCOFF::RelocationType::R_NEG;
+  XCOFFRelocation RelocB = {IndexB, FixupOffsetInCsect, SignAndSize, TypeB};
+  SectionMap[RelocationSec]->Relocations.push_back(RelocB);
+  // We already folded "SymbolA + imm64" above when Type is R_POS for SymbolA,
+  // now we just need to fold "- SymbolB" here.
+  FixedValue -= getVirtualAddress(SymB, SymBSec);
 }
 
 void XCOFFObjectWriter::writeSections(const MCAssembler &Asm,
                                       const MCAsmLayout &Layout) {
   uint32_t CurrentAddressLocation = 0;
   for (const auto *Section : Sections) {
     // Nothing to write for this Section.
     if (Section->Index == Section::UninitializedIndex || Section->IsVirtual)

llvm/test/CodeGen/PowerPC/aix-xcoff-reloc-symb.mir

+# RUN: llc -verify-machineinstrs -mcpu=pwr4 -mtriple powerpc-ibm-aix-xcoff -x mir -verify-machineinstrs -start-after=lazy-machine-block-freq -filetype=obj -o %t.o < %s
+# RUN: llvm-readobj --relocs --expand-relocs -t %t.o | FileCheck --check-prefixes=RELOC,SYM %s
+# RUN: llvm-objdump -D %t.o | FileCheck --check-prefix=DIS %s
+
+---
+name:            foo
+alignment:       16
+tracksRegLiveness: true
+jumpTable:
+  kind:            label-difference32
+  entries:
+    - id:              0
+      blocks:          [ '%bb.0' ]
+body:             |
+  bb.0:
+    successors: %bb.0(0x20000000)
+    liveins: $r2
+    renamable $r3 = LWZtoc %jump-table.0, $r2 :: (load 4 from got)
+    BLR implicit $lr, implicit $rm, implicit killed $r3
+...
+
+# RELOC:       Relocation {{[{][[:space:]] *}}Virtual Address: 0x8
+# RELOC-NEXT:    Symbol: .text ([[#TXT_INDX:]])
+# RELOC-NEXT:    IsSigned: No
+# RELOC-NEXT:    FixupBitValue: 0
+# RELOC-NEXT:    Length: 32
+# RELOC-NEXT:    Type: R_POS (0x0)
+# RELOC-NEXT:  }
+# RELOC-NEXT:  Relocation {
+# RELOC-NEXT:    Virtual Address: 0x8
+# RELOC-NEXT:    Symbol: .rodata ([[#RO_INDX:]])
+# RELOC-NEXT:    IsSigned: No
+# RELOC-NEXT:    FixupBitValue: 0
+# RELOC-NEXT:    Length: 32
+# RELOC-NEXT:    Type: R_NEG (0x1)
+# RELOC-NEXT:  }
+
+# SYM:       Symbol {{[{][[:space:]] *}}Index: [[#TXT_INDX]]{{[[:space:]] *}}Name: .text
+# SYM-NEXT:    Value (RelocatableAddress): 0x0
+# SYM-NEXT:    Section: .text
+# SYM-NEXT:    Type: 0x0
+# SYM-NEXT:    StorageClass: C_HIDEXT (0x6B)
+# SYM-NEXT:    NumberOfAuxEntries: 1
+# SYM-NEXT:    CSECT Auxiliary Entry {
+# SYM-NEXT:      Index: [[#TXT_INDX+1]]
+# SYM-NEXT:      SectionLen: 8
+# SYM-NEXT:      ParameterHashIndex: 0x0
+# SYM-NEXT:      TypeChkSectNum: 0x0
+# SYM-NEXT:      SymbolAlignmentLog2: 4
+# SYM-NEXT:      SymbolType: XTY_SD (0x1)
+# SYM-NEXT:      StorageMappingClass: XMC_PR (0x0)
+# SYM-NEXT:      StabInfoIndex: 0x0
+# SYM-NEXT:      StabSectNum: 0x0
+# SYM-NEXT:    }
+# SYM-NEXT:  }
+# SYM:       Symbol {{[{][[:space:]] *}}Index: [[#RO_INDX]]{{[[:space:]] *}}Name: .rodata
+# SYM-NEXT:    Value (RelocatableAddress): 0x8
+# SYM-NEXT:    Section: .text
+# SYM-NEXT:    Type: 0x0
+# SYM-NEXT:    StorageClass: C_HIDEXT (0x6B)
+# SYM-NEXT:    NumberOfAuxEntries: 1
+# SYM-NEXT:    CSECT Auxiliary Entry {
+# SYM-NEXT:      Index: [[#RO_INDX+1]]
+# SYM-NEXT:      SectionLen: 4
+# SYM-NEXT:      ParameterHashIndex: 0x0
+# SYM-NEXT:      TypeChkSectNum: 0x0
+# SYM-NEXT:      SymbolAlignmentLog2: 2
+# SYM-NEXT:      SymbolType: XTY_SD (0x1)
+# SYM-NEXT:      StorageMappingClass: XMC_RO (0x1)
+# SYM-NEXT:      StabInfoIndex: 0x0
+# SYM-NEXT:      StabSectNum: 0x0
+# SYM-NEXT:    }
+# SYM-NEXT:  }
+
+# DIS:      Disassembly of section .text:
+# DIS-EMPTY:
+# DIS-NEXT: 00000000 <.text>:
+# DIS-NEXT:        0: 80 62 00 00                   lwz 3, 0(2)
+# DIS-NEXT:        4: 4e 80 00 20                   blr
+# DIS-EMPTY:
+# DIS-NEXT: 00000008 <.rodata>:
+# DIS-NEXT:        8: ff ff ff f8                   fmsub 31, 31, 31, 31
+# DIS-EMPTY: