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

[Jason Liu via Phabricator via llvm-commits]

jasonliu added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:598-607
+    // AIX system assembler could not directly deal with TOC entries that are
+    // not within the range that could be represented by an int16_t type in
+    // small code model mode. But it works happily if we keep the offset of
+    // those TOC entries within that range.
+    const int TOCRange = 0x10000;
+    const int Multiply =
+        (EntryDistanceFromTOCBase - PositiveTOCRange) / TOCRange + 1;
----------------
hubert.reinterpretcast wrote:
> 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)
Thanks. Please let me know if my new revision does not address your concern. 


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

https://reviews.llvm.org/D86879