Beginning of alloca implementation for Mips fast-isel

[rkotler]

Summary

Begin to add various address modes; including alloca.

[rkotler]

Fix a bug for this push for when offsets do not fit in the 16 bit signed offset.
I would have put this in a separate patch but the patch needs some of the new features in this
patch; otherwise it would have to be rewritten for this patch.

[rkotler]

load1.ll from initial patch is still part of this.

i'm not sure why it did not show up in the latest patch. i will fix that.

load1.ll does basic alloca functionality testing.

[dsanders]

Sorry it's taken me so long to get to your patches.

lib/Target/Mips/MipsFastISel.cpp

324–325

Nit: I know this is from an earlier patch but the R in AARch64 should be lowercase and there's an unnecessary blank comment line on the line above.

324–337

Nit: Indentation

361–362

I see that the AArch64 port did this too but why use goto when the target of the goto simply does 'break'?

658

It's not very obvious that we're changing Addr here (and at the other points this is called). Returning the new reference would be clearer

1363

Nit: What are we ensuring about the offset? Shouldn't we use ensureOffsetWithinRange or something along those lines?

1364–1365

This if-statement is half-inside and half-outside this function which feels a bit odd. I think it would be best to pass in the size of the constant (and maybe the signedness) and have the if-statement fully inside ensureOffset().

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

19

Nit: indentation

45–46

You define Y_IDX twice here. Shouldn't the second one be a usage.
I'm also not sure about the name Y_IDX since it's not an index.

[dsanders]

lib/Target/Mips/MipsFastISel.cpp

169–170

Nit: Capitalize first letter and delete blank comment line.

173

Nit: Unnecessary blank line

[dsanders]

lib/Target/Mips/MipsFastISel.cpp

361–362

To answer my own question after re-reading the code: Because we're trying to break out of two for-loops and the break at the target is for the switch-statement.

[rkotler]

Patch has been updated to be consistent with current state of LLVM (this patch is old). This version should have no functional changes and has been checked against test-suite at O0/O2 and for mips32 R1/R2.

[rkotler]

Run clang format on patch.

[echristo]

Hi Reed,

Some quick inline comments.

+ assert(isFIBase() && "Invalid base frame index access!");

formatting.

• const MipsSubtarget *Subtarget; const TargetInstrInfo &TII; const TargetLowering &TLI;

+ const MipsSubtarget *Subtarget;

Unnecessary.

