[llvm-commits] [llvm] r101626 - /llvm/trunk/lib/CodeGen/MachineLICM.cpp

[Evan Cheng]

Author: evancheng
Date: Sat Apr 17 02:07:11 2010
New Revision: 101626

URL: http://llvm.org/viewvc/llvm-project?rev=101626&view=rev
Log:
Postra machine licm must add registers defined by loop invariants to *all* of
the live-in sets of BBs in the loop. Otherwise later pass may end up using the
registers and override the invariant. rdar://7852937
No reasonablly sized test case possible.

Modified:
    llvm/trunk/lib/CodeGen/MachineLICM.cpp

Modified: llvm/trunk/lib/CodeGen/MachineLICM.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineLICM.cpp?rev=101626&r1=101625&r2=101626&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MachineLICM.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineLICM.cpp Sat Apr 17 02:07:11 2010
@@ -109,7 +109,7 @@
 
     /// HoistRegionPostRA - Walk the specified region of the CFG and hoist loop
     /// invariants out to the preheader.
-    void HoistRegionPostRA(MachineDomTreeNode *N);
+    void HoistRegionPostRA();
 
     /// HoistPostRA - When an instruction is found to only use loop invariant
     /// operands that is safe to hoist, this instruction is called to do the
@@ -122,10 +122,9 @@
                    SmallSet<int, 32> &StoredFIs,
                    SmallVector<CandidateInfo, 32> &Candidates);
 
-    /// AddToLiveIns - Add 'Reg' to the livein sets of BBs in the backedge path
-    /// from MBB to LoopHeader (inclusive).
-    void AddToLiveIns(unsigned Reg,
-                      MachineBasicBlock *MBB, MachineBasicBlock *LoopHeader);    
+    /// AddToLiveIns - Add register 'Reg' to the livein sets of BBs in the
+    /// current loop.
+    void AddToLiveIns(unsigned Reg);
 
     /// IsLICMCandidate - Returns true if the instruction may be a suitable
     /// candidate for LICM. e.g. If the instruction is a call, then it's obviously
@@ -239,12 +238,12 @@
     if (!CurPreheader)
       continue;
 
-    // CSEMap is initialized for loop header when the first instruction is
-    // being hoisted.
-    MachineDomTreeNode *N = DT->getNode(CurLoop->getHeader());
     if (!PreRegAlloc)
-      HoistRegionPostRA(N);
+      HoistRegionPostRA();
     else {
+      // CSEMap is initialized for loop header when the first instruction is
+      // being hoisted.
+      MachineDomTreeNode *N = DT->getNode(CurLoop->getHeader());
       HoistRegion(N);
       CSEMap.clear();
     }
@@ -349,9 +348,7 @@
 
 /// HoistRegionPostRA - Walk the specified region of the CFG and hoist loop
 /// invariants out to the preheader.
-void MachineLICM::HoistRegionPostRA(MachineDomTreeNode *N) {
-  assert(N != 0 && "Null dominator tree node?");
-
+void MachineLICM::HoistRegionPostRA() {
   unsigned NumRegs = TRI->getNumRegs();
   unsigned *PhysRegDefs = new unsigned[NumRegs];
   std::fill(PhysRegDefs, PhysRegDefs + NumRegs, 0);
@@ -360,15 +357,10 @@
   SmallSet<int, 32> StoredFIs;
 
   // Walk the entire region, count number of defs for each register, and
-  // return potential LICM candidates.
-  SmallVector<MachineDomTreeNode*, 8> WorkList;
-  WorkList.push_back(N);
-  do {
-    N = WorkList.pop_back_val();
-    MachineBasicBlock *BB = N->getBlock();
-
-    if (!CurLoop->contains(MLI->getLoopFor(BB)))
-      continue;
+  // collect potential LICM candidates.
+  const std::vector<MachineBasicBlock*> Blocks = CurLoop->getBlocks();
+  for (unsigned i = 0, e = Blocks.size(); i != e; ++i) {
+    MachineBasicBlock *BB = Blocks[i];
     // Conservatively treat live-in's as an external def.
     // FIXME: That means a reload that're reused in successor block(s) will not
     // be LICM'ed.
@@ -385,11 +377,7 @@
       MachineInstr *MI = &*MII;
       ProcessMI(MI, PhysRegDefs, StoredFIs, Candidates);
     }
-
-    const std::vector<MachineDomTreeNode*> &Children = N->getChildren();
-    for (unsigned I = 0, E = Children.size(); I != E; ++I)
-      WorkList.push_back(Children[I]);
-  } while (!WorkList.empty());
+  }
 
   // Now evaluate whether the potential candidates qualify.
   // 1. Check if the candidate defined register is defined by another
@@ -424,23 +412,11 @@
 }
 
 /// AddToLiveIns - Add register 'Reg' to the livein sets of BBs in the
-/// backedge path from MBB to LoopHeader.
-void MachineLICM::AddToLiveIns(unsigned Reg, MachineBasicBlock *MBB,
-                               MachineBasicBlock *LoopHeader) {
-  SmallPtrSet<MachineBasicBlock*, 4> Visited;
-  SmallVector<MachineBasicBlock*, 4> WorkList;
-  WorkList.push_back(MBB);
-  do {
-    MBB = WorkList.pop_back_val();
-    if (!Visited.insert(MBB))
-      continue;
-    MBB->addLiveIn(Reg);
-    if (MBB == LoopHeader)
-      continue;
-    for (MachineBasicBlock::pred_iterator PI = MBB->pred_begin(),
-           E = MBB->pred_end(); PI != E; ++PI)
-      WorkList.push_back(*PI);
-  } while (!WorkList.empty());
+/// current loop.
+void MachineLICM::AddToLiveIns(unsigned Reg) {
+  const std::vector<MachineBasicBlock*> Blocks = CurLoop->getBlocks();
+  for (unsigned i = 0, e = Blocks.size(); i != e; ++i)
+    Blocks[i]->addLiveIn(Reg);
 }
 
 /// HoistPostRA - When an instruction is found to only use loop invariant
@@ -464,11 +440,10 @@
   MachineBasicBlock *MBB = MI->getParent();
   CurPreheader->splice(CurPreheader->getFirstTerminator(), MBB, MI);
 
-  // Add register to livein list to BBs in the path from loop header to original
-  // BB. Note, currently it's not necessary to worry about adding it to all BB's
-  // with uses. Reload that're reused in successor block(s) are not being
-  // hoisted.
-  AddToLiveIns(Def, MBB, CurLoop->getHeader());
+  // Add register to livein list to all the BBs in the current loop since a 
+  // loop invariant must be kept live throughout the whole loop. This is
+  // important to ensure later passes do not scavenge the def register.
+  AddToLiveIns(Def);
 
   ++NumPostRAHoisted;
   Changed = true;