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

Amy Kwan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 10 14:28:50 PDT 2020


amyk added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp:217
+  unsigned LastOp = Inst.getNumOperands()-1;
+  // The VK_PPC_LINKER_OPT flag should part of and Expr on the last operand.
+  const MCOperand &Operand = Inst.getOperand(LastOp);
----------------
Could you rephrase this sentence? Not sure if it's just me but it seems a little confusing to read.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp:229
+  // 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
+  // this linker opt and we must wait for the second instruction before the
----------------
s/instrctions/instructions


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCELFStreamer.cpp:236
+
+  // For all other instrctions we can create the fixup.
+  const MCExpr *LabelExpr = MCSymbolRefExpr::create(LabelSym, getContext());
----------------
s/instrctions/instructions


================
Comment at: llvm/lib/Target/PowerPC/PPCPreEmitPeephole.cpp:252
+        }
+        if (UseFound.isValid()) {
+          // The first operand of the use is going to be either the element we
----------------
Could we do something like
```
if (!UseFound.isValid())
  continue;
```
maybe?


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

https://reviews.llvm.org/D79864





More information about the llvm-commits mailing list