+ Subtarget(&TM.getSubtarget<MipsSubtarget>()) {

No.

+ TargetSupported = ((Subtarget->getRelocationModel() == Reloc::PIC_) &&
+ ((Subtarget->hasMips32r2() ||
Subtarget->hasMips32()) &&
+ (Subtarget->isABI_O32())));

Ditto.

+ This code is mostly cloned from AArch64 (which cloned it from earlier
+ ports)'

Unnecessary.

+ unsigned Offset = Addr.getOffset();
+ ;
+ MachineFrameInfo &MFI = *MF->getFrameInfo();
+ ;

?

-eric

[rkotler]

Not sure I understand some of these comments.

[echristo]

Unnecessary = "You shouldn't need to make this change"
No = "You can't make this change"

-eric

[rkotler]

Hi Eric,

I appreciate you taking the time to do the review but please make your
comments using phabricator using the phabricator online
interface so that you can put them right where the source goes.

When you make one block it's hard to track them and in this case when I
responded to the email, it threw
away my response to your comments from the phabricator.

Are your comments all formatting issues?

In general we are using clang-format -i for all new Mips code.

Are you looking at this patch in Phabricator so you can see how the code
really looks when the patch is applied?

Reed

[rkotler]

Subtarget is initialized first and then TargetSupported.

It may have been a bad choice of name but TargetSupported is derived
from Subtarget.
That name has been in Mips fast-isel for a long time now. It's a local
variable in the mips fast-isel class.

Probably should have been called SubtargetSupported.

[rkotler]

Unfortunately, my patches that I'm trying to finally commit now are
very old and they have been blocked
in the patch review queue for months now.

Some things are getting done during auto merge that may not be intended.

Since the original patches some other people have made some cosmetic
changes to mips fast-isel and some
other things have changed in llvm.

I can go back and look at the patch history for these lines but if you
have something specific you want you
can just say it.

[rkotler]

I see that you and Chandler made some changes in this area.

I will go back and make sure that this change does not undo those.

[rkotler]

I'm going to back to redo this patch sequence.

The auto merge of my old patch did not work and I need to go back and do
it all by hand
from the master branch so that you and Chandlers patch are not undone in
any way.

Thanks for noticing that.

Reed

[echristo]

Cool deal. Thanks!

-eric

[rkotler]

This hopefully freshens up the patch to current tip of tree without undoing any of Chandler or Eric Cs changes.
I am rerunning test-suite and then will make Daniels requested changes and any of Erics which still remain
after this rebasing.

[rkotler]

Will upload changed source reflecting the latest comments I posted.

lib/Target/Mips/MipsFastISel.cpp

658

I've made the change so that it returns an Address but personally I think that this makes it harder to read and I don't see why it's confusing to modify a reference variable in C++, that's what they are there for.

1363

I've changed the name to ensureOffsetFits.

If the offset does not fit, then the integer is materialized and the signness is taken care of there.

[rkotler]

Test suite passes on O0/O2 for Mips 32 r1/r2.

[dsanders]

+ TargetSupported = ((Subtarget->getRelocationModel() == Reloc::PIC_) &&
+ ((Subtarget->hasMips32r2() ||
Subtarget->hasMips32()) &&
+ (Subtarget->isABI_O32())));

Ditto.

I'd like to get rid of TargetSupported and replace it with more precisely targetted checks. I don't like the way we can't even do addition of two i32's if we aren't generating PIC code. When it was last discussed, I relented and allowed it for now on the understanding that it will be removed in the future.

+ unsigned Offset = Addr.getOffset();
+ ;
+ MachineFrameInfo &MFI = *MF->getFrameInfo();
+ ;

?

I'm not sure how I missed those in the earlier reviews. There seem to be a few of them dotted around. I've noted the ones I've spotted (including the ones you pointed out). They should probably be fixed in a separate commit to avoid adding noise to this one.

lib/Target/Mips/MipsFastISel.cpp

169–170

Done

173

Done

324–325

Done (comment was removed)

324–337

Done

657–658

Nit: Redundant semi-colon

658

They have value syntax which makes them indistinguishable from by-value arguments. If you were already using the return value for something else then I wouldn't have mentioned it but on this occasion we are able to be explicit and explicit is better than implicit.

667

Nit: Redundant semi-colon

669

Nit: Redundant semi-colon

1363

Thanks

1363

Nit: Redundant namespace? MipsFastISel::Address -> Address.

1364–1365

This one hasn't been done

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

19

This nit hasn't been fixed.

45–46

Done

[rkotler]

Getting rid of TargetSupported is possible but we need to add more build bots locally at Imagination to test for non pic and it's currently not in scope for my work now but management decided earlier that you can increase the scope but I just wanted to make it clear that this is an increase of scope of the project to non pic.

At this point, to expand this to non pic is not a big deal.

lib/Target/Mips/MipsFastISel.cpp

667

These are from cutting and pasting code from other ports and deleting a line but not the semicolon but sometimes they are hard to see until I run clang-format and they end up on their own line.

Everything compiles and all tests run so they are easy to miss because there is no functional difference whether they are there or not.

1363

Address is a local type of class MipsFastISel so it's not visible at this point.

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

19

I think this was just because I did not run untabify on this.

45–46

That was a typo. It should have just been another use of Y_IDX

[rkotler]

Incorporate latest comments.

[rkotler]

New patch coming... something was wrong with this diff.

[rkotler]

Missed a pair of superfluous semicolons.

[echristo]

Some inline comments.

lib/Target/Mips/MipsFastISel.cpp

1

Could you fix this comment (in a separate commit) while you're at it?

9

You don't need both includes.

325–337

I might be missing something but Ty looks unused except for passing it down to other invocations of computeAddress?

1363

This routine is just weirdly named/parametered. I'd probably just remove the boolean parameter and do the check inside. It'll make much more sense from the outside too.

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

11

Is this straight from C code or did you construct this up?

[rkotler]

Add one more requested change. Will rerun test-suite at O0/O2 and mips32 r1/r2 on this change.

[echristo]

In D6426#125960, @rkotler wrote:

Add one more requested change. Will rerun test-suite at O0/O2 and mips32 r1/r2 on this change.

This didn't fix anything I brought out in my inline comments from the previous revision. Please take a look at those.

Thanks.

-eric

[rkotler]

Sorry. I will make your changes.

Daniel sent me some private email and i was doing those. I did not see
your comments yet.

[rkotler]

Incorporate Eric Christopher's changes.

[echristo]

You missed a few comments. I've redone them inline and added more.

Thanks!

-eric

lib/Target/Mips/MipsFastISel.cpp

2

Go ahead and fix this up in a separate commit. Pre-approved.

325–337

Ty appears still unused?

659

I think the frequent offset checks are going to be inherently buggy. How about doing something ala ARMComputeAddress (or the equivalent in aarch64)?

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

7

Is this from C code?

[rkotler]

Eliminate unused parameter in ComputeAddress per Eric Christophers suggestion.

I will rerun test-suite after.

[echristo]

Please respond to and/or handle the rest of my comments.

Thanks.

-eric

[rkotler]

lib/Target/Mips/MipsFastISel.cpp

1

Okay. I've copied the header for AArch64, making appropriate changes for Mips.

9

fixed

325–337

This is copied from the other ports. I'm trying to not make changes from the other ports except where necessary. Probably 75% of all the ports code should really be common code and at some time in the future I'm hoping that someone will refactor this.

325–337

Good catch. I just copied that code from AArch64 which also does not seem to use it.
It compiles fine without that. Will run test-suite again.

659

I will investigate this idea for a future patch but I don't want to do more here. Good point though. This kind of code occurs all over llvm so it's not out of style. I have 10 patches in the queue for mips fast-isel and I need to get caught up.

Which code were you thinking of in ARMFastIsel?

1363

In my latest change I removed the boolean. Daniel suggested the same thing. I also renamed it to materializeOffset.

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

7

Originally. Almost every make check test I have written was derived from a standalone executable test. Many, but not all, were originally C code. This one seems to be.

I make sure the executable test works and then usually lop off "main" and compile with -emit-llvm and turn it into an LLVM test.

11

I often start with C code because my make check tests are from executable tests that have been checked.

[rkotler]

Sorry.

Had responded but did not hit the "submit" button.

[echristo]

Replies inline.

lib/Target/Mips/MipsFastISel.cpp

1

As I mentioned, please do this in a separate commit. It's also preapproved so just commit. Thanks.

659

We fix this kind of code as we see it and it's in review right now (also, since you're in here and handling all of this right now). The name of the function is ARMSimplifyAddress and it's immediately after ARMComputeAddress.

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

