[llvm-commits] [llvm] r144481 - in /llvm/trunk: lib/CodeGen/StackSlotColoring.cpp test/CodeGen/X86/2009-07-17-StackColoringBug.ll

Jakob Stoklund Olesen stoklund at 2pi.dk
Sat Nov 12 16:31:23 PST 2011


Author: stoklund
Date: Sat Nov 12 18:31:23 2011
New Revision: 144481

URL: http://llvm.org/viewvc/llvm-project?rev=144481&view=rev
Log:
Remove the -color-ss-with-regs option.

It was off by default.

The new register allocators don't have the problems that made it
necessary to reallocate registers during stack slot coloring.

Removed:
    llvm/trunk/test/CodeGen/X86/2009-07-17-StackColoringBug.ll
Modified:
    llvm/trunk/lib/CodeGen/StackSlotColoring.cpp

Modified: llvm/trunk/lib/CodeGen/StackSlotColoring.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/StackSlotColoring.cpp?rev=144481&r1=144480&r2=144481&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/StackSlotColoring.cpp (original)
+++ llvm/trunk/lib/CodeGen/StackSlotColoring.cpp Sat Nov 12 18:31:23 2011
@@ -40,18 +40,9 @@
              cl::init(false), cl::Hidden,
              cl::desc("Suppress slot sharing during stack coloring"));
 
-static cl::opt<bool>
-ColorWithRegsOpt("color-ss-with-regs",
-                 cl::init(false), cl::Hidden,
-                 cl::desc("Color stack slots with free registers"));
-
-
 static cl::opt<int> DCELimit("ssc-dce-limit", cl::init(-1), cl::Hidden);
 
 STATISTIC(NumEliminated, "Number of stack slots eliminated due to coloring");
-STATISTIC(NumRegRepl,    "Number of stack slot refs replaced with reg refs");
-STATISTIC(NumLoadElim,   "Number of loads eliminated");
-STATISTIC(NumStoreElim,  "Number of stores eliminated");
 STATISTIC(NumDead,       "Number of trivially dead stack accesses eliminated");
 
 namespace {
@@ -127,22 +118,8 @@
     bool OverlapWithAssignments(LiveInterval *li, int Color) const;
     int ColorSlot(LiveInterval *li);
     bool ColorSlots(MachineFunction &MF);
-    bool ColorSlotsWithFreeRegs(SmallVector<int, 16> &SlotMapping,
-                                SmallVector<SmallVector<int, 4>, 16> &RevMap,
-                                BitVector &SlotIsReg);
     void RewriteInstruction(MachineInstr *MI, int OldFI, int NewFI,
                             MachineFunction &MF);
-    bool PropagateBackward(MachineBasicBlock::iterator MII,
-                           MachineBasicBlock *MBB,
-                           unsigned OldReg, unsigned NewReg);
-    bool PropagateForward(MachineBasicBlock::iterator MII,
-                          MachineBasicBlock *MBB,
-                          unsigned OldReg, unsigned NewReg);
-    void UnfoldAndRewriteInstruction(MachineInstr *MI, int OldFI,
-                                    unsigned Reg, const TargetRegisterClass *RC,
-                                    SmallSet<unsigned, 4> &Defs,
-                                    MachineFunction &MF);
-    bool AllMemRefsCanBeUnfolded(int SS);
     bool RemoveDeadStores(MachineBasicBlock* MBB);
   };
 } // end anonymous namespace
@@ -248,79 +225,6 @@
   return false;
 }
 
