[PATCH] D16315: [ppc64] Enable sibling call optimization on ppc64 ELFv1/ELFv2 abi

[Hal Finkel via llvm-commits]

hfinkel added inline comments.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:3848
@@ +3847,3 @@
+static bool
+ResideInSameModule(SDValue Callee, Reloc::Model RelMod) {
+  // If !G, Callee can be an external symbol.
----------------
Function names should start with a lower-case letter.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:3888
@@ +3887,3 @@
+static bool
+NeedStackSlotPassParameters(const PPCSubtarget &Subtarget,
+                            const SmallVectorImpl<ISD::OutputArg> &Outs) {
----------------
Function names should start with a lower-case letter.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:3919
@@ +3918,3 @@
+                               NumBytes, AvailableFPRs, AvailableVRs,
+			       Subtarget.hasQPX()))
+      return true;
----------------
Indent more to line up with other args.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:3953
@@ +3952,3 @@
+  // Functions containing by val parameters are not supported.
+  for (const ISD::InputArg& IA : Ins)
+    if (IA.Flags.isByVal())
----------------
Use std::any_of (with a small lambda).

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:3976
@@ +3975,3 @@
+  if (NeedStackSlotPassParameters(Subtarget, Outs)) {
+    // TODO: Allow SCO if caller and callee has same function prototype
+    return false;
----------------
Are you planning on doing this in follow-up? It seems simple enough to check. Unless there's some complication here, I'd rather you do this up-front so that we get more testing coverage on all of the rest of the logic.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:4007
@@ +4006,3 @@
+    RegisterSDNode *RSD = cast<RegisterSDNode>(OutVals[0]->getOperand(1));
+    unsigned calleeArg0Vreg = RSD->getReg();
+
----------------
Variables start with a capital letter.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:4010
@@ +4009,3 @@
+    // Look at caller's 1st argument (struct-return pointer), get it's vreg id.
+    unsigned callerArg0Vreg = MF.getRegInfo().livein_begin()->second;
+
----------------
Variables start with a capital letter.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:4010
@@ +4009,3 @@
+    // Look at caller's 1st argument (struct-return pointer), get it's vreg id.
+    unsigned callerArg0Vreg = MF.getRegInfo().livein_begin()->second;
+
----------------
hfinkel wrote:
> Variables start with a capital letter.
I don't think that the first live-in register in the list is guaranteed to be the first argument's register. Also, you say "vreg id"; are these virtual registers or physical registers at this point?


http://reviews.llvm.org/D16315