7

It works well then if you put the C code that you used in the file so that it can just be regenerated if something changes as a comment.

[dsanders]

I think we're pretty close now. To summarize, I think we just have the following todo:

• indentation nit on the 'lui' in the test case.
• adding the original C source for the test case.
• make materializeOffset() a little more ARMSimplifyAddress()-like by moving an if-statement inside it.

Getting rid of TargetSupported is possible but we need to add more build bots locally at
Imagination to test for non pic and it's currently not in scope for my work now but
management decided earlier that you can increase the scope but I just wanted to make
it clear that this is an increase of scope of the project to non pic.

I'm not saying we need to remove it now (or even soon). I'm just mentioning the background and the plan since I don't expect anyone will remember a discussion from June/July.

Daniel sent me some private email and i was doing those. I did not see your comments yet.

To be clear, this email didn't request anything that wasn't already in the review. We were discussing the review comments that hadn't been handled in the most recent patch.

lib/Target/Mips/MipsFastISel.cpp

659

While discussing this with Reed, I've previously said that I'd be happy so long as the condition was either fully inside or fully outside the function rather than the mixture that was in the earlier code. However, I also said that fully inside would be better in the long run since we'll eventually end up there when we add support for various kinds of offsets (e.g. MSA's 10-bit scaled offsets). I've had a quick look at ARMSimplifyAddress and it's pretty much how I picture materializeOffset() ending up.

That said, given that Eric has brought this up too I think we might as well do it now. I believe all we need to do at this stage is move this if-statement inside the function to resolve Eric's comment.

667

Done

669

Done

1363

Ok, presumably the argument doesn't need it since it gets the namespace from the function. Thanks for checking.

[rkotler]

Am in the processing of verifying this patch with test-suite at O0/O2 for mips 23 r1/r2

[rkotler]

test-suite passes on latest push on O0/O2 and mips32 R1/R2

[dsanders]

LGTM

lib/Target/Mips/MipsFastISel.cpp

1–3

Phabricator won't let me delete this inline comment for some reason but it will let me edit it.