-/// ColorSlotsWithFreeRegs - If there are any free registers available, try
-/// replacing spill slots references with registers instead.
-bool
-StackSlotColoring::ColorSlotsWithFreeRegs(SmallVector<int, 16> &SlotMapping,
-                                   SmallVector<SmallVector<int, 4>, 16> &RevMap,
-                                   BitVector &SlotIsReg) {
-  if (!(ColorWithRegs || ColorWithRegsOpt) || !VRM->HasUnusedRegisters())
-    return false;
-
-  bool Changed = false;
-  DEBUG(dbgs() << "Assigning unused registers to spill slots:\n");
-  for (unsigned i = 0, e = SSIntervals.size(); i != e; ++i) {
-    LiveInterval *li = SSIntervals[i];
-    int SS = TargetRegisterInfo::stackSlot2Index(li->reg);
-    if (!UsedColors[SS] || li->weight < 20)
-      // If the weight is < 20, i.e. two references in a loop with depth 1,
-      // don't bother with it.
-      continue;
-
-    // These slots allow to share the same registers.
-    bool AllColored = true;
-    SmallVector<unsigned, 4> ColoredRegs;
-    for (unsigned j = 0, ee = RevMap[SS].size(); j != ee; ++j) {
-      int RSS = RevMap[SS][j];
-      const TargetRegisterClass *RC = LS->getIntervalRegClass(RSS);
-      // If it's not colored to another stack slot, try coloring it
-      // to a "free" register.
-      if (!RC) {
-        AllColored = false;
-        continue;
-      }
-      unsigned Reg = VRM->getFirstUnusedRegister(RC);
-      if (!Reg) {
-        AllColored = false;
-        continue;
-      }
-      if (!AllMemRefsCanBeUnfolded(RSS)) {
-        AllColored = false;
-        continue;
-      } else {
-        DEBUG(dbgs() << "Assigning fi#" << RSS << " to "
-                     << TRI->getName(Reg) << '\n');
-        ColoredRegs.push_back(Reg);
-        SlotMapping[RSS] = Reg;
-        SlotIsReg.set(RSS);
-        Changed = true;
-      }
-    }
-
-    // Register and its sub-registers are no longer free.
-    while (!ColoredRegs.empty()) {
-      unsigned Reg = ColoredRegs.back();
-      ColoredRegs.pop_back();
-      VRM->setRegisterUsed(Reg);
-      // If reg is a callee-saved register, it will have to be spilled in
-      // the prologue.
-      MRI->setPhysRegUsed(Reg);
-      for (const unsigned *AS = TRI->getAliasSet(Reg); *AS; ++AS) {
-        VRM->setRegisterUsed(*AS);
-        MRI->setPhysRegUsed(*AS);
-      }
-    }
-    // This spill slot is dead after the rewrites
-    if (AllColored) {
-      MFI->RemoveStackObject(SS);
-      ++NumEliminated;
-    }
-  }
-  DEBUG(dbgs() << '\n');
-
-  return Changed;
-}
-
 /// ColorSlot - Assign a "color" (stack slot) to the specified stack slot.
 ///
 int StackSlotColoring::ColorSlot(LiveInterval *li) {
@@ -372,7 +276,6 @@
   SmallVector<int, 16> SlotMapping(NumObjs, -1);
   SmallVector<float, 16> SlotWeights(NumObjs, 0.0);
   SmallVector<SmallVector<int, 4>, 16> RevMap(NumObjs);
-  BitVector SlotIsReg(NumObjs);
   BitVector UsedColors(NumObjs);
 
   DEBUG(dbgs() << "Color spill slot intervals:\n");
@@ -404,31 +307,19 @@
   DEBUG(dbgs() << '\n');
 #endif
 
-  // Can we "color" a stack slot with a unused register?
-  Changed |= ColorSlotsWithFreeRegs(SlotMapping, RevMap, SlotIsReg);
-
   if (!Changed)
     return false;
 
   // Rewrite all MO_FrameIndex operands.
   SmallVector<SmallSet<unsigned, 4>, 4> NewDefs(MF.getNumBlockIDs());
   for (unsigned SS = 0, SE = SSRefs.size(); SS != SE; ++SS) {
-    bool isReg = SlotIsReg[SS];
     int NewFI = SlotMapping[SS];
-    if (NewFI == -1 || (NewFI == (int)SS && !isReg))
+    if (NewFI == -1 || (NewFI == (int)SS))
       continue;
 
-    const TargetRegisterClass *RC = LS->getIntervalRegClass(SS);
     SmallVector<MachineInstr*, 8> &RefMIs = SSRefs[SS];
     for (unsigned i = 0, e = RefMIs.size(); i != e; ++i)
-      if (!isReg)
-        RewriteInstruction(RefMIs[i], SS, NewFI, MF);
-      else {
-        // Rewrite to use a register instead.
-        unsigned MBBId = RefMIs[i]->getParent()->getNumber();
-        SmallSet<unsigned, 4> &Defs = NewDefs[MBBId];
-        UnfoldAndRewriteInstruction(RefMIs[i], SS, NewFI, RC, Defs, MF);
-      }
+      RewriteInstruction(RefMIs[i], SS, NewFI, MF);
   }
 
   // Delete unused stack slots.
@@ -441,28 +332,6 @@
   return true;
 }
 
