[llvm-commits] [llvm] r78650 - in /llvm/trunk: include/llvm/CodeGen/RegisterScavenging.h lib/CodeGen/RegisterScavenging.cpp test/CodeGen/Blackfin/2009-08-11-RegScavenger-CSR.ll

Jakob Stoklund Olesen stoklund at 2pi.dk
Mon Aug 10 23:25:12 PDT 2009


Author: stoklund
Date: Tue Aug 11 01:25:12 2009
New Revision: 78650

URL: http://llvm.org/viewvc/llvm-project?rev=78650&view=rev
Log:
Rebuild RegScavenger::DistanceMap each time it is needed.

The register scavenger maintains a DistanceMap that maps MI pointers to their
distance from the top of the current MBB. The DistanceMap is built
incrementally in forward() and in bulk in findFirstUse(). It is used by
scavengeRegister() to determine which candidate register has the longest
unused interval.

Unfortunately the DistanceMap contents can become outdated. The first time
scavengeRegister() is called, the DistanceMap is filled to cover the MBB. If
then instructions are inserted in the MBB (as they always are following
scavengeRegister()), the recorded distances are too short. This causes bad
behaviour in the included test case where a register use /after/ the current
position is ignored because findFirstUse() thinks is is /before/ the current
position. A "using an undefined register" assertion follows promptly.

The fix is to build a fresh DistanceMap at the top of scavengeRegister(), and
discard it after use. This means that DistanceMap is no longer needed as a
RegScavenger member variable, and forward() doesn't need to update it.

The fix then discloses issue number two in the same test case: The candidate
search in scavengeRegister() finds a CSR that has been saved in the prologue,
but is currently unused. It would be both inefficient and wrong to spill such
a register in the emergency spill slot. In the present case, the emergency
slot restore is placed immediately before the normal epilogue restore, leading
to a "Redefining a live register" assertion.

Fix number two: When scavengerRegister() stumbles upon an unused register that
is overwritten later in the MBB, return that register early. It is important
to verify that the register is defined later in the MBB, otherwise it might be
an unspilled CSR.

Added:
    llvm/trunk/test/CodeGen/Blackfin/2009-08-11-RegScavenger-CSR.ll
Modified:
    llvm/trunk/include/llvm/CodeGen/RegisterScavenging.h
    llvm/trunk/lib/CodeGen/RegisterScavenging.cpp

Modified: llvm/trunk/include/llvm/CodeGen/RegisterScavenging.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/RegisterScavenging.h?rev=78650&r1=78649&r2=78650&view=diff

==============================================================================
--- llvm/trunk/include/llvm/CodeGen/RegisterScavenging.h (original)
+++ llvm/trunk/include/llvm/CodeGen/RegisterScavenging.h Tue Aug 11 01:25:12 2009
@@ -19,7 +19,6 @@
 
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/ADT/BitVector.h"
-#include "llvm/ADT/DenseMap.h"
 
 namespace llvm {
 
@@ -69,14 +68,6 @@
   /// available, unset means the register is currently being used.
   BitVector RegsAvailable;
 
-  /// CurrDist - Distance from MBB entry to the current instruction MBBI.
-  ///
-  unsigned CurrDist;
-
-  /// DistanceMap - Keep track the distance of a MI from the start of the
-  /// current basic block.
-  DenseMap<MachineInstr*, unsigned> DistanceMap;
-
 public:
   RegScavenger()
     : MBB(NULL), NumPhysRegs(0), Tracking(false),
@@ -112,6 +103,9 @@
   bool isUsed(unsigned Reg) const   { return !RegsAvailable[Reg]; }
   bool isUnused(unsigned Reg) const { return RegsAvailable[Reg]; }
 
+  /// isAliasUsed - Is Reg or an alias currently in use?
+  bool isAliasUsed(unsigned Reg) const;
+
   /// getRegsUsed - return all registers currently in use in used.
   void getRegsUsed(BitVector &used, bool includeReserved);
 
@@ -156,10 +150,6 @@
   /// emergency spill slot. Mark it used.
   void restoreScavengedReg();
 
-  MachineInstr *findFirstUse(MachineBasicBlock *MBB,
-                             MachineBasicBlock::iterator I, unsigned Reg,
-                             unsigned &Dist);
-
   /// Add Reg and all its sub-registers to BV.
   void addRegWithSubRegs(BitVector &BV, unsigned Reg);
 

Modified: llvm/trunk/lib/CodeGen/RegisterScavenging.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegisterScavenging.cpp?rev=78650&r1=78649&r2=78650&view=diff

==============================================================================
--- llvm/trunk/lib/CodeGen/RegisterScavenging.cpp (original)
+++ llvm/trunk/lib/CodeGen/RegisterScavenging.cpp Tue Aug 11 01:25:12 2009
@@ -25,6 +25,7 @@
 #include "llvm/Target/TargetRegisterInfo.h"
 #include "llvm/Target/TargetInstrInfo.h"
 #include "llvm/Target/TargetMachine.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/STLExtras.h"
@@ -48,12 +49,19 @@
     RegsAvailable.set(SubReg);
 }
 
+bool RegScavenger::isAliasUsed(unsigned Reg) const {
+  if (isUsed(Reg))
+    return true;
+  for (const unsigned *R = TRI->getAliasSet(Reg); *R; ++R)
+    if (isUsed(*R))
+      return true;
+  return false;
+}
+
 void RegScavenger::initRegState() {
   ScavengedReg = 0;
   ScavengedRC = NULL;
   ScavengeRestore = NULL;
-  CurrDist = 0;
-  DistanceMap.clear();
 
   // All registers started out unused.
   RegsAvailable.set();
@@ -176,7 +184,6 @@
   }
 
   MachineInstr *MI = MBBI;
