Make foldMemoryOperandImpl() insert instructions instead of caller.

[vkalintiris]

Summary

This patch changes the semantics of the foldMemoryOperandImpl() method in
order to insert the newly constructed instruction into the machine basic
block. We use this change in order to store zero to memory using the $zero
register on Mips.

lib/CodeGen/TargetInstrInfo.cpp

if (std::find(Ops.begin(), Ops.end(), i) != Ops.end()) {
       MIB.addImm(StackMaps::IndirectMemRefOp);
       MIB.addImm(SpillSize);
       MIB.addFrameIndex(FrameIndex);
       MIB.addImm(SpillOffset);
     }
     else
       MIB.addOperand(MO);
   }
-  return NewMI;
+
+  MachineBasicBlock &MBB = *MI->getParent();
+  return MBB.insert(MachineBasicBlock::iterator(MI), NewMI);
 }
 
 /// foldMemoryOperand - Attempt to fold a load or store of the specified stack
 /// slot into the specified machine instruction for the specified operand(s).
 /// If this is possible, a new instruction is returned with the specified
 /// operand folded, otherwise NULL is returned. The client is responsible for
 /// removing the old instruction and adding the new one in the instruction
 /// stream.
assert((!(Flags & MachineMemOperand::MOLoad) ||
            "Folded a use to a non-load!");
     const MachineFrameInfo &MFI = *MF.getFrameInfo();
     assert(MFI.getObjectOffset(FI) != -1);
     MachineMemOperand *MMO =
       MF.getMachineMemOperand(MachinePointerInfo::getFixedStack(FI),
                               Flags, MFI.getObjectSize(FI),
                               MFI.getObjectAlignment(FI));
     NewMI->addMemOperand(MF, MMO);
-
-    // FIXME: change foldMemoryOperandImpl semantics to also insert NewMI.
-    return MBB->insert(MI, NewMI);
+    return NewMI;
   }
 
   // Straight COPY may fold as load/store.
   if (!MI->isCopy() || Ops.size() != 1)
     return nullptr;
 
   const TargetRegisterClass *RC = canFoldCopy(MI, Ops[0]);
   if (!RC)
