[mips] Tighten FastISel restrictions

[sdardis]

Summary

LLVM PR/29052 highlighted that FastISel for MIPS attempted to lower
arguments assuming that it was using the paired 32bit registers to
perform operations for f64. This mode of operation is not supported
for MIPSR6.

This patch resolves the reported issue by adding additional checks
for unsupported floating point unit configuration.

Thanks to mike.k for reporting this issue!

[seanbruno]

For what its worth, this makes the clang enabled build of FreeBSD MIPS64 no longer require any of the fast-isel options that I have been using for the last few weeks.

[sdardis]

Ping?

[vkalintiris]

The code changes LGTM but the test case doesn't look like it is testing the right thing.

test/CodeGen/Mips/Fast-ISel/double-arg.ll

1–2

I'm not sure why we need this RUN test. I would expect a not llc -mattr=fp64 ... test in order to make sure that for 64bit FPU registers we don't follow the FastISel path.

3–4

This test case is covered by check-disabled-mcpus.ll as we don't support MIPS R6 for FastISel.

[sdardis]

Address review comments on test, fix a comment in test.

[vkalintiris]

Thanks! LGTM.

[sdardis]

Thanks, committed rL280706.

lib/Target/Mips/MipsFastISel.cpp

bool MipsFastISel::selectFPExt(const Instruction *I) {
   emitInst(Mips::CVT_D32_S, DestReg).addReg(SrcReg);
   updateValueMap(I, DestReg);
   return true;
 }
 
 bool MipsFastISel::selectSelect(const Instruction *I) {
   assert(isa<SelectInst>(I) && "Expected a select instruction.");
 
+  DEBUG(dbgs() << "selectSelect\n");
+
   MVT VT;
-  if (!isTypeSupported(I->getType(), VT))
+  if (!isTypeSupported(I->getType(), VT) || UnsupportedFPMode) {
+    DEBUG(dbgs() << ".. .. gave up (!isTypeSupported || UnsupportedFPMode)\n");
     return false;
+  }
 
   unsigned CondMovOpc;
   const TargetRegisterClass *RC;
 
   if (VT.isInteger() && !VT.isVector() && VT.getSizeInBits() <= 32) {
     CondMovOpc = Mips::MOVN_I_I;
     RC = &Mips::GPR32RegClass;
   } else if (VT == MVT::f32) {
case MVT::f32:
       // ABI rules require us to skip the corresponding GPR32.
       if (NextGPR32 != GPR32ArgRegs.end())
         NextGPR32++;
       if (NextAFGR64 != AFGR64ArgRegs.end())
         NextAFGR64++;
       break;
 
     case MVT::f64:
+      if (UnsupportedFPMode) {
+        DEBUG(dbgs() << ".. .. gave up (UnsupportedFPMode\n");
+        return false;
+      }
       if (NextAFGR64 == AFGR64ArgRegs.end()) {
         DEBUG(dbgs() << ".. .. gave up (ran out of AFGR64 arguments)\n");
         return false;
       }
       DEBUG(dbgs() << ".. .. AFGR64(" << *NextAFGR64 << ")\n");
       Allocation.emplace_back(&Mips::AFGR64RegClass, *NextAFGR64++);
       // Allocating an FGR32 also allocates the super-register AFGR64, and
       // ABI rules require us to skip the corresponding GPR32 pair.
bool MipsFastISel::fastLowerIntrinsicCall(const IntrinsicInst *II) {
   }
   return false;
 }
 
 bool MipsFastISel::selectRet(const Instruction *I) {
   const Function &F = *I->getParent()->getParent();
   const ReturnInst *Ret = cast<ReturnInst>(I);
 
+  DEBUG(dbgs() << "selectRet\n");
+
   if (!FuncInfo.CanLowerReturn)
     return false;
 
   // Build a list of return value registers.
   SmallVector<unsigned, 4> RetRegs;
 
   if (Ret->getNumOperands() > 0) {
     CallingConv::ID CC = F.getCallingConv();
if (Ret->getNumOperands() > 0) {
 
     if (RVEVT.isVector())
       return false;
 
     MVT RVVT = RVEVT.getSimpleVT();
     if (RVVT == MVT::f128)
       return false;
 
+    // Do not handle FGR64 returns for now.
+    if (RVVT == MVT::f64 && UnsupportedFPMode) {
+      DEBUG(dbgs() << ".. .. gave up (UnsupportedFPMode\n");
+      return false;
+    }
+
     MVT DestVT = VA.getValVT();
     // Special handling for extended integers.
     if (RVVT != DestVT) {
       if (RVVT != MVT::i1 && RVVT != MVT::i8 && RVVT != MVT::i16)
         return false;
 
       if (Outs[0].Flags.isZExt() || Outs[0].Flags.isSExt()) {
         bool IsZExt = Outs[0].Flags.isZExt();

test/CodeGen/Mips/Fast-ISel/double-arg.ll

+; RUN: llc -march=mipsel -mcpu=mips32r2 -fast-isel -relocation-model=pic < %s \
+; RUN:     -fast-isel-abort=3 | FileCheck %s
+; RUN: not llc -march=mipsel -mcpu=mips32r6 -fast-isel -relocation-model=pic < %s \
+; RUN:     -fast-isel-abort=3 2>&1
+
+; Check that FastISel aborts when we have 64bit FPU registers. FastISel currently
+; supports AFGR64 only, which has uses paired 32 bit registers.
+
+define zeroext i1 @f(double %value) {
+entry:
+; CHECK-LABEL: f:
+; CHECK: sdc1
+  %value.addr = alloca double, align 8
+  store double %value, double* %value.addr, align 8
+  ret i1 false
+}