Simplify RegScavenger::forward a bit more.

Verify that early clobber registers and their aliases are not used.

All changes to RegsAvailable are now done as a transaction so the order of
operands makes no difference.

The included test case is from PR4686. It has behaviour that was dependent on the order of operands.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@78465 91177308-0d34-0410-b5e6-96231b3b80d8

3 files changed, 80 insertions, 66 deletions

diff --git a/include/llvm/CodeGen/RegisterScavenging.h b/include/llvm/CodeGen/RegisterScavenging.h
index b0bcc67..478da3d 100644
--- a/include/llvm/CodeGen/RegisterScavenging.h
+++ b/include/llvm/CodeGen/RegisterScavenging.h
@@ -162,8 +162,14 @@ private:
   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);
+
+  /// Add Reg and its aliases to BV.
+  void addRegWithAliases(BitVector &BV, unsigned Reg);
 };
- 
+
 } // End llvm namespace
 
 #endif
diff --git a/lib/CodeGen/RegisterScavenging.cpp b/lib/CodeGen/RegisterScavenging.cpp
index 5a2317d..bb12556 100644
--- a/lib/CodeGen/RegisterScavenging.cpp
+++ b/lib/CodeGen/RegisterScavenging.cpp
@@ -51,13 +51,6 @@ static bool RedefinesSuperRegPart(const MachineInstr *MI, unsigned SubReg,
   return SeenSuperDef && SeenSuperUse;
 }
 
