[XCOFF][AIX] Handle TOC entries that could not be reached by positive range in small code model

[jasonliu]

Summary

In small code model, AIX assembler could not deal with labels that could not be reached within the [-0x8000, 0x8000) range from TOC base.
So when generating the assembly, we would need to help the assembler by subtracting an offset from the label to keep the actual value within [-0x8000, 0x8000).

[Xiangling_L]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

603

Can we add an assertion here to make sure EntryDistanceFromTOCBase - PositiveTOCRange do not go beyond -0x8000?

llvm/test/CodeGen/PowerPC/aix-overflow-toc.test

39 ↗

(On Diff #288953)

Since toc entry is 8 bytes under 64bit mode, does that mean we can only have no more than 8192 entries?

[jasonliu]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

603

EntryDistanceFromTOCBase - PositiveTOCRange is always going to be a positive number. So not sure why we would need an assertion for it to be >= 0x8000?

llvm/test/CodeGen/PowerPC/aix-overflow-toc.test

39 ↗

(On Diff #288953)

The compiler and assembler could still generate more than 8192 entries. But if you need to link it, you would need -bbigtoc for the link to be successful. Basically, if there are more than 8192 entries, we would need to rely on linker to generate branch out code to make everything work.

[jasonliu]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

603

Sorry, I mean >= -0x8000

[Xiangling_L]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

588

The assertion here seems a bit unnecessary, since lookUpOrCreateTOCEntry will always return us a TOCEntry.

593

s/need/needs

597

s/enties/entries;
s/could represented/could be represented or you can remove "could".

603

Sorry for the confusion, I meant Expr -TOCRange * Multiply. And I think your reply below has clarified that there is no such assertion needed.

llvm/test/CodeGen/PowerPC/aix-overflow-toc.test

33 ↗

(On Diff #288953)

nit: trailing whitespace.

[jasonliu]

Address comments.

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

588

I put the assertion here as a defensive mechanism in case the code get move around in the future and someone calls this function before lookUpOrCreateTOCEntry.

[hubert.reinterpretcast]

llvm/test/CodeGen/PowerPC/aix-overflow-toc.test

1 ↗

(On Diff #288953)

Noting that most tests that use the test file itself as a Python script are named with a py file extension (e.g., llvm/test/MC/COFF/bigobj.py).

[jasonliu]

llvm/test/CodeGen/PowerPC/aix-overflow-toc.test

1 ↗

(On Diff #288953)

Yeah... I noticed that.
But for some reason, when I initially use .py as file extension, it results in lit not running that test when running a folder of tests (It's fine if I only run that test) . Could be that I need to tweak some lit config file to make it not skip over? But I didn't get to the bottom of it, and used '.test' as workaround.

[hubert.reinterpretcast]

llvm/test/CodeGen/PowerPC/aix-overflow-toc.test

1 ↗

(On Diff #288953)

Something like llvm/test/CodeGen/SystemZ/Large/lit.local.cfg would could be placed into llvm/test/CodeGen/PowerPC/. Indeed, llvm/test/MC/COFF/bigobj.py doesn't seem to run.

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

590

Is this an int or is this more a ptrdiff_t?

592

This could be constexpr?

595

Since the TOC entries are multiples of 4 or 8, both the < and <= results are the same; however, the <= version matches the semantics better.

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

598–607

I'd like to see a "cleanly derived" version of this for general offsets from the TOC-base virtual address. Also, this part should be pulled into another lambda (taking an input expression and associated offset) that generally handles adjustment of TOC relocation expressions.

For example, the result should be 0 when using an offset value from 0 to 32768 (inclusive) and an offset of 98303 should produce 1 instead of 2.

Part of my rationale is that it is a "code smell" that the current calculation does not work in general. Another factor is that TOC-related accesses at non-zero offsets from the beginning of the entry are meaningful. The TD storage mapping class is used to store data within the TOC (as opposed to storing a pointer to the data in the TOC). We should be able to reuse the calculation for the general-offset case when we need it.

It's probably easier to start with the comment:
We're not just keeping the offset to be within some range. We are causing the modified notional offset from the TOC base (to be encoded into the instruction's D or DS field) to be the signed 16-bit truncation of the original notional offset from the TOC base. This is consistent with the treatment used both by XL C/C++ and by AIX ld -r.

Solving for the adjustment value to subtract:
(int16_t)original_offset = original_offset - adjustment
(int16_t)original_offset + adjustment = original_offset
adjustment = original_offset - (int16_t)original_offset

But we're dealing with C++14 code, so (with the hopes that SignExtend32 is sufficiently well written):
adjustment = original_offset - llvm::SignExtend32<16>(original_offset)

[jasonliu]

Address review comments.

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

598–607

Thanks. Please let me know if my new revision does not address your concern.

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

585–589

Adjust the comment to fit the context.

595

How would this relate to XCOFFObjectWriter::recordRelocation?

I notice that there's a line there:

FixedValue = SectionMap[SymASec]->Address - TOCCsects.front().Address;

Does the new expression formed here mean that we should account for it through Target.getConstant()?

609–614

Since the adjustment is now factored out, the comment doesn't quite fit. Indeed, the code would read fine without a comment if the condition was reversed to make it obvious when special handling is needed.

[hubert.reinterpretcast]

llvm/test/CodeGen/PowerPC/aix-overflow-toc.py

4

Indentation does not match with line 5.

[jasonliu]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

595

From the testing I have done, there seems to be no particular action need to be taken there (except removing that report_fatal_error in there), as the value we put into the instruction could already wrap around naturally when it goes out of the range.

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

595

I think we should add the Target.getConstant() because it could show up for using the TD storage mapping class or maybe for integrated assembly. The report_fatal_error enforces a constraint that the no wraparound is necessary at that stage. As in, we may want the object-writing path to require input that would also work on the assembly-writing path.

[jasonliu]

Address comments.

[hubert.reinterpretcast]

LGTM with minor comments.

llvm/lib/MC/XCOFFObjectWriter.cpp

433

Update the comment to match the code change.

llvm/test/CodeGen/PowerPC/aix-overflow-toc.py

64

Minor nit: Indentation.

[fhahn]

It appears this patch added a test that is failing with expensive checks on GreenDragon: http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive/17604/console. It would be great if you could take a look and revert if it takes longer to fix.

[hubert.reinterpretcast]

In D86879#2273354, @fhahn wrote:

It appears this patch added a test that is failing with expensive checks on GreenDragon: http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive/17604/console. It would be great if you could take a look and revert if it takes longer to fix.

It says:

Timeout: Reached timeout of 600 seconds

We believe that the test is necessarily large. Has the associated bot encountered similar issues before? What was the recommended fix to retain the test in more appropriate environments?

[nemanjai]

In D86879#2274025, @hubert.reinterpretcast wrote:

We believe that the test is necessarily large. Has the associated bot encountered similar issues before? What was the recommended fix to retain the test in more appropriate environments?

Perhaps the test case should be restricted to some key platforms where cross-compilation to AIX is relevant? Perhaps Linux (PPC is already required).

[jasonliu]

In D86879#2274076, @nemanjai wrote:

In D86879#2274025, @hubert.reinterpretcast wrote:

We believe that the test is necessarily large. Has the associated bot encountered similar issues before? What was the recommended fix to retain the test in more appropriate environments?

Perhaps the test case should be restricted to some key platforms where cross-compilation to AIX is relevant? Perhaps Linux (PPC is already required).

I could add
# REQUIRES: aix || linux
to the test if that helps.

[fhahn]

In D86879#2274099, @jasonliu wrote:

In D86879#2274076, @nemanjai wrote:

In D86879#2274025, @hubert.reinterpretcast wrote:

We believe that the test is necessarily large. Has the associated bot encountered similar issues before? What was the recommended fix to retain the test in more appropriate environments?

Perhaps the test case should be restricted to some key platforms where cross-compilation to AIX is relevant? Perhaps Linux (PPC is already required).

I could add
# REQUIRES: aix || linux
to the test if that helps.

The problem here is with expensive checks, which makes it take too long I think. It works fine on Darwin bots without expensive checks. If it is not possible to reduce the size of the test, then we need to disable it at least for the configuration used for http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive. I am not sure if there's another way to force a big gap in PPC assembly.

llvm/lib/MC/XCOFFObjectWriter.cpp

 //
 // We don't represent the sections in the MC layer as there is nothing
 // interesting about them at at that level: they carry information that is
 // only relevant to the ObjectWriter, so we materialize them in this class.
 namespace {
 
 constexpr unsigned DefaultSectionAlign = 4;
 constexpr int16_t MaxSectionIndex = INT16_MAX;
-constexpr uint16_t MaxTOCSizeInARegion = UINT16_MAX;
 
 // Packs the csect's alignment and type into a byte.
 uint8_t getEncodedType(const MCSectionXCOFF *);
 
 struct XCOFFRelocation {
   uint32_t SymbolTableIndex;
   uint32_t FixupOffsetInCsect;
   uint8_t SignAndSize;
void XCOFFObjectWriter::recordRelocation(MCAssembler &Asm,
 
   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.
     FixedValue = getVirtualAddress(SymA, SymASec) + Target.getConstant();
   else if (Type == XCOFF::RelocationType::R_TOC ||
            Type == XCOFF::RelocationType::R_TOCL) {
     // The FixedValue should be the TC entry offset from TOC-base.
-    FixedValue = SectionMap[SymASec]->Address - TOCCsects.front().Address;
-    if (FixedValue >= MaxTOCSizeInARegion)
-      report_fatal_error(
-          "handling of TOC entries could not fit in the initial TOC "
-          "entry region is not yet supported");
+    const int64_t TOCEntryOffset = SectionMap[SymASec]->Address -
+                                   TOCCsects.front().Address +
+                                   Target.getConstant();
+    if (Type == XCOFF::RelocationType::R_TOC && !isInt<16>(TOCEntryOffset))
+      report_fatal_error("TOCEntryOffset overflows in small code model mode");
+
+    FixedValue = TOCEntryOffset;
   }
 
   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();

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

for (const MachineOperand &MO: MI->operands()) {
         } else {
           if (PPC::SPERCRegClass.contains(Reg))
             llvm_unreachable("SPE register found in FPU-targeted code!");
         }
       }
     }
   }
 #endif
+
+  auto getTOCRelocAdjustedExprForXCOFF = [this](const MCExpr *Expr,
+                                                ptrdiff_t OriginalOffset) {
+    // Apply an offset to the TOC-based expression such that the adjusted
+    // notional offset from the TOC base (to be encoded into the instruction's D
+    // or DS field) is the signed 16-bit truncation of the original notional
+    // offset from the TOC base.
+    // This is consistent with the treatment used both by XL C/C++ and
+    // by AIX ld -r.
+    ptrdiff_t Adjustment =
+        OriginalOffset - llvm::SignExtend32<16>(OriginalOffset);
+    return MCBinaryExpr::createAdd(
+        Expr, MCConstantExpr::create(-Adjustment, OutContext), OutContext);
+  };
+
+  auto getTOCEntryLoadingExprForXCOFF =
+      [IsPPC64, getTOCRelocAdjustedExprForXCOFF,
+       this](const MCSymbol *MOSymbol, const MCExpr *Expr) -> const MCExpr * {
+    const unsigned EntryByteSize = IsPPC64 ? 8 : 4;
+    const auto TOCEntryIter = TOC.find(MOSymbol);
+    assert(TOCEntryIter != TOC.end() &&
+           "Could not find the TOC entry for this symbol.");
+    const ptrdiff_t EntryDistanceFromTOCBase =
+        (TOCEntryIter - TOC.begin()) * EntryByteSize;
+    constexpr int16_t PositiveTOCRange = INT16_MAX;
+
+    if (EntryDistanceFromTOCBase > PositiveTOCRange)
+      return getTOCRelocAdjustedExprForXCOFF(Expr, EntryDistanceFromTOCBase);
+
+    return Expr;
+  };
+
   // Lower multi-instruction pseudo operations.
   switch (MI->getOpcode()) {
   default: break;
   case TargetOpcode::DBG_VALUE:
     llvm_unreachable("Should be handled target independently");
   case TargetOpcode::STACKMAP:
     return LowerSTACKMAP(SM, *MI);
   case TargetOpcode::PATCHPOINT:
     return LowerPATCHPOINT(SM, *MI);
case PPC::LWZtoc: {
 
     // AIX uses the label directly as the lwz displacement operand for
     // references into the toc section. The displacement value will be generated
     // relative to the toc-base.
     if (IsAIX) {
       assert(
           TM.getCodeModel() == CodeModel::Small &&
           "This pseudo should only be selected for 32-bit small code model.");
+      Exp = getTOCEntryLoadingExprForXCOFF(MOSymbol, Exp);
       TmpInst.getOperand(1) = MCOperand::createExpr(Exp);
       EmitToStreamer(*OutStreamer, TmpInst);
       return;
     }
 
     // Create an explicit subtract expression between the local symbol and
     // '.LTOC' to manifest the toc-relative offset.
     const MCExpr *PB = MCSymbolRefExpr::create(
case PPC::LDtoc: {
 
     // Change the opcode to LD.
     TmpInst.setOpcode(PPC::LD);
 
     const MachineOperand &MO = MI->getOperand(1);
     assert((MO.isGlobal() || MO.isCPI() || MO.isJTI() || MO.isBlockAddress()) &&
            "Invalid operand!");
 
+    // Map the operand to its corresponding MCSymbol.
+    const MCSymbol *const MOSymbol = getMCSymbolForTOCPseudoMO(MO, *this);
+
     // Map the machine operand to its corresponding MCSymbol, then map the
     // global address operand to be a reference to the TOC entry we will
     // synthesize later.
-    MCSymbol *TOCEntry =
-        lookUpOrCreateTOCEntry(getMCSymbolForTOCPseudoMO(MO, *this));
+    MCSymbol *TOCEntry = lookUpOrCreateTOCEntry(MOSymbol);
 
     const MCSymbolRefExpr::VariantKind VK =
         IsAIX ? MCSymbolRefExpr::VK_None : MCSymbolRefExpr::VK_PPC_TOC;
     const MCExpr *Exp =
         MCSymbolRefExpr::create(TOCEntry, VK, OutContext);
-    TmpInst.getOperand(1) = MCOperand::createExpr(Exp);
+    TmpInst.getOperand(1) = MCOperand::createExpr(
+        IsAIX ? getTOCEntryLoadingExprForXCOFF(MOSymbol, Exp) : Exp);
     EmitToStreamer(*OutStreamer, TmpInst);
     return;
   }
   case PPC::ADDIStocHA: {
     assert((IsAIX && !IsPPC64 && TM.getCodeModel() == CodeModel::Large) &&
            "This pseudo should only be selected for 32-bit large code model on"
            " AIX.");
 
if (M.empty())
     return;
 
   // Switch to section to emit TOC base.
   OutStreamer->SwitchSection(getObjFileLowering().getTOCBaseSection());
 
   PPCTargetStreamer *TS =
       static_cast<PPCTargetStreamer *>(OutStreamer->getTargetStreamer());
 
-  const unsigned EntryByteSize = Subtarget->isPPC64() ? 8 : 4;
-  const unsigned TOCEntriesByteSize = TOC.size() * EntryByteSize;
-  // TODO: If TOC entries' size is larger than 32768, then we run out of
-  // positive displacement to reach the TOC entry. We need to decide how to
-  // handle entries' size larger than that later.
-  if (TOCEntriesByteSize > 32767) {
-    report_fatal_error("Handling of TOC entry displacement larger than 32767 "
-                       "is not yet implemented.");
-  }
-
   for (auto &I : TOC) {
     // Setup the csect for the current TC entry.
     MCSectionXCOFF *TCEntry = cast<MCSectionXCOFF>(
         getObjFileLowering().getSectionForTOCEntry(I.first, TM));
     OutStreamer->SwitchSection(TCEntry);
 
     OutStreamer->emitLabel(I.second);
     if (TS != nullptr)

llvm/test/CodeGen/PowerPC/aix-overflow-toc.py

+# RUN: python %s > %t.ll
+# RUN: llc -mtriple powerpc-ibm-aix-xcoff -code-model=small -mcpu=pwr4 -mattr=-altivec -O0 < %t.ll | \
+# RUN:   FileCheck --check-prefix=ASM32 %s
+
+# RUN: llc -mtriple powerpc64-ibm-aix-xcoff -code-model=small -mcpu=pwr4 -mattr=-altivec -O0 < %t.ll | \
+# RUN:   FileCheck --check-prefix=ASM64 %s
+
+# RUN: llc -mtriple powerpc-ibm-aix-xcoff -code-model=small -mcpu=pwr4 -mattr=-altivec -O0 \
+# RUN:     -filetype=obj -o %t.o < %t.ll
+# RUN: llvm-objdump -D -r --symbol-description %t.o | FileCheck --check-prefix=DIS32 %s
+
+# RUN: not --crash llc -mtriple powerpc64-ibm-aix-xcoff \
+# RUN:     -mcpu=pwr4 -mattr=-altivec -filetype=obj -o %t.o 2>&1 < %t.ll | \
+# RUN:   FileCheck --check-prefix=XCOFF64 %s
+# XCOFF64: LLVM ERROR: 64-bit XCOFF object files are not supported yet.
+
+numentries = 12290
+for x in range(0, numentries):
+    print("@a%d = global i32 0, align 4" % (x))
+
+print("define void @foo() {")
+print("entry:")
+for x in range(0, numentries):
+    print("store i32 1, i32* @a%d, align 4" % (x))
+print("ret void")
+print("}")
+
+# 32-bit assembly check
+# ASM32:  lwz 3, L..C0(2)
+# ASM32:  lwz 3, L..C1(2)
+
+# ASM32:  lwz 3, L..C8191(2)
+# ASM32:  lwz 3, L..C8192-65536(2)
+# ASM32:  lwz 3, L..C8193-65536(2)
+
+# ASM32:  lwz 3, L..C12288-65536(2)
+# ASM32:  lwz 3, L..C12289-65536(2)
+
+# 64-bit assembly check
+# ASM64:  ld 3, L..C0(2)
+# ASM64:  ld 3, L..C1(2)
+
+# ASM64:  ld 3, L..C4095(2)
+# ASM64:  ld 3, L..C4096-65536(2)
+# ASM64:  ld 3, L..C4097-65536(2)
+
+# ASM64:  ld 3, L..C12287-65536(2)
+# ASM64:  ld 3, L..C12288-131072(2)
+# ASM64:  ld 3, L..C12289-131072(2)
+
+# DIS32:   0: 80 62 00 00   lwz 3, 0(2)
+# DIS32:  00000002:  R_TOC  (idx: 24590) a0[TC]
+# DIS32:   c: 80 62 00 04   lwz 3, 4(2)
+# DIS32:  0000000e:  R_TOC  (idx: 24592) a1[TC]
+
+# DIS32:    fffc: 80 62 7f fc   lwz 3, 32764(2)
+# DIS32:      0000fffe:  R_TOC  (idx: 40972) a8191[TC]
+# DIS32:   10004: 80 62 80 00   lwz 3, -32768(2)
+# DIS32:      00010006:  R_TOC  (idx: 40974) a8192[TC]
+# DIS32:   1000c: 80 62 80 04   lwz 3, -32764(2)
+# DIS32:      0001000e:  R_TOC  (idx: 40976) a8193[TC]
+
+# DIS32:   18004: 80 62 c0 00   lwz 3, -16384(2)
+# DIS32:       00018006:  R_TOC  (idx: 49166) a12288[TC]
+# DIS32:   1800c: 80 62 c0 04   lwz 3, -16380(2)
+# DIS32:      0001800e:  R_TOC  (idx: 49168) a12289[TC]

llvm/test/CodeGen/PowerPC/lit.local.cfg

 if not 'PowerPC' in config.root.targets:
     config.unsupported = True
+
+config.suffixes.add('.py')