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

Hubert Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 10 12:46:27 PDT 2020


hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:585-589
+    // 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.
----------------
Adjust the comment to fit the context.


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:595
+    return MCBinaryExpr::createAdd(
+        Expr, MCConstantExpr::create(-Adjustment, OutContext), OutContext);
+  };
----------------
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()`?


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:609-614
+    // Nothing needs to be done when TOC entries are still within positive
+    // range.
+    if (EntryDistanceFromTOCBase <= PositiveTOCRange)
+      return Expr;
+
+    return getTOCRelocAdjustedExprForXCOFF(Expr, EntryDistanceFromTOCBase);
----------------
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.


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

https://reviews.llvm.org/D86879



More information about the llvm-commits mailing list