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

[Xiangling Liao via Phabricator via llvm-commits]

Xiangling_L added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:588
+    const auto TOCEntryIter = TOC.find(MOSymbol);
+    assert(TOCEntryIter != TOC.end() &&
+           "Could not find the TOC entry for this symbol.");
----------------
The assertion here seems a bit unnecessary, since `lookUpOrCreateTOCEntry` will always return us a TOCEntry.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:593
+    const int16_t PositiveTOCRange = INT16_MAX;
+    // Nothing need to be done when TOC entries are still within positive range.
+    if (EntryDistanceFromTOCBase < PositiveTOCRange)
----------------
s/need/needs


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:597
+
+    // AIX system assembler could not directly deal with TOC enties that are not
+    // within the range that could represented by an int16_t type in small code
----------------
s/enties/entries;
s/could represented/could be represented or you can remove "could".


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:603
+    const int Multiply =
+        (EntryDistanceFromTOCBase - PositiveTOCRange) / TOCRange + 1;
+    return MCBinaryExpr::createAdd(
----------------
jasonliu wrote:
> jasonliu wrote:
> > Xiangling_L wrote:
> > > Can we add an assertion here to make sure `EntryDistanceFromTOCBase - PositiveTOCRange` do not go beyond `-0x8000`?
> > `EntryDistanceFromTOCBase - PositiveTOCRange` is always going to be a positive number. So not sure why we would need an assertion for it to be >= 0x8000?
> Sorry, I mean >= -0x8000
Sorry for the confusion, I meant `Expr -TOCRange * Multiply`. And I think your reply below has clarified that there is no such assertion needed.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-overflow-toc.test:33
+
+# ASM64:  ld 3, L..C4095(2) 
+# ASM64:  ld 3, L..C4096-65536(2)
----------------
nit: trailing whitespace.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86879/new/

https://reviews.llvm.org/D86879