I was going to ask why this change was reverted but I've gone back through the comments and answered my own question (it's being committed separately)

1363

I'm happy with this solution to the "it's not obvious the argument is modified" issue since the function name now implies the modification.

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

57–62

Nit: The blank lines should probably be ';' since this is one big block comment

lib/Target/Mips/MipsFastISel.cpp

 //===-- MipsastISel.cpp - Mips FastISel implementation
 //---------------------===//
 
 #include "MipsCCState.h"
+#include "MipsInstrInfo.h"
 #include "MipsISelLowering.h"
 #include "MipsMachineFunction.h"
 #include "MipsRegisterInfo.h"
+#include "MipsSEInstrInfo.h"
 #include "MipsSubtarget.h"
 #include "MipsTargetMachine.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/CodeGen/FastISel.h"
 #include "llvm/CodeGen/FunctionLoweringInfo.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/IR/GlobalAlias.h"
private:
     const GlobalValue *GV;
 
   public:
     // Innocuous defaults for our address.
     Address() : Kind(RegBase), Offset(0), GV(0) { Base.Reg = 0; }
     void setKind(BaseKind K) { Kind = K; }
     BaseKind getKind() const { return Kind; }
     bool isRegBase() const { return Kind == RegBase; }
+    bool isFIBase() const { return Kind == FrameIndexBase; }
     void setReg(unsigned Reg) {
       assert(isRegBase() && "Invalid base register access!");
       Base.Reg = Reg;
     }
     unsigned getReg() const {
       assert(isRegBase() && "Invalid base register access!");
       return Base.Reg;
     }
+    void setFI(unsigned FI) {
+      assert(isFIBase() && "Invalid base frame index access!");
+      Base.FI = FI;
+    }
+    unsigned getFI() const {
+      assert(isFIBase() && "Invalid base frame index access!");
+      return Base.FI;
+    }
+
     void setOffset(int64_t Offset_) { Offset = Offset_; }
     int64_t getOffset() const { return Offset; }
     void setGlobalValue(const GlobalValue *G) { GV = G; }
     const GlobalValue *getGlobalValue() { return GV; }
   };
 
   /// Subtarget - Keep a pointer to the MipsSubtarget around so that we can
   /// make the right decision when generating code for different targets.
private:
   bool selectFPToInt(const Instruction *I, bool IsSigned);
   bool selectRet(const Instruction *I);
   bool selectTrunc(const Instruction *I);
   bool selectIntExt(const Instruction *I);
 
   // Utility helper routines.
   bool isTypeLegal(Type *Ty, MVT &VT);
   bool isLoadTypeLegal(Type *Ty, MVT &VT);
-  bool computeAddress(const Value *Obj, Address &Addr);
+  bool computeAddress(const Value *Obj, Address &Addr, Type *Ty = nullptr);
   bool computeCallAddress(const Value *V, Address &Addr);
 
   // Emit helper routines.
   bool emitCmp(unsigned DestReg, const CmpInst *CI);
   bool emitLoad(MVT VT, unsigned &ResultReg, Address &Addr,
                 unsigned Alignment = 0);
   bool emitStore(MVT VT, unsigned SrcReg, Address Addr,
                  MachineMemOperand *MMO = nullptr);
private:
 
   // Call handling routines.
 private:
   CCAssignFn *CCAssignFnForCall(CallingConv::ID CC) const;
   bool processCallArgs(CallLoweringInfo &CLI, SmallVectorImpl<MVT> &ArgVTs,
                        unsigned &NumBytes);
   bool finishCall(CallLoweringInfo &CLI, MVT RetVT, unsigned NumBytes);
 