-static bool RedefinesSuperRegPart(const MachineInstr *MI,
-                                  const MachineOperand &MO,
-                                  const TargetRegisterInfo *TRI) {
-  assert(MO.isReg() && MO.isDef() && "Not a register def!");
-  return RedefinesSuperRegPart(MI, MO.getReg(), TRI);
-}
-
 bool RegScavenger::isSuperRegUsed(unsigned Reg) const {
   for (const unsigned *SuperRegs = TRI->getSuperRegisters(Reg);
        unsigned SuperReg = *SuperRegs; ++SuperRegs)
@@ -190,6 +183,18 @@ static bool isLiveInButUnusedBefore(unsigned Reg, MachineInstr *MI,
 }
 #endif
 
+void RegScavenger::addRegWithSubRegs(BitVector &BV, unsigned Reg) {
+  BV.set(Reg);
+  for (const unsigned *R = TRI->getSubRegisters(Reg); *R; R++)
+    BV.set(*R);
+}
+
+void RegScavenger::addRegWithAliases(BitVector &BV, unsigned Reg) {
+  BV.set(Reg);
+  for (const unsigned *R = TRI->getAliasSet(Reg); *R; R++)
+    BV.set(*R);
+}
+
 void RegScavenger::forward() {
   // Move ptr forward.
   if (!Tracking) {
@@ -209,76 +214,59 @@ void RegScavenger::forward() {
     ScavengeRestore = NULL;
   }
 
-  // Separate register operands into 3 classes: uses, defs, earlyclobbers.
-  SmallVector<std::pair<const MachineOperand*,unsigned>, 4> UseMOs;
-  SmallVector<std::pair<const MachineOperand*,unsigned>, 4> DefMOs;
-  SmallVector<std::pair<const MachineOperand*,unsigned>, 4> EarlyClobberMOs;
+  // Find out which registers are early clobbered, killed, defined, and marked
+  // def-dead in this instruction.
+  BitVector EarlyClobberRegs(NumPhysRegs);
+  BitVector KillRegs(NumPhysRegs);
+  BitVector DefRegs(NumPhysRegs);
+  BitVector DeadRegs(NumPhysRegs);
   for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
     const MachineOperand &MO = MI->getOperand(i);
-    if (!MO.isReg() || MO.getReg() == 0 || MO.isUndef())
+    if (!MO.isReg() || MO.isUndef())
       continue;
-    if (MO.isUse())
-      UseMOs.push_back(std::make_pair(&MO,i));
-    else if (MO.isEarlyClobber())
-      EarlyClobberMOs.push_back(std::make_pair(&MO,i));
-    else {
-      assert(MO.isDef());
-      DefMOs.push_back(std::make_pair(&MO,i));
-    }
-  }
-
-  // Process uses first.
-  BitVector KillRegs(NumPhysRegs);
-  for (unsigned i = 0, e = UseMOs.size(); i != e; ++i) {
-    const MachineOperand MO = *UseMOs[i].first;
-    unsigned Idx = UseMOs[i].second;
     unsigned Reg = MO.getReg();
+    if (!Reg || isReserved(Reg))
+      continue;
 
-    assert(isUsed(Reg) && "Using an undefined register!");
-
-    // Two-address operands implicitly kill.
-    if ((MO.isKill() || MI->isRegTiedToDefOperand(Idx)) && !isReserved(Reg)) {
-      KillRegs.set(Reg);
-
-      // Mark sub-registers as used.
-      for (const unsigned *SubRegs = TRI->getSubRegisters(Reg);
-           unsigned SubReg = *SubRegs; ++SubRegs)
-        KillRegs.set(SubReg);
+    if (MO.isUse()) {
+      // Two-address operands implicitly kill.
+      if (MO.isKill() || MI->isRegTiedToDefOperand(i))
+        addRegWithSubRegs(KillRegs, Reg);
+    } else {
+      assert(MO.isDef());
+      if (MO.isDead())
+        addRegWithSubRegs(DeadRegs, Reg);
+      else
+        addRegWithSubRegs(DefRegs, Reg);
+      if (MO.isEarlyClobber())
+        addRegWithAliases(EarlyClobberRegs, Reg);
     }
   }
 
-  // Change states of all registers after all the uses are processed to guard
-  // against multiple uses.
-  setUnused(KillRegs);
-
-  // Process early clobber defs then process defs. We can have a early clobber
-  // that is dead, it should not conflict with a def that happens one "slot"
-  // (see InstrSlots in LiveIntervalAnalysis.h) later.
-  unsigned NumECs = EarlyClobberMOs.size();
-  unsigned NumDefs = DefMOs.size();
-
-  for (unsigned i = 0, e = NumECs + NumDefs; i != e; ++i) {
-    const MachineOperand &MO = (i < NumECs)
-      ? *EarlyClobberMOs[i].first : *DefMOs[i-NumECs].first;
-    unsigned Reg = MO.getReg();
-    if (MO.isUndef())
+  // Verify uses and defs.
+  for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
+    const MachineOperand &MO = MI->getOperand(i);
+    if (!MO.isReg() || MO.isUndef())
       continue;
-
-    // If it's dead upon def, then it is now free.
-    if (MO.isDead()) {
-      setUnused(Reg, MI);
+    unsigned Reg = MO.getReg();
+    if (!Reg || isReserved(Reg))
       continue;
+    if (MO.isUse()) {
+      assert(isUsed(Reg) && "Using an undefined register!");
+      assert(!EarlyClobberRegs.test(Reg) &&
+             "Using an early clobbered register!");
+    } else {
+      assert(MO.isDef());
+      assert((KillRegs.test(Reg) || isUnused(Reg) || isSuperRegUsed(Reg) ||
+              isLiveInButUnusedBefore(Reg, MI, MBB, TRI, MRI)) &&
+             "Re-defining a live register!");
     }
-
-    // Skip if this is merely redefining part of a super-register.
-    if (RedefinesSuperRegPart(MI, MO, TRI))
-      continue;
-
-    assert((isReserved(Reg) || isUnused(Reg) || isSuperRegUsed(Reg) ||
-            isLiveInButUnusedBefore(Reg, MI, MBB, TRI, MRI)) &&
-           "Re-defining a live register!");
-    setUsed(Reg);
   }
+
+  // Commit the changes.
+  setUnused(KillRegs);
+  setUnused(DeadRegs);
+  setUsed(DefRegs);
 }
 
 void RegScavenger::getRegsUsed(BitVector &used, bool includeReserved) {
diff --git a/test/CodeGen/Thumb2/2009-08-08-ScavengerAssert.ll b/test/CodeGen/Thumb2/2009-08-08-ScavengerAssert.ll
new file mode 100644
index 0000000..0d47c50
--- /dev/null
+++ b/test/CodeGen/Thumb2/2009-08-08-ScavengerAssert.ll
@@ -0,0 +1,20 @@
+; RUN: llvm-as < %s | llc -mtriple=armv7-eabi -mattr=+vfp2
+; PR4686
+
+@g_d = external global double		; <double*> [#uses=1]
+
+define arm_aapcscc void @foo(float %yIncr) {
+entry:
+	br i1 undef, label %bb, label %bb4
+
+bb:		; preds = %entry
+	%0 = call arm_aapcs_vfpcc  float @bar()		; <float> [#uses=1]
+	%1 = fpext float %0 to double		; <double> [#uses=1]
+	store double %1, double* @g_d, align 8
+	br label %bb4
+
+bb4:		; preds = %bb, %entry
+	unreachable
+}
+
+declare arm_aapcs_vfpcc float @bar()