-/// AllMemRefsCanBeUnfolded - Return true if all references of the specified
-/// spill slot index can be unfolded.
-bool StackSlotColoring::AllMemRefsCanBeUnfolded(int SS) {
-  SmallVector<MachineInstr*, 8> &RefMIs = SSRefs[SS];
-  for (unsigned i = 0, e = RefMIs.size(); i != e; ++i) {
-    MachineInstr *MI = RefMIs[i];
-    if (TII->isLoadFromStackSlot(MI, SS) ||
-        TII->isStoreToStackSlot(MI, SS))
-      // Restore and spill will become copies.
-      return true;
-    if (!TII->getOpcodeAfterMemoryUnfold(MI->getOpcode(), false, false))
-      return false;
-    for (unsigned j = 0, ee = MI->getNumOperands(); j != ee; ++j) {
-      MachineOperand &MO = MI->getOperand(j);
-      if (MO.isFI() && MO.getIndex() != SS)
-        // If it uses another frameindex, we can, currently* unfold it.
-        return false;
-    }
-  }
-  return true;
-}
-
 /// RewriteInstruction - Rewrite specified instruction by replacing references
 /// to old frame index with new one.
 void StackSlotColoring::RewriteInstruction(MachineInstr *MI, int OldFI,
@@ -489,179 +358,6 @@
       (*I)->setValue(NewSV);
 }
 