+private:
+  // if the offset does not fit then materialize it in a register first.
+  void ensureOffset(Address &Addr, bool FitsOffset);
+
 public:
   // Backend specific FastISel code.
   explicit MipsFastISel(FunctionLoweringInfo &funcInfo,
                         const TargetLibraryInfo *libInfo)
       : FastISel(funcInfo, libInfo), TM(funcInfo.MF->getTarget()),
         Subtarget(
             &static_cast<const MipsSubtarget &>(funcInfo.MF->getSubtarget())),
         TII(*Subtarget->getInstrInfo()), TLI(*Subtarget->getTargetLowering()) {
     MFI = funcInfo.MF->getInfo<MipsFunctionInfo>();
     Context = &funcInfo.Fn->getContext();
unsigned MipsFastISel::fastMaterializeConstant(const Constant *C) {
   else if (const GlobalValue *GV = dyn_cast<GlobalValue>(C))
     return materializeGV(GV, VT);
   else if (isa<ConstantInt>(C))
     return materializeInt(C, VT);
 
   return 0;
 }
 
-bool MipsFastISel::computeAddress(const Value *Obj, Address &Addr) {
-  // This construct looks a big awkward but it is how other ports handle this
-  // and as this function is more fully completed, these cases which
-  // return false will have additional code in them.
+bool MipsFastISel::computeAddress(const Value *Obj, Address &Addr, Type *Ty) {
   //
-  if (isa<Instruction>(Obj))
-    return false;
-  else if (isa<ConstantExpr>(Obj))
+  // This code is mostly cloned from AARch64 (which cloned it from earlier
+  // ports)
+  const User *U = nullptr;
+  unsigned Opcode = Instruction::UserOp1;
+  if (const Instruction *I = dyn_cast<Instruction>(Obj)) {
+    // Don't walk into other basic blocks unless the object is an alloca from
+    // another block, otherwise it may not have a virtual register assigned.
+    if (FuncInfo.StaticAllocaMap.count(static_cast<const AllocaInst *>(Obj)) ||
+        FuncInfo.MBBMap[I->getParent()] == FuncInfo.MBB) {
+      Opcode = I->getOpcode();
+      U = I;
+    }
+  } else if (isa<ConstantExpr>(Obj))
     return false;
+  switch (Opcode) {
+  default:
+    break;
+  case Instruction::BitCast: {
+    // Look through bitcasts.
+    return computeAddress(U->getOperand(0), Addr, Ty);
+  }
+  case Instruction::GetElementPtr: {
+    Address SavedAddr = Addr;
+    uint64_t TmpOffset = Addr.getOffset();
+    // Iterate through the GEP folding the constants into offsets where
+    // we can.
+    gep_type_iterator GTI = gep_type_begin(U);
+    for (User::const_op_iterator i = U->op_begin() + 1, e = U->op_end(); i != e;
+         ++i, ++GTI) {
+      const Value *Op = *i;
+      if (StructType *STy = dyn_cast<StructType>(*GTI)) {
+        const StructLayout *SL = DL.getStructLayout(STy);
+        unsigned Idx = cast<ConstantInt>(Op)->getZExtValue();
+        TmpOffset += SL->getElementOffset(Idx);
+      } else {
+        uint64_t S = DL.getTypeAllocSize(GTI.getIndexedType());
+        for (;;) {
+          if (const ConstantInt *CI = dyn_cast<ConstantInt>(Op)) {
+            // Constant-offset addressing.
+            TmpOffset += CI->getSExtValue() * S;
+            break;
+          }
+          if (canFoldAddIntoGEP(U, Op)) {
+            // A compatible add with a constant operand. Fold the constant.
+            ConstantInt *CI =
+                cast<ConstantInt>(cast<AddOperator>(Op)->getOperand(1));
+            TmpOffset += CI->getSExtValue() * S;
+            // Iterate on the other operand.
+            Op = cast<AddOperator>(Op)->getOperand(0);
+            continue;
+          }
+          // Unsupported
+          goto unsupported_gep;
+        }
+      }
+    }
+    // Try to grab the base operand now.
+    Addr.setOffset(TmpOffset);
+    if (computeAddress(U->getOperand(0), Addr, Ty))
+      return true;
+    // We failed, restore everything and try the other options.
+    Addr = SavedAddr;
+  unsupported_gep:
+    break;
+  }
+  case Instruction::Alloca: {
+    const AllocaInst *AI = cast<AllocaInst>(Obj);
+    DenseMap<const AllocaInst *, int>::iterator SI =
+        FuncInfo.StaticAllocaMap.find(AI);
+    if (SI != FuncInfo.StaticAllocaMap.end()) {
+      Addr.setKind(Address::FrameIndexBase);
+      Addr.setFI(SI->second);
+      return true;
+    }
+    break;
+  }
+  }
   Addr.setReg(getRegForValue(Obj));
   return Addr.getReg() != 0;
 }
 
 bool MipsFastISel::computeCallAddress(const Value *V, Address &Addr) {
   const GlobalValue *GV = dyn_cast<GlobalValue>(V);
   if (GV && isa<Function>(GV) && dyn_cast<Function>(GV)->isIntrinsic())
     return false;
if (UnsupportedFPMode)
       return false;
     ResultReg = createResultReg(&Mips::AFGR64RegClass);
     Opc = Mips::LDC1;
     break;
   }
   default:
     return false;
   }
+  if (Addr.isRegBase()) {
+    ensureOffset(Addr, isInt<16>(Addr.getOffset()));
     emitInstLoad(Opc, ResultReg, Addr.getReg(), Addr.getOffset());
     return true;
   }
+  if (Addr.isFIBase()) {
+    unsigned FI = Addr.getFI();
+    unsigned Align = 4;
+    unsigned Offset = Addr.getOffset();
+    MachineFrameInfo &MFI = *MF->getFrameInfo();
+    MachineMemOperand *MMO = MF->getMachineMemOperand(
+        MachinePointerInfo::getFixedStack(FI), MachineMemOperand::MOLoad,
+        MFI.getObjectSize(FI), Align);
+    BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(Opc), ResultReg)
+        .addFrameIndex(FI)
+        .addImm(Offset)
+        .addMemOperand(MMO);
+    return true;
+  }
+  return false;
+}
 
 bool MipsFastISel::emitStore(MVT VT, unsigned SrcReg, Address &Addr,
                              unsigned Alignment) {
   //
   // more cases will be handled here in following patches.
   //
   unsigned Opc;
   switch (VT.SimpleTy) {
case MVT::f32:
     break;
   case MVT::f64:
     if (UnsupportedFPMode)
       return false;
     Opc = Mips::SDC1;
     break;
   default:
     return false;
   }
+  if (Addr.isRegBase()) {
+    ensureOffset(Addr, isInt<16>(Addr.getOffset()));
     emitInstStore(Opc, SrcReg, Addr.getReg(), Addr.getOffset());
     return true;
   }
+  if (Addr.isFIBase()) {
+    unsigned FI = Addr.getFI();
+    unsigned Align = 4;
+    unsigned Offset = Addr.getOffset();
+    ;
+    MachineFrameInfo &MFI = *MF->getFrameInfo();
+    ;
+    MachineMemOperand *MMO = MF->getMachineMemOperand(
+        MachinePointerInfo::getFixedStack(FI), MachineMemOperand::MOLoad,
+        MFI.getObjectSize(FI), Align);
+    BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(Opc))
+        .addReg(SrcReg)
+        .addFrameIndex(FI)
+        .addImm(Offset)
+        .addMemOperand(MMO);
+    return true;
+  }
+  return false;
+}
 
 bool MipsFastISel::selectLoad(const Instruction *I) {
   // Atomic loads need special handling.
   if (cast<LoadInst>(I)->isAtomic())
     return false;
 
   // Verify we have a legal type before going any further.
   MVT VT;
   if (!isLoadTypeLegal(I->getType(), VT))
     return false;
 
   // See if we can handle this address.
   Address Addr;
-  if (!computeAddress(I->getOperand(0), Addr))
+  if (!computeAddress(I->getOperand(0), Addr, I->getType()))
     return false;
 
   unsigned ResultReg;
   if (!emitLoad(VT, ResultReg, Addr, cast<LoadInst>(I)->getAlignment()))
     return false;
   updateValueMap(I, ResultReg);
   return true;
 }
