[mips][FastISel] Handle functions which return i1, i8 or i16.

[vkalintiris]

Summary

Allow MIPS FastISel to handle functions which return i1/i8/i16 signed/unsigned.

[dsanders]

Testcase?

lib/Target/Mips/MipsFastISel.cpp

1103–1104

Type promotion should be handled by RetCC_Mips where it can be used by both Fast ISel and SelectionDAG. I suggest adding 'CCIfType<[i1, i8, i16], CCPromoteToType<i32>>' to RetCC_MipsO32.

I'll explain how to run the ABI tester to verify the change off-list.

1404–1405

This ought to be a test for integer types <= 32bits. For example, it's entirely possible to support i2 sign-extension to i11, or i25 zero-extension to i29. However, this will require emitIntZExt() and emitIntSExt*() to be updated to use shl/srl and shl/sra pairs, and in the case of emitIntZExt() also calculate the andi mask where appropriate.

That should be done in a later patch so no changes required at the moment.

[vkalintiris]

Handle type promotion for return values through RetCC_Mips.

[dsanders]

Thanks. It still needs a testcase though. You may be able to re-use the calling convention test at test/CodeGen/Mips/cconv/return.ll if support for loads is good enough.

[vkalintiris]

Added a testcase and modified the patch to handle ret values for i1/i8/i16
when they don't require sign/zero extension.

[vkalintiris]

In D6765#162473, @dsanders wrote:

You may be able to re-use the calling convention test at test/CodeGen/Mips/cconv/return.ll if support for loads is good enough.

There's not any test for FastISel outside from the test/CodeGen/Mips/Fast-ISel directory. That's why I added the tests in the Fast-ISel directory. In the future, I would like to rename the test/CodeGen/Mips/Fast-ISel/retabi.ll file to ret.ll as we check essentially for the ret IR instruction.

[dsanders]

LGTM

In D6765#163069, @vkalintiris wrote:

In D6765#162473, @dsanders wrote:

You may be able to re-use the calling convention test at test/CodeGen/Mips/cconv/return.ll if support for loads is good enough.

There's not any test for FastISel outside from the test/CodeGen/Mips/Fast-ISel directory. That's why I added the tests in the Fast-ISel directory. In the future, I would like to rename the test/CodeGen/Mips/Fast-ISel/retabi.ll file to ret.ll as we check essentially for the ret IR instruction.

That's true at the moment, but in the long term I'd like to merge the ABI tests for FastISel with the ABI tests for SelectionDAG. I'd be surprised if they produce significantly different output for such simple tests.

test/CodeGen/Mips/Fast-ISel/retabi.ll

34 ↗

(On Diff #24605)

It's mandatory to use $2 so $[[REG_S:[0-9]+]] should check for $2 instead. Similar for the other new test cases.

lib/Target/Mips/MipsCallingConv.td

def CC_MipsO32 : CallingConv<[
   // Integer values get stored in stack slots that are 8 bytes in
   // size and 8-byte aligned.
   CCIfType<[f64], CCAssignToStack<8, 8>>
 ]>;
 
 // Only the return rules are defined here for O32. The rules for argument
 // passing are defined in MipsISelLowering.cpp.
 def RetCC_MipsO32 : CallingConv<[
+  // Promote i1/i8/i16 return values to i32.
+  CCIfType<[i1, i8, i16], CCPromoteToType<i32>>,
+
   // i32 are returned in registers V0, V1, A0, A1
   CCIfType<[i32], CCAssignToReg<[V0, V1, A0, A1]>>,
 
   // f32 are returned in registers F0, F2
   CCIfType<[f32], CCAssignToReg<[F0, F2]>>,
 
   // f64 arguments are returned in D0_64 and D2_64 in FP64bit mode or
   // in D0 and D1 in FP32bit mode.

lib/Target/Mips/MipsFastISel.cpp

 
 bool MipsFastISel::finishCall(CallLoweringInfo &CLI, MVT RetVT,
                               unsigned NumBytes) {
   CallingConv::ID CC = CLI.CallConv;
   emitInst(Mips::ADJCALLSTACKUP).addImm(16);
   if (RetVT != MVT::isVoid) {
     SmallVector<CCValAssign, 16> RVLocs;
     CCState CCInfo(CC, false, *FuncInfo.MF, RVLocs, *Context);
     CCInfo.AnalyzeCallResult(RetVT, RetCC_Mips);
 
     // Only handle a single return value.
     if (RVLocs.size() != 1)
       return false;
     // Copy all of the result registers out of their specified physreg.
     MVT CopyVT = RVLocs[0].getValVT();
     // Special handling for extended integers.
     if (RetVT == MVT::i1 || RetVT == MVT::i8 || RetVT == MVT::i16)
       CopyVT = MVT::i32;
 
     unsigned ResultReg = createResultReg(TLI.getRegClassFor(CopyVT));
+    if (!ResultReg)
+      return false;
     BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
             TII.get(TargetOpcode::COPY),
             ResultReg).addReg(RVLocs[0].getLocReg());
     CLI.InRegs.push_back(RVLocs[0].getLocReg());
 
     CLI.ResultReg = ResultReg;
     CLI.NumResultRegs = 1;
   }
bool MipsFastISel::fastLowerCall(CallLoweringInfo &CLI) {
   // Let SDISel handle vararg functions.
   if (IsVarArg)
     return false;
 
   // FIXME: Only handle *simple* calls for now.
   MVT RetVT;
   if (CLI.RetTy->isVoidTy())
     RetVT = MVT::isVoid;
-  else if (!isTypeLegal(CLI.RetTy, RetVT))
+  else if (!isTypeSupported(CLI.RetTy, RetVT))
     return false;
 
   for (auto Flag : CLI.OutFlags)
     if (Flag.isInReg() || Flag.isSRet() || Flag.isNest() || Flag.isByVal())
       return false;
 
   // Set up the argument vectors.
   SmallVector<MVT, 16> OutVTs;

 }
 
 bool MipsFastISel::emitIntExt(MVT SrcVT, unsigned SrcReg, MVT DestVT,
                               unsigned DestReg, bool IsZExt) {
   // FastISel does not have plumbing to deal with extensions where the SrcVT or
   // DestVT are odd things, so test to make sure that they are both types we can
   // handle (i1/i8/i16/i32 for SrcVT and i8/i16/i32/i64 for DestVT), otherwise
   // bail out to SelectionDAG.
   if (((DestVT != MVT::i8) && (DestVT != MVT::i16) && (DestVT != MVT::i32)) ||
       ((SrcVT != MVT::i1) && (SrcVT != MVT::i8) && (SrcVT != MVT::i16)))
     return false;
   if (IsZExt)
     return emitIntZExt(SrcVT, SrcReg, DestVT, DestReg);
   return emitIntSExt(SrcVT, SrcReg, DestVT, DestReg);
 }
 
 unsigned MipsFastISel::emitIntExt(MVT SrcVT, unsigned SrcReg, MVT DestVT,
                                   bool isZExt) {