[PATCH] First stage of call lowering for Mips fast-isel

[Daniel Sanders]

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:770
@@ +769,3 @@
+
+  emitInst(Mips::ADJCALLSTACKDOWN).addImm(16);
+  // Process the args.
----------------
rkotler wrote:
> rkotler wrote:
> > dsanders wrote:
> > > The 16-bytes reserved space is allocated by the prologue of the callee not the caller.
> > > 
> > > However, I discovered something strange in the SelectionDAG implementation this morning. MipsTargetLowering::LowerCall() calls CCInfo.AllocateStack() to allocate the reserved argument area even though LowerCall is lowering from the callers point of view. Removing the call breaks the frame size calculation so it's clearly important but I haven't figured out why. I would have expected the one in MipsTargetLowering::LowerFormalArguments() to be the important one.
> > For O32, I think it's the caller.
> > From the ABI Doc:
> > 
> > https://dmz-portal.mips.com/mw/images/f/fe/MD00305-2B-ABIDESC-SPC-01.03.pdf
> > 
> > 7. The caller will always build an argument data
> > structure, even though it may remain unused in whole
> > or part. Moreover, the data structure is always a
> > minimum of 16 bytes (four register-sized slots) in
> > size.
> > 
> > 
> BTW, with this patch, I pass all of test-suite at O0/O2 for mips32r1 and mips32r2.
Yes you're right, I had double-checked with a misleading testcase. This makes me suspicious of the code in LowerFormalArguments() instead since it also allocates the reserved region of the stack despite operating from the callee side.

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:828-829
@@ +827,4 @@
+    }
+    case CCValAssign::AExt:
+    // Intentional fall-through.
+    case CCValAssign::ZExt: {
----------------
rkotler wrote:
> dsanders wrote:
> > It makes no difference for 32-bit but AExt is normally treated the same way as SExt by our backend.
> What do you want here?
I think that in the absence of a good reason to do otherwise, we should be consistent with the other parts of our backend and generate sign-extends for AExt. It's not required though.

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:843-849
@@ +842,9 @@
+    // Now copy/store arg to correct locations.
+    if (VA.isRegLoc() && !VA.needsCustom()) {
+      BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
+              TII.get(TargetOpcode::COPY), VA.getLocReg()).addReg(ArgReg);
+      CLI.OutRegs.push_back(VA.getLocReg());
+    } else if (VA.needsCustom()) {
+      // FIXME: Handle custom args.
+      return false;
+    } else {
----------------
rkotler wrote:
> dsanders wrote:
> > We don't use custom arguments. We ought to delete the references to VA.needsCustom()
> It seems we should then make this llvm_unreachable then. If someone later changes the compiler and we have custom args, we want to fail here because that is our assumption.
That sounds good to me.

http://reviews.llvm.org/D5714