for (auto Reg : CLI.OutRegs)
     MIB.addReg(Reg, RegState::Implicit);
 
   // Add a register mask with the call-preserved registers.
   // Proper defs for return values will be added by setPhysRegsDeadExcept().
   MIB.addRegMask(TRI.getCallPreservedMask(CC));
 
   CLI.Call = MIB;
 
-  // Add implicit physical register uses to the call.
-  for (auto Reg : CLI.OutRegs)
-    MIB.addReg(Reg, RegState::Implicit);
-
-  // Add a register mask with the call-preserved registers.  Proper
-  // defs for return values will be added by setPhysRegsDeadExcept().
-  MIB.addRegMask(TRI.getCallPreservedMask(CC));
-
-  CLI.Call = MIB;
   // Finish off the call including any return values.
   return finishCall(CLI, RetVT, NumBytes);
 }
 
 bool MipsFastISel::selectRet(const Instruction *I) {
   const Function &F = *I->getParent()->getParent();
   const ReturnInst *Ret = cast<ReturnInst>(I);
 
if ((VMVT == MVT::i8) || (VMVT == MVT::i16)) {
     unsigned TempReg = createResultReg(&Mips::GPR32RegClass);
     if (!emitIntExt(VMVT, VReg, MVT::i32, TempReg, IsUnsigned))
       return 0;
     VReg = TempReg;
   }
   return VReg;
 }
 
