[PATCH] D70126: [PowerPC] Refactor FinishCall

Xiangling Liao via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 26 13:58:54 PST 2019


Xiangling_L added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:5068
+    return callsShareTOCBase(&Caller, Callee, TM) ? PPCISD::CALL
+                                                 : PPCISD::CALL_NOP;
+
----------------
Please fix the formatting issue here.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:5145
+static void prepareDescriptorIndirectCall(SelectionDAG &DAG, SDValue &Callee,
+                                        SDValue &Glue, SDValue &Chain,
+                                        SDValue CallSeqStart,
----------------
Please fix the formatting issue from line 5145 - line 5149.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:5249
+      SDValue AddTOC = DAG.getNode(ISD::ADD, dl, MVT::i64, StackPtr, TOCOff);
+      Ops.push_back(AddTOC);
+    }
----------------
I feel it's worth mentioning  in the comment `The address needs to go after the chain input but before the flag (or any other variadic arguments).` for `AddTOC` like the original comment does, in case ppl mistakenly move this around in the future.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:5277
+    Ops.push_back(DAG.getRegister(Subtarget.isPPC64() ? PPC::X2 : PPC::R2,
+                                  Subtarget.isPPC64() ? MVT::i64 : MVT::i32));
 
----------------
Since there are a few spots using `Subtarget.isPPC64()` and `Subtarget.isPPC64() ? MVT::i64 : MVT::i32)`, can we create a variable for each to shorten the code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70126





More information about the llvm-commits mailing list