-/// PropagateBackward - Traverse backward and look for the definition of
-/// OldReg. If it can successfully update all of the references with NewReg,
-/// do so and return true.
-bool StackSlotColoring::PropagateBackward(MachineBasicBlock::iterator MII,
-                                          MachineBasicBlock *MBB,
-                                          unsigned OldReg, unsigned NewReg) {
-  if (MII == MBB->begin())
-    return false;
-
-  SmallVector<MachineOperand*, 4> Uses;
-  SmallVector<MachineOperand*, 4> Refs;
-  while (--MII != MBB->begin()) {
-    bool FoundDef = false;  // Not counting 2address def.
-
-    Uses.clear();
-    const MCInstrDesc &MCID = MII->getDesc();
-    for (unsigned i = 0, e = MII->getNumOperands(); i != e; ++i) {
-      MachineOperand &MO = MII->getOperand(i);
-      if (!MO.isReg())
-        continue;
-      unsigned Reg = MO.getReg();
-      if (Reg == 0)
-        continue;
-      if (Reg == OldReg) {
-        if (MO.isImplicit())
-          return false;
-
-        // Abort the use is actually a sub-register def. We don't have enough
-        // information to figure out if it is really legal.
-        if (MO.getSubReg() || MII->isSubregToReg())
-          return false;
-
-        const TargetRegisterClass *RC = TII->getRegClass(MCID, i, TRI);
-        if (RC && !RC->contains(NewReg))
-          return false;
-
-        if (MO.isUse()) {
-          Uses.push_back(&MO);
-        } else {
-          Refs.push_back(&MO);
-          if (!MII->isRegTiedToUseOperand(i))
-            FoundDef = true;
-        }
-      } else if (TRI->regsOverlap(Reg, NewReg)) {
-        return false;
-      } else if (TRI->regsOverlap(Reg, OldReg)) {
-        if (!MO.isUse() || !MO.isKill())
-          return false;
-      }
-    }
-
-    if (FoundDef) {
-      // Found non-two-address def. Stop here.
-      for (unsigned i = 0, e = Refs.size(); i != e; ++i)
-        Refs[i]->setReg(NewReg);
-      return true;
-    }
-
-    // Two-address uses must be updated as well.
-    for (unsigned i = 0, e = Uses.size(); i != e; ++i)
-      Refs.push_back(Uses[i]);
-  }
-  return false;
-}
-
-/// PropagateForward - Traverse forward and look for the kill of OldReg. If
-/// it can successfully update all of the uses with NewReg, do so and
-/// return true.
-bool StackSlotColoring::PropagateForward(MachineBasicBlock::iterator MII,
-                                         MachineBasicBlock *MBB,
-                                         unsigned OldReg, unsigned NewReg) {
-  if (MII == MBB->end())
-    return false;
-
-  SmallVector<MachineOperand*, 4> Uses;
-  while (++MII != MBB->end()) {
-    bool FoundKill = false;
-    const MCInstrDesc &MCID = MII->getDesc();
-    for (unsigned i = 0, e = MII->getNumOperands(); i != e; ++i) {
-      MachineOperand &MO = MII->getOperand(i);
-      if (!MO.isReg())
-        continue;
-      unsigned Reg = MO.getReg();
-      if (Reg == 0)
-        continue;
-      if (Reg == OldReg) {
-        if (MO.isDef() || MO.isImplicit())
-          return false;
-
-        // Abort the use is actually a sub-register use. We don't have enough
-        // information to figure out if it is really legal.
-        if (MO.getSubReg())
-          return false;
-
-        const TargetRegisterClass *RC = TII->getRegClass(MCID, i, TRI);
-        if (RC && !RC->contains(NewReg))
-          return false;
-        if (MO.isKill())
-          FoundKill = true;
-
-        Uses.push_back(&MO);
-      } else if (TRI->regsOverlap(Reg, NewReg) ||
-                 TRI->regsOverlap(Reg, OldReg))
-        return false;
-    }
-    if (FoundKill) {
-      for (unsigned i = 0, e = Uses.size(); i != e; ++i)
-        Uses[i]->setReg(NewReg);
-      return true;
-    }
-  }
-  return false;
-}
-
-/// UnfoldAndRewriteInstruction - Rewrite specified instruction by unfolding
-/// folded memory references and replacing those references with register
-/// references instead.
-void
-StackSlotColoring::UnfoldAndRewriteInstruction(MachineInstr *MI, int OldFI,
-                                               unsigned Reg,
-                                               const TargetRegisterClass *RC,
-                                               SmallSet<unsigned, 4> &Defs,
-                                               MachineFunction &MF) {
-  MachineBasicBlock *MBB = MI->getParent();
-  if (unsigned DstReg = TII->isLoadFromStackSlot(MI, OldFI)) {
-    if (PropagateForward(MI, MBB, DstReg, Reg)) {
-      DEBUG(dbgs() << "Eliminated load: ");
-      DEBUG(MI->dump());
-      ++NumLoadElim;
-    } else {
-      BuildMI(*MBB, MI, MI->getDebugLoc(), TII->get(TargetOpcode::COPY),
-              DstReg).addReg(Reg);
-      ++NumRegRepl;
-    }
-
-    if (!Defs.count(Reg)) {
-      // If this is the first use of Reg in this MBB and it wasn't previously
-      // defined in MBB, add it to livein.
-      MBB->addLiveIn(Reg);
-      Defs.insert(Reg);
-    }
-  } else if (unsigned SrcReg = TII->isStoreToStackSlot(MI, OldFI)) {
-    if (MI->killsRegister(SrcReg) && PropagateBackward(MI, MBB, SrcReg, Reg)) {
-      DEBUG(dbgs() << "Eliminated store: ");
-      DEBUG(MI->dump());
-      ++NumStoreElim;
-    } else {
-      BuildMI(*MBB, MI, MI->getDebugLoc(), TII->get(TargetOpcode::COPY), Reg)
-        .addReg(SrcReg);
-      ++NumRegRepl;
-    }
-
-    // Remember reg has been defined in MBB.
-    Defs.insert(Reg);
-  } else {
-    SmallVector<MachineInstr*, 4> NewMIs;
-    bool Success = TII->unfoldMemoryOperand(MF, MI, Reg, false, false, NewMIs);
-    (void)Success; // Silence compiler warning.
-    assert(Success && "Failed to unfold!");
-    MachineInstr *NewMI = NewMIs[0];
-    MBB->insert(MI, NewMI);
-    ++NumRegRepl;
-
-    if (NewMI->readsRegister(Reg)) {
-      if (!Defs.count(Reg))
-        // If this is the first use of Reg in this MBB and it wasn't previously
-        // defined in MBB, add it to livein.
-        MBB->addLiveIn(Reg);
-      Defs.insert(Reg);
-    }
-  }
-  MBB->erase(MI);
-}
 
 /// RemoveDeadStores - Scan through a basic block and look for loads followed
 /// by stores.  If they're both using the same stack slot, then the store is

