[mips][FastISel] Instantiate the MipsFastISel class only for targets that support FastISel.

[vkalintiris]

Summary

Instead of instantiating the MipsFastISel class and checking if the
target is supported in the overriden methods, we should perform that
check before creating the class. This allows us to enable FastISel *only*
for targets that truly support it, ie. MIPS32 to MIPS32R5.

[sdardis]

Comments inlined.

lib/Target/Mips/MipsISelLowering.cpp

464–465

I find this comment strange given the block beneath. I think this should be:

// FastISel is supported only for [MIPS32,MIPS32R5] ISAs.
bool UseFastISel = TM.Options.EnableFastISel && Subtarget.hasMips32() && !Subtarget.hasMips32r6());

467–469

With my comment above, I think this should check for inMicroMips(), with the comment reflecting that we don't support compressed instruction sets.

Aside, I've barely looked but is mips16e rejected at a higher level for FastISel?

[vkalintiris]

Cleanup FastISel support checks and remember to reject it on MIPS16 as well.

[ehostunreach]

lib/Target/Mips/MipsISelLowering.cpp

464–465

That was intentional as a coding style, ie. set UseFastISel and use the checks that follow to unset it. I merged the ISA-checks together.

467–469

I added a check for the MIPS16 mode and a test case as well.

[sdardis]

LTGM, with one comment nit, inlined.

lib/Target/Mips/MipsISelLowering.cpp

464–465

My request here is that the comment on what ISAs we support doesn't mention MIPS32R6. So that comment should be:

// We support only the standard encoding [MIPS32,MIPS32R5] ISAs.

Otherwise the comments don't quite match the code. IMHO.

[vkalintiris]

lib/Target/Mips/MipsISelLowering.cpp

464–465

Sorry, I didn't understand you initial comment. I updated the review before committing the patch and replaced the MIPS32R6 bit with the MIPS32R5 ISA.

lib/Target/Mips/MipsFastISel.cpp

class MipsFastISel final : public FastISel {
 
   // Convenience variables to avoid some queries.
   LLVMContext *Context;
 
   bool fastLowerArguments() override;
   bool fastLowerCall(CallLoweringInfo &CLI) override;
   bool fastLowerIntrinsicCall(const IntrinsicInst *II) override;
 
-  bool TargetSupported;
   bool UnsupportedFPMode; // To allow fast-isel to proceed and just not handle
   // floating point but not reject doing fast-isel in other
   // situations
 
 private:
   // Selection routines.
   bool selectLogicalOp(const Instruction *I);
   bool selectLoad(const Instruction *I);
public:
   // Backend specific FastISel code.
   explicit MipsFastISel(FunctionLoweringInfo &funcInfo,
                         const TargetLibraryInfo *libInfo)
       : FastISel(funcInfo, libInfo), TM(funcInfo.MF->getTarget()),
         Subtarget(&funcInfo.MF->getSubtarget<MipsSubtarget>()),
         TII(*Subtarget->getInstrInfo()), TLI(*Subtarget->getTargetLowering()) {
     MFI = funcInfo.MF->getInfo<MipsFunctionInfo>();
     Context = &funcInfo.Fn->getContext();
-    bool ISASupported = !Subtarget->hasMips32r6() &&
-                        !Subtarget->inMicroMipsMode() && Subtarget->hasMips32();
-    TargetSupported =
-        ISASupported && TM.isPositionIndependent() && getABI().IsO32();
-    UnsupportedFPMode = Subtarget->isFP64bit();
+    UnsupportedFPMode = Subtarget->isFP64bit() || Subtarget->useSoftFloat();
   }
 
   unsigned fastMaterializeAlloca(const AllocaInst *AI) override;
   unsigned fastMaterializeConstant(const Constant *C) override;
   bool fastSelectInstruction(const Instruction *I) override;
 
 #include "MipsGenFastISel.inc"
 };
