[llvm-branch-commits] [llvm-branch] r96074 - in /llvm/branches/Apple/Hermes: lib/CodeGen/SimpleRegisterCoalescing.cpp lib/CodeGen/VirtRegRewriter.cpp test/CodeGen/X86/2010-02-12-CoalescerBug-Impdef.ll

[Jakob Stoklund Olesen]

Author: stoklund
Date: Fri Feb 12 20:08:28 2010
New Revision: 96074

URL: http://llvm.org/viewvc/llvm-project?rev=96074&view=rev
Log:
--- Merging r96072 into '.':
A    test/CodeGen/X86/2010-02-12-CoalescerBug-Impdef.ll
U    lib/CodeGen/VirtRegRewriter.cpp
U    lib/CodeGen/SimpleRegisterCoalescing.cpp

Added:
    llvm/branches/Apple/Hermes/test/CodeGen/X86/2010-02-12-CoalescerBug-Impdef.ll
      - copied unchanged from r96072, llvm/trunk/test/CodeGen/X86/2010-02-12-CoalescerBug-Impdef.ll
Modified:
    llvm/branches/Apple/Hermes/lib/CodeGen/SimpleRegisterCoalescing.cpp
    llvm/branches/Apple/Hermes/lib/CodeGen/VirtRegRewriter.cpp

Modified: llvm/branches/Apple/Hermes/lib/CodeGen/SimpleRegisterCoalescing.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/Apple/Hermes/lib/CodeGen/SimpleRegisterCoalescing.cpp?rev=96074&r1=96073&r2=96074&view=diff

==============================================================================
--- llvm/branches/Apple/Hermes/lib/CodeGen/SimpleRegisterCoalescing.cpp (original)
+++ llvm/branches/Apple/Hermes/lib/CodeGen/SimpleRegisterCoalescing.cpp Fri Feb 12 20:08:28 2010
@@ -771,11 +771,16 @@
     SubIdx = 0;
   }
 
+  // Copy the register use-list before traversing it. We may be adding operands
+  // and invalidating pointers.
+  SmallVector<std::pair<MachineInstr*, unsigned>, 32> reglist;
   for (MachineRegisterInfo::reg_iterator I = mri_->reg_begin(SrcReg),
-         E = mri_->reg_end(); I != E; ) {
-    MachineOperand &O = I.getOperand();
-    MachineInstr *UseMI = &*I;
-    ++I;
+         E = mri_->reg_end(); I != E; ++I)
+    reglist.push_back(std::make_pair(&*I, I.getOperandNo()));
+
+  for (unsigned N=0; N != reglist.size(); ++N) {
+    MachineInstr *UseMI = reglist[N].first;
+    MachineOperand &O = UseMI->getOperand(reglist[N].second);
     unsigned OldSubIdx = O.getSubReg();
     if (DstIsPhys) {
       unsigned UseDstReg = DstReg;
@@ -796,6 +801,19 @@
 
       O.setReg(UseDstReg);
       O.setSubReg(0);
+      if (OldSubIdx) {
+        // Def and kill of subregister of a virtual register actually defs and
+        // kills the whole register. Add imp-defs and imp-kills as needed.
+        if (O.isDef()) {
+          if(O.isDead())
+            UseMI->addRegisterDead(DstReg, tri_, true);
+          else
+            UseMI->addRegisterDefined(DstReg, tri_);
+        } else if (!O.isUndef() &&
+                   (O.isKill() ||
+                    UseMI->isRegTiedToDefOperand(&O-&UseMI->getOperand(0))))
+          UseMI->addRegisterKilled(DstReg, tri_, true);
+      }
       continue;
     }
 

Modified: llvm/branches/Apple/Hermes/lib/CodeGen/VirtRegRewriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/branches/Apple/Hermes/lib/CodeGen/VirtRegRewriter.cpp?rev=96074&r1=96073&r2=96074&view=diff

==============================================================================
--- llvm/branches/Apple/Hermes/lib/CodeGen/VirtRegRewriter.cpp (original)
+++ llvm/branches/Apple/Hermes/lib/CodeGen/VirtRegRewriter.cpp Fri Feb 12 20:08:28 2010
@@ -62,6 +62,7 @@
 
 /// substitutePhysReg - Replace virtual register in MachineOperand with a
 /// physical register. Do the right thing with the sub-register index.
+/// Note that operands may be added, so the MO reference is no longer valid.
 static void substitutePhysReg(MachineOperand &MO, unsigned Reg,
                               const TargetRegisterInfo &TRI) {
   if (unsigned SubIdx = MO.getSubReg()) {
@@ -123,14 +124,15 @@
           continue;
         unsigned pReg = VRM.getPhys(reg);
         mri->setPhysRegUsed(pReg);
-        for (MachineRegisterInfo::reg_iterator regItr = mri->reg_begin(reg),
-             regEnd = mri->reg_end(); regItr != regEnd;) {
-          MachineOperand &mop = regItr.getOperand();
-          assert(mop.isReg() && mop.getReg() == reg && "reg_iterator broken?");
-          ++regItr;
-          substitutePhysReg(mop, pReg, *tri);
-          changed = true;
-        }
+        // Copy the register use-list before traversing it.
+        SmallVector<std::pair<MachineInstr*, unsigned>, 32> reglist;
+        for (MachineRegisterInfo::reg_iterator I = mri->reg_begin(reg),
+               E = mri->reg_end(); I != E; ++I)
+          reglist.push_back(std::make_pair(&*I, I.getOperandNo()));
+        for (unsigned N=0; N != reglist.size(); ++N)
+          substitutePhysReg(reglist[N].first->getOperand(reglist[N].second),
+                            pReg, *tri);
+        changed |= !reglist.empty();
       }
     }
     
@@ -1850,19 +1852,18 @@
       KilledMIRegs.clear();
       for (unsigned j = 0, e = VirtUseOps.size(); j != e; ++j) {
         unsigned i = VirtUseOps[j];
-        MachineOperand &MO = MI.getOperand(i);
-        unsigned VirtReg = MO.getReg();
+        unsigned VirtReg = MI.getOperand(i).getReg();
         assert(TargetRegisterInfo::isVirtualRegister(VirtReg) &&
                "Not a virtual register?");
 
-        unsigned SubIdx = MO.getSubReg();
+        unsigned SubIdx = MI.getOperand(i).getSubReg();
         if (VRM.isAssignedReg(VirtReg)) {
           // This virtual register was assigned a physreg!
           unsigned Phys = VRM.getPhys(VirtReg);
           RegInfo->setPhysRegUsed(Phys);
-          if (MO.isDef())
+          if (MI.getOperand(i).isDef())
             ReusedOperands.markClobbered(Phys);
-          substitutePhysReg(MO, Phys, *TRI);
+          substitutePhysReg(MI.getOperand(i), Phys, *TRI);
           if (VRM.isImplicitlyDefined(VirtReg))
             // FIXME: Is this needed?
             BuildMI(MBB, &MI, MI.getDebugLoc(),
@@ -1871,10 +1872,10 @@
         }
 
         // This virtual register is now known to be a spilled value.
-        if (!MO.isUse())
+        if (!MI.getOperand(i).isUse())
           continue;  // Handle defs in the loop below (handle use&def here though)
 
-        bool AvoidReload = MO.isUndef();
+        bool AvoidReload = MI.getOperand(i).isUndef();
         // Check if it is defined by an implicit def. It should not be spilled.
         // Note, this is for correctness reason. e.g.
         // 8   %reg1024<def> = IMPLICIT_DEF