-  DistanceMap.insert(std::make_pair(MI, CurrDist++));
 
   if (MI == ScavengeRestore) {
     ScavengedReg = 0;
@@ -286,12 +293,25 @@
   return (Reg == -1) ? 0 : Reg;
 }
 
+/// DistanceMap - Keep track the distance of an MI from the current position.
+typedef DenseMap<MachineInstr*, unsigned> DistanceMap;
+
+/// Build a distance map for instructions from I to E.
+static void buildDistanceMap(DistanceMap &DM,
+                             MachineBasicBlock::iterator I,
+                             MachineBasicBlock::iterator E) {
+  DM.clear();
+  for (unsigned d = 0; I != E; ++I, ++d)
+    DM.insert(DistanceMap::value_type(I, d));
+}
+
 /// findFirstUse - Calculate the distance to the first use of the
-/// specified register.
-MachineInstr*
-RegScavenger::findFirstUse(MachineBasicBlock *MBB,
-                           MachineBasicBlock::iterator I, unsigned Reg,
-                           unsigned &Dist) {
+/// specified register in the range covered by DM.
+static MachineInstr *findFirstUse(const MachineBasicBlock *MBB,
+                                  const DistanceMap &DM,
+                                  unsigned Reg,
+                                  unsigned &Dist) {
+  const MachineRegisterInfo *MRI = &MBB->getParent()->getRegInfo();
   MachineInstr *UseMI = 0;
   Dist = ~0U;
   for (MachineRegisterInfo::reg_iterator RI = MRI->reg_begin(Reg),
@@ -299,19 +319,10 @@
     MachineInstr *UDMI = &*RI;
     if (UDMI->getParent() != MBB)
       continue;
-    DenseMap<MachineInstr*, unsigned>::iterator DI = DistanceMap.find(UDMI);
-    if (DI == DistanceMap.end()) {
-      // If it's not in map, it's below current MI, let's initialize the
-      // map.
-      I = next(I);
-      unsigned Dist = CurrDist + 1;
-      while (I != MBB->end()) {
-        DistanceMap.insert(std::make_pair(I, Dist++));
-        I = next(I);
-      }
-    }
-    DI = DistanceMap.find(UDMI);
-    if (DI->second > CurrDist && DI->second < Dist) {
+    DistanceMap::const_iterator DI = DM.find(UDMI);
+    if (DI == DM.end())
+      continue;
+    if (DI->second < Dist) {
       Dist = DI->second;
       UseMI = UDMI;
     }
@@ -338,6 +349,10 @@
       Candidates.reset(MO.getReg());
   }
 
+  // Prepare to call findFirstUse() a number of times.
+  DistanceMap DM;
+  buildDistanceMap(DM, I, MBB->end());
+
   // Find the register whose use is furthest away.
   unsigned SReg = 0;
   unsigned MaxDist = 0;
@@ -345,15 +360,23 @@
   int Reg = Candidates.find_first();
   while (Reg != -1) {
     unsigned Dist;
-    MachineInstr *UseMI = findFirstUse(MBB, I, Reg, Dist);
+    MachineInstr *UseMI = findFirstUse(MBB, DM, Reg, Dist);
     for (const unsigned *AS = TRI->getAliasSet(Reg); *AS; ++AS) {
       unsigned AsDist;
-      MachineInstr *AsUseMI = findFirstUse(MBB, I, *AS, AsDist);
+      MachineInstr *AsUseMI = findFirstUse(MBB, DM, *AS, AsDist);
       if (AsDist < Dist) {
         Dist = AsDist;
         UseMI = AsUseMI;
       }
     }
+
+    // If we found an unused register that is defined by a later instruction,
+    // there is no reason to spill it. We have probably found a callee-saved
+    // register that has been saved in the prologue, but happens to be unused at
+    // this point.
+    if (!isAliasUsed(Reg) && UseMI != NULL)
+      return Reg;
+
     if (Dist >= MaxDist) {
       MaxDist = Dist;
       MaxUseMI = UseMI;

Added: llvm/trunk/test/CodeGen/Blackfin/2009-08-11-RegScavenger-CSR.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Blackfin/2009-08-11-RegScavenger-CSR.ll?rev=78650&view=auto

==============================================================================
--- llvm/trunk/test/CodeGen/Blackfin/2009-08-11-RegScavenger-CSR.ll (added)
+++ llvm/trunk/test/CodeGen/Blackfin/2009-08-11-RegScavenger-CSR.ll Tue Aug 11 01:25:12 2009
@@ -0,0 +1,17 @@
+; RUN: llvm-as < %s | llc -march=bfin -verify-machineinstrs
+
+declare i64 @llvm.cttz.i64(i64) nounwind readnone
+
+declare i16 @llvm.cttz.i16(i16) nounwind readnone
+
+declare i8 @llvm.cttz.i8(i8) nounwind readnone
+
+define void @cttztest(i8 %A, i16 %B, i32 %C, i64 %D, i8* %AP, i16* %BP, i32* %CP, i64* %DP) {
+	%a = call i8 @llvm.cttz.i8(i8 %A)		; <i8> [#uses=1]
+	%b = call i16 @llvm.cttz.i16(i16 %B)		; <i16> [#uses=1]
+	%d = call i64 @llvm.cttz.i64(i64 %D)		; <i64> [#uses=1]
+	store i8 %a, i8* %AP
+	store i16 %b, i16* %BP
+	store i64 %d, i64* %DP
+	ret void
+}





More information about the llvm-commits mailing list