if ((MI->getOpcode() == TargetOpcode::STACKMAP ||
     NewMI = foldPatchpoint(MF, MI, Ops, FrameIndex, *this);
   } else {
     // Ask the target to do the actual folding.
     NewMI = foldMemoryOperandImpl(MF, MI, Ops, LoadMI);
   }
 
   if (!NewMI) return nullptr;
 
-  NewMI = MBB.insert(MI, NewMI);
-
   // Copy the memoperands from the load to the folded instruction.
   if (MI->memoperands_empty()) {
     NewMI->setMemRefs(LoadMI->memoperands_begin(),
                       LoadMI->memoperands_end());
   }
   else {
     // Handle the rare case of folding multiple loads.
     NewMI->setMemRefs(MI->memoperands_begin(),

lib/Target/Mips/Mips16InstrInfo.h

public:
 
   void loadRegFromStack(MachineBasicBlock &MBB,
                         MachineBasicBlock::iterator MBBI,
                         unsigned DestReg, int FrameIndex,
                         const TargetRegisterClass *RC,
                         const TargetRegisterInfo *TRI,
                         int64_t Offset) const override;
 
+  MachineInstr *foldMemoryOperandImpl(MachineFunction &MF,
+                                      MachineInstr *MI,
+                                      ArrayRef<unsigned> Ops,
+                                      int FrameIndex) const override;
+
   bool expandPostRAPseudo(MachineBasicBlock::iterator MI) const override;
 
   unsigned getOppositeBranchOpc(unsigned Opc) const override;
 
   // Adjust SP by FrameSize bytes. Save RA, S0, S1
   void makeFrame(unsigned SP, int64_t FrameSize, MachineBasicBlock &MBB,
                  MachineBasicBlock::iterator I) const;
 

lib/Target/Mips/Mips16InstrInfo.cpp

void Mips16InstrInfo::loadRegFromStack(MachineBasicBlock &MBB,
 
   if (Mips::CPU16RegsRegClass.hasSubClassEq(RC))
     Opc = Mips::LwRxSpImmX16;
   assert(Opc && "Register class not handled!");
   BuildMI(MBB, I, DL, get(Opc), DestReg).addFrameIndex(FI).addImm(Offset)
     .addMemOperand(MMO);
 }
 
+MachineInstr *Mips16InstrInfo::foldMemoryOperandImpl(MachineFunction &MF,
+                                                     MachineInstr *MI,
+                                                     ArrayRef<unsigned> Ops,
+                                                     int FrameIndex) const {
+  return nullptr;
+}
+
 bool Mips16InstrInfo::expandPostRAPseudo(MachineBasicBlock::iterator MI) const {
   MachineBasicBlock &MBB = *MI->getParent();
   switch(MI->getDesc().getOpcode()) {
   default:
     return false;
   case Mips::RetRA16:
     ExpandRetRA16(MBB, MI, Mips::JrcRa16);
     break;

lib/Target/Mips/MipsInstrInfo.h

public:
   void loadRegFromStackSlot(MachineBasicBlock &MBB,
                             MachineBasicBlock::iterator MBBI,
                             unsigned DestReg, int FrameIndex,
                             const TargetRegisterClass *RC,
                             const TargetRegisterInfo *TRI) const override {
     loadRegFromStack(MBB, MBBI, DestReg, FrameIndex, RC, TRI, 0);
   }
 
+  virtual MachineInstr *foldMemoryOperandImpl(MachineFunction &MF,
+                                              MachineInstr *MI,
+                                              ArrayRef<unsigned> Ops,
+                                              int FrameIndex) const = 0;
+
   virtual void storeRegToStack(MachineBasicBlock &MBB,
                                MachineBasicBlock::iterator MI,
                                unsigned SrcReg, bool isKill, int FrameIndex,
                                const TargetRegisterClass *RC,
                                const TargetRegisterInfo *TRI,
                                int64_t Offset) const = 0;
 
   virtual void loadRegFromStack(MachineBasicBlock &MBB,

lib/Target/Mips/MipsSEInstrInfo.h

public:
 
   void loadRegFromStack(MachineBasicBlock &MBB,
                         MachineBasicBlock::iterator MI,
                         unsigned DestReg, int FrameIndex,
                         const TargetRegisterClass *RC,
                         const TargetRegisterInfo *TRI,
                         int64_t Offset) const override;
 
+  MachineInstr *foldMemoryOperandImpl(MachineFunction &MF,
+                                      MachineInstr *MI,
+                                      ArrayRef<unsigned> Ops,
+                                      int FrameIndex) const override;
+
   bool expandPostRAPseudo(MachineBasicBlock::iterator MI) const override;
 
   unsigned getOppositeBranchOpc(unsigned Opc) const override;
 
   /// Adjust SP by Amount bytes.
   void adjustStackPtr(unsigned SP, int64_t Amount, MachineBasicBlock &MBB,
                       MachineBasicBlock::iterator I) const override;
 

lib/Target/Mips/MipsSEInstrInfo.cpp

loadRegFromStack(MachineBasicBlock &MBB, MachineBasicBlock::iterator I,
   else if (RC->hasType(MVT::v2i64) || RC->hasType(MVT::v2f64))
     Opc = Mips::LD_D;
 
   assert(Opc && "Register class not handled!");
   BuildMI(MBB, I, DL, get(Opc), DestReg).addFrameIndex(FI).addImm(Offset)
     .addMemOperand(MMO);
 }
 
+MachineInstr *MipsSEInstrInfo::foldMemoryOperandImpl(MachineFunction &MF,
+                                                     MachineInstr *MI,
+                                                     ArrayRef<unsigned> Ops,
+                                                     int FrameIndex) const {
+  unsigned Opc = MI->getOpcode();
+  if ((Opc != Mips::ADDiu) && (Opc != Mips::DADDiu))
+    return nullptr;
+
+  unsigned Reg = MI->getOperand(1).getReg();
+  if ((Reg != Mips::ZERO) && (Reg != Mips::ZERO_64))
+    return nullptr;
+
+  int64_t Imm = MI->getOperand(2).getImm();
+  if (Imm != 0)
+    return nullptr;
+
+  const MachineRegisterInfo &MRI = MF.getRegInfo();
+  const TargetRegisterClass *RC = MRI.getRegClass(MI->getOperand(0).getReg());
+
+  storeRegToStack(*MI->getParent(), MI, Reg, false, FrameIndex, RC,
+                  MRI.getTargetRegisterInfo(), 0);
+
+  return --MachineBasicBlock::iterator(MI);
+}
+
+
 bool MipsSEInstrInfo::expandPostRAPseudo(MachineBasicBlock::iterator MI) const {
   MachineBasicBlock &MBB = *MI->getParent();
   bool isMicroMips = Subtarget.inMicroMipsMode();
   unsigned Opc;
 
   switch(MI->getDesc().getOpcode()) {
   default:
     return false;

lib/Target/SystemZ/SystemZInstrInfo.cpp

SystemZInstrInfo::convertToThreeAddress(MachineFunction::iterator &MFI,
   }
   return nullptr;
 }
 
 MachineInstr *SystemZInstrInfo::foldMemoryOperandImpl(MachineFunction &MF,
                                                       MachineInstr *MI,
                                                       ArrayRef<unsigned> Ops,
                                                       int FrameIndex) const {
+  MachineBasicBlock &MBB = *MI->getParent();
   const MachineFrameInfo *MFI = MF.getFrameInfo();
   unsigned Size = MFI->getObjectSize(FrameIndex);
   unsigned Opcode = MI->getOpcode();
 
   if (Ops.size() == 2 && Ops[0] == 0 && Ops[1] == 1) {
     if ((Opcode == SystemZ::LA || Opcode == SystemZ::LAY) &&
         isInt<8>(MI->getOperand(2).getImm()) &&
         !MI->getOperand(3).getReg()) {
       // LA(Y) %reg, CONST(%reg) -> AGSI %mem, CONST
-      return BuildMI(MF, MI->getDebugLoc(), get(SystemZ::AGSI))
+      return BuildMI(MBB, MI, MI->getDebugLoc(), get(SystemZ::AGSI))
         .addFrameIndex(FrameIndex).addImm(0)
         .addImm(MI->getOperand(2).getImm());
     }
     return nullptr;
   }
 
   // All other cases require a single operand.
   if (Ops.size() != 1)
     return nullptr;
 
   unsigned OpNum = Ops[0];
   assert(Size == MF.getRegInfo()
          .getRegClass(MI->getOperand(OpNum).getReg())->getSize() &&
          "Invalid size combination");
 
   if ((Opcode == SystemZ::AHI || Opcode == SystemZ::AGHI) &&
       OpNum == 0 &&
       isInt<8>(MI->getOperand(2).getImm())) {
     // A(G)HI %reg, CONST -> A(G)SI %mem, CONST
     Opcode = (Opcode == SystemZ::AHI ? SystemZ::ASI : SystemZ::AGSI);
-    return BuildMI(MF, MI->getDebugLoc(), get(Opcode))
+    return BuildMI(MBB, MI, MI->getDebugLoc(), get(Opcode))
       .addFrameIndex(FrameIndex).addImm(0)
       .addImm(MI->getOperand(2).getImm());
   }
 
   if (Opcode == SystemZ::LGDR || Opcode == SystemZ::LDGR) {
     bool Op0IsGPR = (Opcode == SystemZ::LGDR);
     bool Op1IsGPR = (Opcode == SystemZ::LDGR);
     // If we're spilling the destination of an LDGR or LGDR, store the
     // source register instead.
     if (OpNum == 0) {
       unsigned StoreOpcode = Op1IsGPR ? SystemZ::STG : SystemZ::STD;
-      return BuildMI(MF, MI->getDebugLoc(), get(StoreOpcode))
+      return BuildMI(MBB, MI, MI->getDebugLoc(), get(StoreOpcode))
         .addOperand(MI->getOperand(1)).addFrameIndex(FrameIndex)
         .addImm(0).addReg(0);
     }
     // If we're spilling the source of an LDGR or LGDR, load the
     // destination register instead.
     if (OpNum == 1) {
       unsigned LoadOpcode = Op0IsGPR ? SystemZ::LG : SystemZ::LD;
       unsigned Dest = MI->getOperand(0).getReg();
-      return BuildMI(MF, MI->getDebugLoc(), get(LoadOpcode), Dest)
+      return BuildMI(MBB, MI, MI->getDebugLoc(), get(LoadOpcode), Dest)
         .addFrameIndex(FrameIndex).addImm(0).addReg(0);
     }
   }
 
   // Look for cases where the source of a simple store or the destination
   // of a simple load is being spilled.  Try to use MVC instead.
   //
   // Although MVC is in practice a fast choice in these cases, it is still
   // logically a bytewise copy.  This means that we cannot use it if the
   // load or store is volatile.  We also wouldn't be able to use MVC if
   // the two memories partially overlap, but that case cannot occur here,
   // because we know that one of the memories is a full frame index.
   //
   // For performance reasons, we also want to avoid using MVC if the addresses
   // might be equal.  We don't worry about that case here, because spill slot
   // coloring happens later, and because we have special code to remove
   // MVCs that turn out to be redundant.
   if (OpNum == 0 && MI->hasOneMemOperand()) {
     MachineMemOperand *MMO = *MI->memoperands_begin();
     if (MMO->getSize() == Size && !MMO->isVolatile()) {
       // Handle conversion of loads.
       if (isSimpleBD12Move(MI, SystemZII::SimpleBDXLoad)) {
-        return BuildMI(MF, MI->getDebugLoc(), get(SystemZ::MVC))
+        return BuildMI(MBB, MI, MI->getDebugLoc(), get(SystemZ::MVC))
           .addFrameIndex(FrameIndex).addImm(0).addImm(Size)
           .addOperand(MI->getOperand(1)).addImm(MI->getOperand(2).getImm())
           .addMemOperand(MMO);
       }
       // Handle conversion of stores.
       if (isSimpleBD12Move(MI, SystemZII::SimpleBDXStore)) {
-        return BuildMI(MF, MI->getDebugLoc(), get(SystemZ::MVC))
+        return BuildMI(MBB, MI, MI->getDebugLoc(), get(SystemZ::MVC))
           .addOperand(MI->getOperand(1)).addImm(MI->getOperand(2).getImm())
           .addImm(Size).addFrameIndex(FrameIndex).addImm(0)
           .addMemOperand(MMO);
       }
     }
   }
 
   // If the spilled operand is the final one, try to change <INSN>R
   // into <INSN>.
   int MemOpcode = SystemZ::getMemOpcode(Opcode);
   if (MemOpcode >= 0) {
     unsigned NumOps = MI->getNumExplicitOperands();
     if (OpNum == NumOps - 1) {
       const MCInstrDesc &MemDesc = get(MemOpcode);
       uint64_t AccessBytes = SystemZII::getAccessSize(MemDesc.TSFlags);
       assert(AccessBytes != 0 && "Size of access should be known");
       assert(AccessBytes <= Size && "Access outside the frame index");
       uint64_t Offset = Size - AccessBytes;
-      MachineInstrBuilder MIB = BuildMI(MF, MI->getDebugLoc(), get(MemOpcode));
+      MachineInstrBuilder MIB = BuildMI(MBB, MI, MI->getDebugLoc(),
+                                        get(MemOpcode));
       for (unsigned I = 0; I < OpNum; ++I)
         MIB.addOperand(MI->getOperand(I));
       MIB.addFrameIndex(FrameIndex).addImm(Offset);
       if (MemDesc.TSFlags & SystemZII::HasIndex)
         MIB.addReg(0);
       return MIB;
     }
   }

lib/Target/X86/X86FastISel.cpp

bool X86FastISel::tryToFoldLoadIntoMI(MachineInstr *MI, unsigned OpNo,
 
   MachineInstr *Result =
     XII.foldMemoryOperandImpl(*FuncInfo.MF, MI, OpNo, AddrOps,
                               Size, Alignment, /*AllowCommute=*/true);
   if (!Result)
     return false;
 
   Result->addMemOperand(*FuncInfo.MF, createMachineMemOperandFor(LI));
-  FuncInfo.MBB->insert(FuncInfo.InsertPt, Result);
   MI->eraseFromParent();
   return true;
 }
 
 
 namespace llvm {
   FastISel *X86::createFastISel(FunctionLoweringInfo &funcInfo,
                                 const TargetLibraryInfo *libInfo) {
     return new X86FastISel(funcInfo, libInfo);
   }
 }

lib/Target/X86/X86InstrInfo.cpp

 }
 
 MachineInstr *X86InstrInfo::foldMemoryOperandImpl(MachineFunction &MF,
                                                   MachineInstr *MI,
                                                   unsigned OpNum,
                                                   ArrayRef<MachineOperand> MOs,
                                                   unsigned Size, unsigned Align,
                                                   bool AllowCommute) const {
+  MachineBasicBlock &MBB = *MI->getParent();
   const DenseMap<unsigned,
                  std::pair<unsigned,unsigned> > *OpcodeTablePtr = nullptr;
   bool isCallRegIndirect = Subtarget.callRegIndirect();
   bool isTwoAddrFold = false;
 
   // For CPUs that favor the register form of a call,
   // do not fold loads into calls.
   if (isCallRegIndirect &&
if (isTwoAddr && NumOps >= 2 && OpNum < 2 &&
       MI->getOperand(1).isReg() &&
       MI->getOperand(0).getReg() == MI->getOperand(1).getReg()) {
     OpcodeTablePtr = &RegOp2MemOpTable2Addr;
     isTwoAddrFold = true;
   } else if (OpNum == 0) {
     if (MI->getOpcode() == X86::MOV32r0) {
       NewMI = MakeM0Inst(*this, X86::MOV32mi, MOs, MI);
       if (NewMI)
-        return NewMI;
+        return MBB.insert(MachineBasicBlock::iterator(MI), NewMI);
     }
 
     OpcodeTablePtr = &RegOp2MemOpTable0;
   } else if (OpNum == 1) {
     OpcodeTablePtr = &RegOp2MemOpTable1;
   } else if (OpNum == 2) {
     OpcodeTablePtr = &RegOp2MemOpTable2;
   } else if (OpNum == 3) {
if (I != OpcodeTablePtr->end()) {
         // value and zero-extend the top bits. Change the destination register
         // to a 32-bit one.
         unsigned DstReg = NewMI->getOperand(0).getReg();
         if (TargetRegisterInfo::isPhysicalRegister(DstReg))
           NewMI->getOperand(0).setReg(RI.getSubReg(DstReg, X86::sub_32bit));
         else
           NewMI->getOperand(0).setSubReg(X86::sub_32bit);
       }
-      return NewMI;
+
+      return MBB.insert(MachineBasicBlock::iterator(MI), NewMI);
     }
   }
 
   // If the instruction and target operand are commutable, commute the
   // instruction and try again.
   if (AllowCommute) {
     unsigned OriginalOpIdx = OpNum, CommuteOpIdx1, CommuteOpIdx2;
     if (findCommutedOpIndices(MI, CommuteOpIdx1, CommuteOpIdx2)) {

test/CodeGen/Mips/fold-zero-copy.ll

+; RUN: llc < %s -march=mips -mcpu=mips2 | FileCheck %s
+; RUN: llc < %s -march=mips -mcpu=mips32 | FileCheck %s
+; RUN: llc < %s -march=mips -mcpu=mips32r2 | FileCheck %s
+; RUN: llc < %s -march=mips -mcpu=mips32r6 | FileCheck %s
+
+; NOTE: This test was generated with csmith and reduced with bugpoint.
+; We don't test for 64-bit targets because that requires performing the same
+; time consuming procedure (csmith + bugpoint) again. Despite that, the code
+; path for the checks of this test is the same for both 32-bit and 64-bit
+; targets, so we are essentially checking both targets with just one test.
+
+@g_51 = external global [8 x [9 x [3 x i32]]], align 4
+@g_295 = external global i8, align 4
+@g_604 = external unnamed_addr global [6 x i16], align 2
+@g_1754 = external unnamed_addr global i64, align 8
+@g_1790 = external unnamed_addr global i32, align 4
+@g_721 = external global i32**, align 4
+@g_1478 = external global [7 x [2 x [5 x i8*]]], align 4
+
+declare fastcc void @foo()
+
+define void @func_1() {
+; CHECK-LABEL: func_1:
+
+  ; CHECK:      sw        $zero, {{[0-9]+}}($sp)
+  ; CHECK-NOT:  addiu     $[[T0:[0-9]+]], $zero, 0
+  ; CHECK-NOT:  sw        $[[T0]], {{[0-9]+}}($sp)
+
+for.cond.preheader:
+  br label %for.body
+
+for.body:
+  br i1 undef, label %for.end, label %for.body
+
+for.end:
+  %0 = phi i64 [ 1, %for.end ], [ 3, %for.body ]
+  %1 = load volatile i8*, i8** getelementptr inbounds ([7 x [2 x [5 x i8*]]], [7 x [2 x [5 x i8*]]]* @g_1478, i32 0, i32 3, i32 0, i32 4), align 4
+  store i64 0, i64* @g_1754, align 8
+  %cmp475 = icmp slt i64 %0, 0
+  %conv484 = zext i1 %cmp475 to i32
+  %2 = load i16, i16* getelementptr inbounds ([6 x i16], [6 x i16]* @g_604, i32 0, i32 5), align 2
+  %conv4855 = zext i16 %2 to i32
+  %and = and i32 %conv484, %conv4855
+  %conv486 = trunc i32 %and to i16
+  store i16 %conv486, i16* getelementptr inbounds ([6 x i16], [6 x i16]* @g_604, i32 0, i32 5), align 2
+  store i8 0, i8* @g_295, align 4
+  tail call fastcc void @foo()
+  store i32 1, i32* @g_1790, align 4
+  %3 = load volatile i32**, i32*** @g_721, align 4
+  store i32 1, i32* getelementptr inbounds ([8 x [9 x [3 x i32]]], [8 x [9 x [3 x i32]]]* @g_51, i32 0, i32 0, i32 5, i32 1), align 4
+  br label %for.end
+}