[PATCH] D79864: [PowerPC] Add linker opt for PC Relative GOT indirect accesses

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp marked 3 inline comments as done.
stefanp added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp:228
+
+  // Obtian the label symbol and if the instruction is PLDpc return immediately.
+  // The PLDpc is the first of the two instrctions that are linked together by
----------------
anil9 wrote:
> nit : Obtian -> Obtaini
I'm not sure what you want here... Did I have typo it before and have `Obaini` there?


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp:246
+  assert(DF && "Expecting a valid data fragment.");
+  DF->getFixups().push_back(MCFixup::create(LabelSym->getOffset()-8,
+                            SubExpr2,
----------------
anil9 wrote:
> Can this -8 be handled in some other way, like defined above as something. Looks a bit like a magic number.
The 8 is the size of a prefixed instruction in bytes. I don't want to define it somewhere because it is used only in a couple of places but I will add a comment to explain it. This whole section deserves a better comment.


================
Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:238
+        for (auto SearchBBI = ++BBI; SearchBBI != MBB.instr_end();
+             ++SearchBBI) {
+          if (SearchBBI->readsRegister(Reg, TRI) &&
----------------
anil9 wrote:
> This did not fit in one line ?
no... off by one character. :(


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

https://reviews.llvm.org/D79864