unsigned MipsFastISel::emitLogicalOp(unsigned ISDOpc, MVT RetVT,
   if (!ResultReg)
     return 0;
 
   emitInst(Opc, ResultReg).addReg(LHSReg).addReg(RHSReg);
   return ResultReg;
 }
 
 unsigned MipsFastISel::fastMaterializeAlloca(const AllocaInst *AI) {
-  if (!TargetSupported)
-    return 0;
-
   assert(TLI.getValueType(DL, AI->getType(), true) == MVT::i32 &&
          "Alloca should always return a pointer.");
 
   DenseMap<const AllocaInst *, int>::iterator SI =
       FuncInfo.StaticAllocaMap.find(AI);
 
   if (SI != FuncInfo.StaticAllocaMap.end()) {
     unsigned ResultReg = createResultReg(&Mips::GPR32RegClass);
emitInst(Mips::LW, DestReg)
       .addReg(MFI->getGlobalBaseReg())
       .addSym(Sym, MipsII::MO_GOT);
   return DestReg;
 }
 
 // Materialize a constant into a register, and return the register
 // number (or zero if we failed to handle it).
 unsigned MipsFastISel::fastMaterializeConstant(const Constant *C) {
-  if (!TargetSupported)
-    return 0;
-
   EVT CEVT = TLI.getValueType(DL, C->getType(), true);
 
   // Only handle simple types.
   if (!CEVT.isSimple())
     return 0;
   MVT VT = CEVT.getSimpleVT();
 
   if (const ConstantFP *CFP = dyn_cast<ConstantFP>(C))
bool MipsFastISel::fastLowerArguments() {
 
   MF->getInfo<MipsFunctionInfo>()->setFormalArgInfo(IncomingArgSizeInBytes,
                                                     false);
 
   return true;
 }
 
 bool MipsFastISel::fastLowerCall(CallLoweringInfo &CLI) {
-  if (!TargetSupported)
-    return false;
-
   CallingConv::ID CC = CLI.CallConv;
   bool IsTailCall = CLI.IsTailCall;
   bool IsVarArg = CLI.IsVarArg;
   const Value *Callee = CLI.Callee;
   MCSymbol *Symbol = CLI.Symbol;
 
   // Do not handle FastCC.
   if (CC == CallingConv::Fast)
bool MipsFastISel::fastLowerCall(CallLoweringInfo &CLI) {
 
   CLI.Call = MIB;
 
   // Finish off the call including any return values.
   return finishCall(CLI, RetVT, NumBytes);
 }
 
 bool MipsFastISel::fastLowerIntrinsicCall(const IntrinsicInst *II) {
-  if (!TargetSupported)
-    return false;
-
   switch (II->getIntrinsicID()) {
   default:
     return false;
   case Intrinsic::bswap: {
     Type *RetTy = II->getCalledFunction()->getReturnType();
 
     MVT VT;
     if (!isTypeSupported(RetTy, VT))
bool MipsFastISel::selectShift(const Instruction *I) {
   }
 
   emitInst(Opcode, ResultReg).addReg(Op0Reg).addReg(Op1Reg);
   updateValueMap(I, ResultReg);
   return true;
 }
 
 bool MipsFastISel::fastSelectInstruction(const Instruction *I) {
-  if (!TargetSupported)
-    return false;
   switch (I->getOpcode()) {
   default:
     break;
   case Instruction::Load:
     return selectLoad(I);
   case Instruction::Store:
     return selectStore(I);
   case Instruction::SDiv:

lib/Target/Mips/MipsISelLowering.cpp

const MipsTargetLowering *MipsTargetLowering::create(const MipsTargetMachine &TM,
 
   return llvm::createMipsSETargetLowering(TM, STI);
 }
 
 // Create a fast isel object.
 FastISel *
 MipsTargetLowering::createFastISel(FunctionLoweringInfo &funcInfo,
                                   const TargetLibraryInfo *libInfo) const {
-  if (!funcInfo.MF->getTarget().Options.EnableFastISel)
-    return TargetLowering::createFastISel(funcInfo, libInfo);
-  return Mips::createFastISel(funcInfo, libInfo);
+  const MipsTargetMachine &TM =
+      static_cast<const MipsTargetMachine &>(funcInfo.MF->getTarget());
+
+  // We support only the standard encoding [MIPS32,MIPS32R6) ISAs.
+  bool UseFastISel = TM.Options.EnableFastISel && Subtarget.hasMips32() &&
+                     !Subtarget.hasMips32r6() && !Subtarget.inMips16Mode() &&
+                     !Subtarget.inMicroMipsMode();
+
+  // Disable if we don't generate PIC or the ABI isn't O32.
+  if (!TM.isPositionIndependent() || !TM.getABI().IsO32())
+    UseFastISel = false;
+
+  return UseFastISel ? Mips::createFastISel(funcInfo, libInfo) : nullptr;
 }
 
 EVT MipsTargetLowering::getSetCCResultType(const DataLayout &, LLVMContext &,
                                            EVT VT) const {
   if (!VT.isVector())
     return MVT::i32;
   return VT.changeVectorElementTypeToInteger();
 }

test/CodeGen/Mips/Fast-ISel/check-disabled-mcpus.ll

+; Targets where we should not enable FastISel.
 ; RUN: llc -march=mips -mcpu=mips2 -O0 -relocation-model=pic \
 ; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s
 ; RUN: llc -march=mips -mcpu=mips3 -O0 -relocation-model=pic -target-abi n64 \
 ; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s
 ; RUN: llc -march=mips -mcpu=mips4 -O0 -relocation-model=pic -target-abi n64 \
 ; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s
 
 ; RUN: llc -march=mips -mcpu=mips32r6 -O0 -relocation-model=pic \
 ; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s
+
+; RUN: llc -march=mips -mattr=mips16 -O0 -relocation-model=pic \
+; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s
+
 ; RUN: llc -march=mips -mcpu=mips32r2 -mattr=+micromips -O0 -relocation-model=pic \
 ; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s
+; RUN: llc -march=mips -mcpu=mips32r3 -mattr=+micromips -O0 -relocation-model=pic \
+; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s
+; RUN: llc -march=mips -mcpu=mips32r5 -mattr=+micromips -O0 -relocation-model=pic \
+; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s
 
 ; RUN: llc -march=mips -mcpu=mips64 -O0 -relocation-model=pic -target-abi n64 \
 ; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s
 ; RUN: llc -march=mips -mcpu=mips64r2 -O0 -relocation-model=pic -target-abi n64 \
 ; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s
 ; RUN: llc -march=mips -mcpu=mips64r3 -O0 -relocation-model=pic -target-abi n64 \
 ; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s
 ; RUN: llc -march=mips -mcpu=mips64r5 -O0 -relocation-model=pic -target-abi n64 \
 ; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s
 ; RUN: llc -march=mips -mcpu=mips32r6 -O0 -relocation-model=pic \
 ; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s
 
-; CHECK: FastISel missed terminator:   ret i32 0
+; Valid targets for FastISel.
+; RUN: llc -march=mips -mcpu=mips32r0 -O0 -relocation-model=pic \
+; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s -check-prefix=FISEL
+; RUN: llc -march=mips -mcpu=mips32r2 -O0 -relocation-model=pic \
+; RUN:     -fast-isel-verbose <%s 2>&1 | FileCheck %s -check-prefix=FISEL
+
+; The CHECK prefix is being used by those targets that do not support FastISel.
+; By checking that we don't emit the "FastISel missed terminator..." message,
+; we ensure that we do not generate code through FastISel.
+
+; CHECK-NOT: FastISel missed terminator:   ret i64 0
+
+; The above CHECK will only be valid as long as we *do* emit the missed
+; terminator message for targets that support FastISel. If we add support
+; for i64 return values in the future, then the following FISEL check-prefix
+; will fail and we will have to come up with a new test.
+
+; FISEL: FastISel missed terminator:   ret i64 0
 
-define i32 @foo() {
+define i64 @foo() {
 entry:
-  ret i32 0
+  ret i64 0
 }

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

-; RUN: not llc -march=mipsel -mcpu=mips32r2 -fast-isel -mattr=+fp64 < %s \
-; RUN:     -fast-isel-abort=3
+; RUN: not llc -march=mipsel -mcpu=mips32r2 -mattr=+fp64 \
+; RUN:         -O0 -relocation-model=pic -fast-isel-abort=3 < %s
 
 ; Check that FastISel aborts when we have 64bit FPU registers. FastISel currently
 ; supports AFGR64 only, which 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
 }

test/CodeGen/Mips/tailcall/tail-call-arguments-clobber.ll

 ; loads and stores.
 
 ; O32 case: The last two arguments should appear at 16(sp), 20(sp). The order
 ;           of the loads doesn't matter, but they have to become before the
 ;           stores
 declare i32 @func2(i32, i32, i32, i32, i32, i32)
 
 define i32 @func1(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e, i32 %f){
+; MIPS32-LABEL: func1:
 
 ; MIPS32: lw ${{[0-9]+}}, {{[0-9]+}}($sp)
 ; MIPS32-NEXT: lw ${{[0-9]+}}, {{[0-9]+}}($sp)
 ; MIPS32-NEXT: sw ${{[0-9]+}}, {{[0-9]+}}($sp)
 ; MIPS32-NEXT: sw ${{[0-9]+}}, {{[0-9]+}}($sp)
   %retval = tail call i32 @func1(i32 %a, i32 %f, i32 %c, i32 %d, i32 %e, i32 %b)
 
   ret i32 %retval
 }
 
 ; N64, N32 cases: N64 and N32 both pass 8 arguments in registers. The order
 ;           of the loads doesn't matter, but they have to become before the
 ;           stores
 
 declare i64 @func4(i64, i64, i64, i64, i64, i64, i64, i64, i64, i64)
 
 define i64 @func3(i64 %a, i64 %b, i64 %c, i64 %d,
                   i64 %e, i64 %f, i64 %g, i64 %h,
                   i64 %i, i64 %j){
+; MIPS64-LABEL: func3:
 
 ; MIPS64: ld ${{[0-9]+}}, {{[0-9]+}}($sp)
 ; MIPS64-NEXT: ld ${{[0-9]+}}, {{[0-9]+}}($sp)
 ; MIPS64-NEXT: sd ${{[0-9]+}}, {{[0-9]+}}($sp)
 ; MIPS64-NEXT: sd ${{[0-9]+}}, {{[0-9]+}}($sp)
   %retval = tail call i64 @func4(i64 %a, i64 %j, i64 %c, i64 %d, i64 %e, i64 %f, i64 %g, i64 %h, i64 %i, i64 %b)
 
   ret i64 %retval
 }
 
 

test/DebugInfo/Mips/dsr-non-fixed-objects.ll

-; RUN: llc -march=mips -mcpu=mips32r2 -O0 -filetype=obj <%s | \
+; RUN: llc -march=mips -mcpu=mips32r2 -O0 -filetype=obj -fast-isel=0 <%s | \
 ; RUN:    llvm-dwarfdump -debug-dump=all - | FileCheck %s -check-prefix=F2
-; RUN: llc -march=mips -mcpu=mips32r2 -O0 -filetype=obj <%s | \
+; RUN: llc -march=mips -mcpu=mips32r2 -O0 -filetype=obj -fast-isel=0 <%s | \
 ; RUN:    llvm-dwarfdump -debug-dump=all - | FileCheck %s -check-prefix=F3
 
 declare void @llvm.dbg.declare(metadata, metadata, metadata)
 
 declare void @foo(i32*)
 
 ; void foo(int *);
 ;