[PATCH] D76160: [PowerPC][Future] Add offsets to PC Relative relocations.

[Anil Mahmud via Phabricator via llvm-commits]

anil9 added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp:198
+  // If this is not a MCExpr then we are in case 3) and we are dealing with
+  // a value known at compile time not a relocation.
   const MCOperand &MO = MI.getOperand(OpNo);
----------------
s/compile time not/ compile time, not/


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp:210
+  case MCExpr::SymbolRef: {
+    // Case 1). Relocation alone.
     const MCSymbolRefExpr *SRE = cast<MCSymbolRefExpr>(Expr);
----------------
Generally the comments are above the code, here it seems the comments like Case1) are after the cases.


================
Comment at: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMCCodeEmitter.cpp:253
+    return (CE->getValue() & 0x3FFFFFFFFUL) | RegBits;
+  }
+  }
----------------
Indentation.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:15679
+  int64_t NewOffset = GSDN->getOffset() + ConstNode->getSExtValue();
+  SDLoc DL(GSDN);
+
----------------
Does choosing to initialize DL depend on whether you swapped the operands earlier or not in line 15666 ?


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

https://reviews.llvm.org/D76160