Removed: llvm/trunk/test/CodeGen/X86/2009-07-17-StackColoringBug.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/2009-07-17-StackColoringBug.ll?rev=144480&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/2009-07-17-StackColoringBug.ll (original)
+++ llvm/trunk/test/CodeGen/X86/2009-07-17-StackColoringBug.ll (removed)
@@ -1,55 +0,0 @@
-; RUN: llc < %s -mtriple=i386-pc-linux-gnu -disable-fp-elim -color-ss-with-regs | not grep dil
-; PR4552
-
-target triple = "i386-pc-linux-gnu"
- at g_8 = internal global i32 0		; <i32*> [#uses=1]
- at g_72 = internal global i32 0		; <i32*> [#uses=1]
- at llvm.used = appending global [1 x i8*] [i8* bitcast (i32 (i32, i8, i8)* @uint84 to i8*)], section "llvm.metadata"		; <[1 x i8*]*> [#uses=0]
-
-define i32 @uint84(i32 %p_15, i8 signext %p_17, i8 signext %p_19) nounwind {
-entry:
-	%g_72.promoted = load i32* @g_72		; <i32> [#uses=1]
-	%g_8.promoted = load i32* @g_8		; <i32> [#uses=1]
-	br label %bb
-
-bb:		; preds = %func_40.exit, %entry
-	%g_8.tmp.1 = phi i32 [ %g_8.promoted, %entry ], [ %g_8.tmp.0, %func_40.exit ]		; <i32> [#uses=3]
-	%g_72.tmp.1 = phi i32 [ %g_72.promoted, %entry ], [ %g_72.tmp.0, %func_40.exit ]		; <i32> [#uses=3]
-	%retval12.i4.i.i = trunc i32 %g_8.tmp.1 to i8		; <i8> [#uses=2]
-	%0 = trunc i32 %g_72.tmp.1 to i8		; <i8> [#uses=2]
-	%1 = mul i8 %retval12.i4.i.i, %0		; <i8> [#uses=1]
-	%2 = icmp eq i8 %1, 0		; <i1> [#uses=1]
-	br i1 %2, label %bb2.i.i, label %bb.i.i
-
-bb.i.i:		; preds = %bb
-	%3 = sext i8 %0 to i32		; <i32> [#uses=1]
-	%4 = and i32 %3, 50295		; <i32> [#uses=1]
-	%5 = icmp eq i32 %4, 0		; <i1> [#uses=1]
-	br i1 %5, label %bb2.i.i, label %func_55.exit.i
-
-bb2.i.i:		; preds = %bb.i.i, %bb
-	br label %func_55.exit.i
-
-func_55.exit.i:		; preds = %bb2.i.i, %bb.i.i
-	%g_72.tmp.2 = phi i32 [ 1, %bb2.i.i ], [ %g_72.tmp.1, %bb.i.i ]		; <i32> [#uses=1]
-	%6 = phi i32 [ 1, %bb2.i.i ], [ %g_72.tmp.1, %bb.i.i ]		; <i32> [#uses=1]
-	%7 = trunc i32 %6 to i8		; <i8> [#uses=2]
-	%8 = mul i8 %7, %retval12.i4.i.i		; <i8> [#uses=1]
-	%9 = icmp eq i8 %8, 0		; <i1> [#uses=1]
-	br i1 %9, label %bb2.i4.i, label %bb.i3.i
-
-bb.i3.i:		; preds = %func_55.exit.i
-	%10 = sext i8 %7 to i32		; <i32> [#uses=1]
-	%11 = and i32 %10, 50295		; <i32> [#uses=1]
-	%12 = icmp eq i32 %11, 0		; <i1> [#uses=1]
-	br i1 %12, label %bb2.i4.i, label %func_40.exit
-
-bb2.i4.i:		; preds = %bb.i3.i, %func_55.exit.i
-	br label %func_40.exit
-
-func_40.exit:		; preds = %bb2.i4.i, %bb.i3.i
-	%g_72.tmp.0 = phi i32 [ 1, %bb2.i4.i ], [ %g_72.tmp.2, %bb.i3.i ]		; <i32> [#uses=1]
-	%phitmp = icmp sgt i32 %g_8.tmp.1, 0		; <i1> [#uses=1]
-	%g_8.tmp.0 = select i1 %phitmp, i32 %g_8.tmp.1, i32 1		; <i32> [#uses=1]
-	br label %bb
-}





More information about the llvm-commits mailing list