+void MipsFastISel::ensureOffset(Address &Addr, bool FitsOffset) {
+  if (FitsOffset)
+    return;
+  unsigned TempReg =
+      materialize32BitInt(Addr.getOffset(), &Mips::GPR32RegClass);
+  unsigned DestReg = createResultReg(&Mips::GPR32RegClass);
+  emitInst(Mips::ADDu, DestReg).addReg(TempReg).addReg(Addr.getReg());
+  Addr.setReg(DestReg);
+  Addr.setOffset(0);
+  return;
+}
+
 namespace llvm {
 FastISel *Mips::createFastISel(FunctionLoweringInfo &funcInfo,
                                const TargetLibraryInfo *libInfo) {
   return new MipsFastISel(funcInfo, libInfo);
 }
 }

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

+; RUN: llc -march=mipsel -relocation-model=pic -O0 -mips-fast-isel -fast-isel-abort -mcpu=mips32r2 \
+; RUN:     < %s | FileCheck %s
+; RUN: llc -march=mipsel -relocation-model=pic -O0 -mips-fast-isel -fast-isel-abort -mcpu=mips32 \
+; RUN:     < %s | FileCheck %s
+
+@x = common global [128000 x float] zeroinitializer, align 4
+@y = global float* getelementptr inbounds ([128000 x float]* @x, i32 0, i32 0), align 4
+@result = common global float 0.000000e+00, align 4
+@.str = private unnamed_addr constant [5 x i8] c"%f \0A\00", align 1
+
+; Function Attrs: nounwind
+define void @foo() {
+entry:
+; CHECK-LABEL:   .ent  foo
+  %0 = load float** @y, align 4
+  %arrayidx = getelementptr inbounds float* %0, i32 64000
+  store float 5.500000e+00, float* %arrayidx, align 4
+; CHECK:  lui $[[REG_FPCONST_INT:[0-9]+]], 16560
+; CHECK:  mtc1  $[[REG_FPCONST_INT]], $f[[REG_FPCONST:[0-9]+]]
+; CHECK:  lw  $[[REG_Y_GOT:[0-9]+]], %got(y)(${{[0-9]+}})
+; CHECK:  lw  $[[REG_Y:[0-9]+]], 0($[[REG_Y_GOT]])
+; CHECK:  lui $[[REG_IDX_UPPER:[0-9]+]], 3
+; CHECK:  ori $[[REG_IDX:[0-9]+]], $[[REG_IDX_UPPER]], 59392
+; CHECK:  addu  $[[REG_Y_IDX:[0-9]+]], $[[REG_IDX]], $[[REG_Y]]
+; CHECK:  swc1  $f[[REG_FPCONST]], 0($[[REG_Y_IDX]])
+  ret void
+; CHECK-LABEL:   .end  foo
+}
+
+; Function Attrs: nounwind
+define void @goo() {
+entry:
+; CHECK-LABEL:   .ent  goo
+  %0 = load float** @y, align 4
+  %arrayidx = getelementptr inbounds float* %0, i32 64000
+  %1 = load float* %arrayidx, align 4
+  store float %1, float* @result, align 4
+; CHECK-DAG:  lw  $[[REG_RESULT:[0-9]+]], %got(result)(${{[0-9]+}})
+; CHECK-DAG:  lw  $[[REG_Y_GOT:[0-9]+]], %got(y)(${{[0-9]+}})
+; CHECK-DAG:  lw  $[[REG_Y:[0-9]+]], 0($[[REG_Y_GOT]])
+; CHECK-DAG:  lui $[[REG_IDX_UPPER:[0-9]+]], 3
+; CHECK-DAG:  ori $[[REG_IDX:[0-9]+]], $[[REG_IDX_UPPER]], 59392
+; CHECK-DAG:  addu  $[[REG_Y_IDX:[0-9]+]], $[[REG_IDX]], $[[REG_Y]]
+; CHECK-DAG:  lwc1  $f[[Y_IDX:[0-9]+]], 0($[[REG_Y_IDX]])
+; CHECK-DAG:  swc1  $f[[Y_IDX:[0-9]+]], 0($[[REG_RESULT]])
+; CHECK-LABEL:   .end  goo
+  ret void
+}
+