Modify the two address instruction pass to remove the duplicate

operand of the instruction and thus simplify the register allocation.


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

include/llvm/CodeGen/MachineInstr.h

@@ -207,10 +207,6 @@ public:
     return *this;
   }
 
-  bool operator==(const MachineOperand& rhs) const {
-    return regNum == rhs.regNum && opType == rhs.opType;
-  }
-
   // Accessor methods.  Caller is responsible for checking the
   // operand type before invoking the corresponding accessor.
   // 
@@ -285,14 +281,14 @@ public:
     return *SymbolName;
   }
 
-  bool          isUse           () const { return flags & USEFLAG; }
+  bool            isUse           () const { return flags & USEFLAG; }
-  bool          isEverUsed      (const MachineInstr&) const;
+  MachineOperand& setUse          ()       { flags |= USEFLAG; return *this; }
-  bool		isDef           () const { return flags & DEFFLAG; }
+  bool		  isDef           () const { return flags & DEFFLAG; }
-  bool          isHiBits32      () const { return flags & HIFLAG32; }
+  MachineOperand& setDef          ()       { flags |= DEFFLAG; return *this; }
-  bool          isEverDefined   (const MachineInstr&) const;
+  bool            isHiBits32      () const { return flags & HIFLAG32; }
-  bool          isLoBits32      () const { return flags & LOFLAG32; }
+  bool            isLoBits32      () const { return flags & LOFLAG32; }
-  bool          isHiBits64      () const { return flags & HIFLAG64; }
+  bool            isHiBits64      () const { return flags & HIFLAG64; }
-  bool          isLoBits64      () const { return flags & LOFLAG64; }
+  bool            isLoBits64      () const { return flags & LOFLAG64; }
 
   // used to check if a machine register has been allocated to this operand
   bool hasAllocatedReg() const {

include/llvm/CodeGen/MachineInstrBuilder.h

@@ -138,9 +138,10 @@ inline MachineInstrBuilder BuildMI(int Opcode, unsigned NumOperands) {
 /// calls that are expected, it does not include the destination register.
 ///
 inline MachineInstrBuilder BuildMI(int Opcode, unsigned NumOperands,
-                                   unsigned DestReg) {
+                                   unsigned DestReg,
+                                   MOTy::UseType useType = MOTy::Def) {
   return MachineInstrBuilder(new MachineInstr(Opcode, NumOperands+1,
-                                   true, true)).addReg(DestReg, MOTy::Def);
+                                   true, true)).addReg(DestReg, useType);
 }
 
 

lib/CodeGen/MachineInstr.cpp

@@ -27,24 +27,6 @@ namespace llvm {
 //
 extern const TargetInstrDescriptor *TargetInstrDescriptors;
 
-bool MachineOperand::isEverUsed(const MachineInstr& mi) const
-{
-    for (int i = 0, e = mi.getNumOperands(); i != e; ++i) {
-        if (*this == mi.getOperand(i) && mi.getOperand(i).isUse())
-            return true;
-    }
-    return false;
-}
-
-bool MachineOperand::isEverDefined(const MachineInstr& mi) const
-{
-    for (int i = 0, e = mi.getNumOperands(); i != e; ++i) {
-        if (*this == mi.getOperand(i) && mi.getOperand(i).isDef())
-            return true;
-    }
-    return false;
-}
-
 // Constructor for instructions with variable #operands
 MachineInstr::MachineInstr(MachineOpCode OpCode, unsigned  numOperands)
   : opCode(OpCode),

lib/CodeGen/RegAllocLinearScan.cpp

@@ -109,10 +109,6 @@ namespace {
         typedef std::vector<const LiveIntervals::Interval*> IntervalPtrs;
         IntervalPtrs unhandled_, fixed_, active_, inactive_;
 
-        typedef std::vector<unsigned> Regs;
-        Regs tempUseOperands_;
-        Regs tempDefOperands_;
-
         PhysRegTracker prt_;
 
         typedef std::map<unsigned, unsigned> Virt2PhysMap;
@@ -428,7 +424,6 @@ bool RA::runOnMachineFunction(MachineFunction &fn) {
 
         for (currentInstr_ = currentMbb_->begin();
              currentInstr_ != currentMbb_->end(); ) {
-
             DEBUG(std::cerr << "\tinstruction: ";
                   (*currentInstr_)->print(std::cerr, *tm_););
 
@@ -465,13 +460,17 @@ bool RA::runOnMachineFunction(MachineFunction &fn) {
                 continue;
             }
 
+            typedef std::vector<unsigned> Regs;
+            Regs toClear;
+            Regs toSpill;
+
+            const unsigned numOperands = (*currentInstr_)->getNumOperands();
+
             DEBUG(std::cerr << "\t\tloading temporarily used operands to "
                   "registers:\n");
-            for (unsigned i = 0, e = (*currentInstr_)->getNumOperands();
+            for (unsigned i = 0; i != numOperands; ++i) {
-                 i != e; ++i) {
                 MachineOperand& op = (*currentInstr_)->getOperand(i);
-                if (op.isVirtualRegister() && op.isUse() &&
+                if (op.isVirtualRegister() && op.isUse()) {
-                    !op.isEverDefined(**currentInstr_)) {
                     unsigned virtReg = op.getAllocatedRegNum();
                     unsigned physReg = 0;
                     Virt2PhysMap::const_iterator it = v2pMap_.find(virtReg);
@@ -481,26 +480,28 @@ bool RA::runOnMachineFunction(MachineFunction &fn) {
                     else {
                         physReg = getFreeTempPhysReg(virtReg);
                         loadVirt2PhysReg(virtReg, physReg);
-                        tempUseOperands_.push_back(virtReg);
+                        // we will clear uses that are not also defs
+                        // before we allocate registers the defs
+                        if (op.isDef())
+                            toSpill.push_back(virtReg);
+                        else
+                            toClear.push_back(virtReg);
                     }
                     (*currentInstr_)->SetMachineOperandReg(i, physReg);
                 }
             }
 
-            DEBUG(std::cerr << "\t\tclearing temporarily used operands:\n");
+            DEBUG(std::cerr << "\t\tclearing temporarily used but not defined "
-            for (unsigned i = 0, e = tempUseOperands_.size(); i != e; ++i) {
+                  "operands:\n");
-                clearVirtReg(tempUseOperands_[i]);
+            std::for_each(toClear.begin(), toClear.end(),
-            }
+                          std::bind1st(std::mem_fun(&RA::clearVirtReg), this));
-            tempUseOperands_.clear();
 
             DEBUG(std::cerr << "\t\tassigning temporarily defined operands to "
                   "registers:\n");
-            for (unsigned i = 0, e = (*currentInstr_)->getNumOperands();
+            for (unsigned i = 0; i != numOperands; ++i) {
-                 i != e; ++i) {
                 MachineOperand& op = (*currentInstr_)->getOperand(i);
                 if (op.isVirtualRegister()) {
-                    assert(op.isEverDefined(**currentInstr_) &&
+                    assert(!op.isUse() && "we should not have uses here!");
-                           "operand should be defined by this instruction");
                     unsigned virtReg = op.getAllocatedRegNum();
                     unsigned physReg = 0;
                     Virt2PhysMap::const_iterator it = v2pMap_.find(virtReg);
@@ -510,21 +511,18 @@ bool RA::runOnMachineFunction(MachineFunction &fn) {
                     else {
                         physReg = getFreeTempPhysReg(virtReg);
                         assignVirt2PhysReg(virtReg, physReg);
-                        tempDefOperands_.push_back(virtReg);
+                        // need to spill this after we are done with
+                        // this instruction
+                        toSpill.push_back(virtReg);
                     }
                     (*currentInstr_)->SetMachineOperandReg(i, physReg);
                 }
             }
+            ++currentInstr_; // spills will go after this instruction
 
-            DEBUG(std::cerr << "\t\tspilling temporarily defined operands "
+            DEBUG(std::cerr << "\t\tspilling temporarily defined operands:\n");
-                  "of this instruction:\n");
+            std::for_each(toSpill.begin(), toSpill.end(),
-            ++currentInstr_; // we want to insert after this instruction
+                          std::bind1st(std::mem_fun(&RA::spillVirtReg), this));
-            for (unsigned i = 0, e = tempDefOperands_.size(); i != e; ++i) {
-                spillVirtReg(tempDefOperands_[i]);
-            }
-            --currentInstr_; // restore currentInstr_ iterator
-            tempDefOperands_.clear();
-            ++currentInstr_;
         }
     }
 

lib/CodeGen/TwoAddressInstructionPass.cpp

@@ -16,11 +16,14 @@
 // to:
 //
 //     A = B
-//     A = A op C
+//     A op= C
 //
-// Note that if a register allocator chooses to use this pass, that it has to
+// Note that if a register allocator chooses to use this pass, that it
-// be capable of handling the non-SSA nature of these rewritten virtual 
+// has to be capable of handling the non-SSA nature of these rewritten
-// registers.
+// virtual registers.
+//
+// It is also worth noting that the duplicate operand of the two
+// address instruction is removed.
 //
 //===----------------------------------------------------------------------===//
 
@@ -98,63 +101,70 @@ bool TwoAddressInstructionPass::runOnMachineFunction(MachineFunction &MF) {
                    mi->getOperand(1).isUse() &&
                    "two address instruction invalid");
 
-            // we have nothing to do if the two operands are the same
+            // if the two operands are the same we just remove the use
+            // and mark the def as def&use
             if (mi->getOperand(0).getAllocatedRegNum() ==
-                mi->getOperand(1).getAllocatedRegNum())
+                mi->getOperand(1).getAllocatedRegNum()) {
-                continue;
-
-            MadeChange = true;
-
-            // rewrite:
-            //     a = b op c
-            // to:
-            //     a = b
-            //     a = a op c
-            unsigned regA = mi->getOperand(0).getAllocatedRegNum();
-            unsigned regB = mi->getOperand(1).getAllocatedRegNum();
-
-            assert(MRegisterInfo::isVirtualRegister(regA) &&
-                   MRegisterInfo::isVirtualRegister(regB) &&
-                   "cannot update physical register live information");
-
-            // first make sure we do not have a use of a in the
-            // instruction (a = b + a for example) because our
-            // transformation will not work. This should never occur
-            // because we are in SSA form.
-            for (unsigned i = 1; i != mi->getNumOperands(); ++i)
-                assert(!mi->getOperand(i).isRegister() ||
-                       mi->getOperand(i).getAllocatedRegNum() != (int)regA);
-
-            const TargetRegisterClass* rc =MF.getSSARegMap()->getRegClass(regA);
-            unsigned Added = MRI.copyRegToReg(*mbbi, mii, regA, regB, rc);
-            numInstrsAdded += Added;
-
-            MachineInstr* prevMi = *(mii - 1);
-            DEBUG(std::cerr << "\t\tadded instruction: ";
-                  prevMi->print(std::cerr, TM));
-
-            // update live variables for regA
-            assert(Added == 1 && "Cannot handle multi-instruction copies yet!");
-            LiveVariables::VarInfo& varInfo = LV.getVarInfo(regA);
-            varInfo.DefInst = prevMi;
-
-            // update live variables for regB
-            if (LV.removeVirtualRegisterKilled(regB, &*mbbi, mi))
-                LV.addVirtualRegisterKilled(regB, &*mbbi, prevMi);
-
-            if (LV.removeVirtualRegisterDead(regB, &*mbbi, mi))
-                LV.addVirtualRegisterDead(regB, &*mbbi, prevMi);
-
-            // replace all occurences of regB with regA
-            for (unsigned i = 1; i < mi->getNumOperands(); ++i) {
-                if (mi->getOperand(i).isRegister() &&
-                    mi->getOperand(i).getReg() == regB)
-                    mi->SetMachineOperandReg(i, regA);
             }
+            else {
+                MadeChange = true;
+
+                // rewrite:
+                //     a = b op c
+                // to:
+                //     a = b
+                //     a = a op c
+                unsigned regA = mi->getOperand(0).getAllocatedRegNum();
+                unsigned regB = mi->getOperand(1).getAllocatedRegNum();
+
+                assert(MRegisterInfo::isVirtualRegister(regA) &&
+                       MRegisterInfo::isVirtualRegister(regB) &&
+                       "cannot update physical register live information");
+
+                // first make sure we do not have a use of a in the
+                // instruction (a = b + a for example) because our
+                // transformation will not work. This should never occur
+                // because we are in SSA form.
+                for (unsigned i = 1; i != mi->getNumOperands(); ++i)
+                    assert(!mi->getOperand(i).isRegister() ||
+                           mi->getOperand(i).getAllocatedRegNum() != (int)regA);
+
+                const TargetRegisterClass* rc =
+                    MF.getSSARegMap()->getRegClass(regA);
+                unsigned Added = MRI.copyRegToReg(*mbbi, mii, regA, regB, rc);
+                numInstrsAdded += Added;
+
+                MachineInstr* prevMi = *(mii - 1);
+                DEBUG(std::cerr << "\t\tadded instruction: ";
+                      prevMi->print(std::cerr, TM));
+
+                // update live variables for regA
+                assert(Added == 1 &&
+                       "Cannot handle multi-instruction copies yet!");
+                LiveVariables::VarInfo& varInfo = LV.getVarInfo(regA);
+                varInfo.DefInst = prevMi;
+
+                // update live variables for regB
+                if (LV.removeVirtualRegisterKilled(regB, &*mbbi, mi))
+                    LV.addVirtualRegisterKilled(regB, &*mbbi, prevMi);
+
+                if (LV.removeVirtualRegisterDead(regB, &*mbbi, mi))
+                    LV.addVirtualRegisterDead(regB, &*mbbi, prevMi);
+
+                // replace all occurences of regB with regA
+                for (unsigned i = 1, e = mi->getNumOperands(); i != e; ++i) {
+                    if (mi->getOperand(i).isRegister() &&
+                        mi->getOperand(i).getReg() == regB)
+                        mi->SetMachineOperandReg(i, regA);
+                }
+            }
+
+            assert(mi->getOperand(0).isDef());
+            mi->getOperand(0).setUse();
+            mi->RemoveOperand(1);
+
             DEBUG(std::cerr << "\t\tmodified original to: ";
                   mi->print(std::cerr, TM));
-            assert(mi->getOperand(0).getAllocatedRegNum() ==
-                   mi->getOperand(1).getAllocatedRegNum());
         }
     }
 

lib/Target/X86/PeepholeOptimizer.cpp

@@ -67,16 +67,36 @@ bool PH::PeepholeOptimize(MachineBasicBlock &MBB,
     // immediate despite the fact that the operands are 16 or 32 bits.  Because
     // this can save three bytes of code size (and icache space), we want to
     // shrink them if possible.
-  case X86::ADDri16:  case X86::ADDri32:
-  case X86::SUBri16:  case X86::SUBri32:
   case X86::IMULri16: case X86::IMULri32:
-  case X86::ANDri16:  case X86::ANDri32:
-  case X86::ORri16:   case X86::ORri32:
-  case X86::XORri16:  case X86::XORri32:
     assert(MI->getNumOperands() == 3 && "These should all have 3 operands!");
     if (MI->getOperand(2).isImmediate()) {
       int Val = MI->getOperand(2).getImmedValue();
       // If the value is the same when signed extended from 8 bits...
+      if (Val == (signed int)(signed char)Val) {
+        unsigned Opcode;
+        switch (MI->getOpcode()) {
+        default: assert(0 && "Unknown opcode value!");
+        case X86::IMULri16: Opcode = X86::IMULri16b; break;
+        case X86::IMULri32: Opcode = X86::IMULri32b; break;
+        }
+        unsigned R0 = MI->getOperand(0).getReg();
+        unsigned R1 = MI->getOperand(1).getReg();
+        *I = BuildMI(Opcode, 2, R0).addReg(R1).addZImm((char)Val);
+        delete MI;
+        return true;
+      }
+    }
+    return false;
+
+  case X86::ADDri16:  case X86::ADDri32:
+  case X86::SUBri16:  case X86::SUBri32:
+  case X86::ANDri16:  case X86::ANDri32:
+  case X86::ORri16:   case X86::ORri32:
+  case X86::XORri16:  case X86::XORri32:
+    assert(MI->getNumOperands() == 2 && "These should all have 2 operands!");
+    if (MI->getOperand(1).isImmediate()) {
+      int Val = MI->getOperand(1).getImmedValue();
+      // If the value is the same when signed extended from 8 bits...
       if (Val == (signed int)(signed char)Val) {
         unsigned Opcode;
         switch (MI->getOpcode()) {
@@ -85,8 +105,6 @@ bool PH::PeepholeOptimize(MachineBasicBlock &MBB,
         case X86::ADDri32:  Opcode = X86::ADDri32b; break;
         case X86::SUBri16:  Opcode = X86::SUBri16b; break;
         case X86::SUBri32:  Opcode = X86::SUBri32b; break;
-        case X86::IMULri16: Opcode = X86::IMULri16b; break;
-        case X86::IMULri32: Opcode = X86::IMULri32b; break;
         case X86::ANDri16:  Opcode = X86::ANDri16b; break;
         case X86::ANDri32:  Opcode = X86::ANDri32b; break;
         case X86::ORri16:   Opcode = X86::ORri16b; break;
@@ -95,8 +113,7 @@ bool PH::PeepholeOptimize(MachineBasicBlock &MBB,
         case X86::XORri32:  Opcode = X86::XORri32b; break;
         }
         unsigned R0 = MI->getOperand(0).getReg();
-        unsigned R1 = MI->getOperand(1).getReg();
+        *I = BuildMI(Opcode, 1, R0, MOTy::UseAndDef).addZImm((char)Val);
-        *I = BuildMI(Opcode, 2, R0).addReg(R1).addZImm((char)Val);
         delete MI;
         return true;
       }

lib/Target/X86/Printer.cpp

@@ -609,35 +609,31 @@ void Printer::printMachineInstruction(const MachineInstr *MI) {
     return;
   }
   case X86II::MRMDestReg: {
-    // There are two acceptable forms of MRMDestReg instructions, those with 2,
+    // There are three forms of MRMDestReg instructions, those with 2
-    // 3 and 4 operands:
+    // or 3 operands:
     //
-    // 2 Operands: this is for things like mov that do not read a second input
+    // 2 Operands: this is for things like mov that do not read a
+    // second input.
     //
-    // 3 Operands: in this form, the first two registers (the destination, and
+    // 2 Operands: two address instructions which def&use the first
-    // the first operand) should be the same, post register allocation.  The 3rd
+    // argument and use the second as input.
-    // operand is an additional input.  This should be for things like add
-    // instructions.
     //
-    // 4 Operands: This form is for instructions which are 3 operands forms, but
+    // 3 Operands: in this form, two address instructions are the same
-    // have a constant argument as well.
+    // as in 2 but have a constant argument as well.
     //
     bool isTwoAddr = TII.isTwoAddrInstr(Opcode);
     assert(MI->getOperand(0).isRegister() &&
            (MI->getNumOperands() == 2 ||
-	    (isTwoAddr && MI->getOperand(1).isRegister() &&
+            (MI->getNumOperands() == 3 && MI->getOperand(2).isImmediate()))
-	     MI->getOperand(0).getReg() == MI->getOperand(1).getReg() &&
-	     (MI->getNumOperands() == 3 ||
-	      (MI->getNumOperands() == 4 && MI->getOperand(3).isImmediate()))))
            && "Bad format for MRMDestReg!");
 
     O << TII.getName(MI->getOpCode()) << " ";
     printOp(MI->getOperand(0));
     O << ", ";
-    printOp(MI->getOperand(1+isTwoAddr));
+    printOp(MI->getOperand(1));
-    if (MI->getNumOperands() == 4) {
+    if (MI->getNumOperands() == 3) {
       O << ", ";
-      printOp(MI->getOperand(3));
+      printOp(MI->getOperand(2));
     }
     O << "\n";
     return;
@@ -659,40 +655,35 @@ void Printer::printMachineInstruction(const MachineInstr *MI) {
   }
 
   case X86II::MRMSrcReg: {
-    // There are three forms that are acceptable for MRMSrcReg instructions,
+    // There are three forms that are acceptable for MRMSrcReg
-    // those with 3 and 2 operands:
+    // instructions, those with 2 or 3 operands:
     //
-    // 3 Operands: in this form, the last register (the second input) is the
+    // 2 Operands: this is for things like mov that do not read a
-    // ModR/M input.  The first two operands should be the same, post register
+    // second input.
-    // allocation.  This is for things like: add r32, r/m32
+    //
+    // 2 Operands: in this form, the last register is the ModR/M
+    // input.  The first operand is a def&use.  This is for things
+    // like: add r32, r/m32
     //
     // 3 Operands: in this form, we can have 'INST R1, R2, imm', which is used
     // for instructions like the IMULri instructions.
     //
-    // 2 Operands: this is for things like mov that do not read a second input
     //
     assert(MI->getOperand(0).isRegister() &&
            MI->getOperand(1).isRegister() &&
-           (MI->getNumOperands() == 2 || 
+           (MI->getNumOperands() == 2 ||
-            (MI->getNumOperands() == 3 && 
+            (MI->getNumOperands() == 3 &&
-             (MI->getOperand(2).isRegister() ||
+             (MI->getOperand(2).isImmediate())))
-              MI->getOperand(2).isImmediate())))
            && "Bad format for MRMSrcReg!");
-    if (MI->getNumOperands() == 3 && !MI->getOperand(2).isImmediate() &&
-        MI->getOperand(0).getReg() != MI->getOperand(1).getReg())
-      O << "**";
 
     O << TII.getName(MI->getOpCode()) << " ";
     printOp(MI->getOperand(0));
-
-    // If this is IMULri* instructions, print the non-two-address operand.
-    if (MI->getNumOperands() == 3 && MI->getOperand(2).isImmediate()) {
-      O << ", ";
-      printOp(MI->getOperand(1));
-    }
-
     O << ", ";
-    printOp(MI->getOperand(MI->getNumOperands()-1));
+    printOp(MI->getOperand(1));
+    if (MI->getNumOperands() == 3) {
+        O << ", ";
+        printOp(MI->getOperand(2));
+    }
     O << "\n";
     return;
   }
@@ -705,7 +696,7 @@ void Printer::printMachineInstruction(const MachineInstr *MI) {
            (MI->getNumOperands() == 1+4 && isMem(MI, 1)) || 
            (MI->getNumOperands() == 2+4 && MI->getOperand(1).isRegister() && 
             isMem(MI, 2))
-           && "Bad format for MRMDestReg!");
+           && "Bad format for MRMSrcMem!");
     if (MI->getNumOperands() == 2+4 &&
         MI->getOperand(0).getReg() != MI->getOperand(1).getReg())
       O << "**";

lib/Target/X86/X86AsmPrinter.cpp

@@ -609,35 +609,31 @@ void Printer::printMachineInstruction(const MachineInstr *MI) {
     return;
   }
   case X86II::MRMDestReg: {
-    // There are two acceptable forms of MRMDestReg instructions, those with 2,
+    // There are three forms of MRMDestReg instructions, those with 2
-    // 3 and 4 operands:
+    // or 3 operands:
     //
-    // 2 Operands: this is for things like mov that do not read a second input
+    // 2 Operands: this is for things like mov that do not read a
+    // second input.
     //
-    // 3 Operands: in this form, the first two registers (the destination, and
+    // 2 Operands: two address instructions which def&use the first
-    // the first operand) should be the same, post register allocation.  The 3rd
+    // argument and use the second as input.
-    // operand is an additional input.  This should be for things like add
-    // instructions.
     //
-    // 4 Operands: This form is for instructions which are 3 operands forms, but
+    // 3 Operands: in this form, two address instructions are the same
-    // have a constant argument as well.
+    // as in 2 but have a constant argument as well.
     //
     bool isTwoAddr = TII.isTwoAddrInstr(Opcode);
     assert(MI->getOperand(0).isRegister() &&
            (MI->getNumOperands() == 2 ||
-	    (isTwoAddr && MI->getOperand(1).isRegister() &&
+            (MI->getNumOperands() == 3 && MI->getOperand(2).isImmediate()))
-	     MI->getOperand(0).getReg() == MI->getOperand(1).getReg() &&
-	     (MI->getNumOperands() == 3 ||
-	      (MI->getNumOperands() == 4 && MI->getOperand(3).isImmediate()))))
            && "Bad format for MRMDestReg!");
 
     O << TII.getName(MI->getOpCode()) << " ";
     printOp(MI->getOperand(0));
     O << ", ";
-    printOp(MI->getOperand(1+isTwoAddr));
+    printOp(MI->getOperand(1));
-    if (MI->getNumOperands() == 4) {
+    if (MI->getNumOperands() == 3) {
       O << ", ";
-      printOp(MI->getOperand(3));
+      printOp(MI->getOperand(2));
     }
     O << "\n";
     return;
@@ -659,40 +655,35 @@ void Printer::printMachineInstruction(const MachineInstr *MI) {
   }
 
   case X86II::MRMSrcReg: {
-    // There are three forms that are acceptable for MRMSrcReg instructions,
+    // There are three forms that are acceptable for MRMSrcReg
-    // those with 3 and 2 operands:
+    // instructions, those with 2 or 3 operands:
     //
-    // 3 Operands: in this form, the last register (the second input) is the
+    // 2 Operands: this is for things like mov that do not read a
-    // ModR/M input.  The first two operands should be the same, post register
+    // second input.
-    // allocation.  This is for things like: add r32, r/m32
+    //
+    // 2 Operands: in this form, the last register is the ModR/M
+    // input.  The first operand is a def&use.  This is for things
+    // like: add r32, r/m32
     //
     // 3 Operands: in this form, we can have 'INST R1, R2, imm', which is used
     // for instructions like the IMULri instructions.
     //
-    // 2 Operands: this is for things like mov that do not read a second input
     //
     assert(MI->getOperand(0).isRegister() &&
            MI->getOperand(1).isRegister() &&
-           (MI->getNumOperands() == 2 || 
+           (MI->getNumOperands() == 2 ||
-            (MI->getNumOperands() == 3 && 
+            (MI->getNumOperands() == 3 &&
-             (MI->getOperand(2).isRegister() ||
+             (MI->getOperand(2).isImmediate())))
-              MI->getOperand(2).isImmediate())))
            && "Bad format for MRMSrcReg!");
-    if (MI->getNumOperands() == 3 && !MI->getOperand(2).isImmediate() &&
-        MI->getOperand(0).getReg() != MI->getOperand(1).getReg())
-      O << "**";
 
     O << TII.getName(MI->getOpCode()) << " ";
     printOp(MI->getOperand(0));
-
-    // If this is IMULri* instructions, print the non-two-address operand.
-    if (MI->getNumOperands() == 3 && MI->getOperand(2).isImmediate()) {
-      O << ", ";
-      printOp(MI->getOperand(1));
-    }
-
     O << ", ";
-    printOp(MI->getOperand(MI->getNumOperands()-1));
+    printOp(MI->getOperand(1));
+    if (MI->getNumOperands() == 3) {
+        O << ", ";
+        printOp(MI->getOperand(2));
+    }
     O << "\n";
     return;
   }
@@ -705,7 +696,7 @@ void Printer::printMachineInstruction(const MachineInstr *MI) {
            (MI->getNumOperands() == 1+4 && isMem(MI, 1)) || 
            (MI->getNumOperands() == 2+4 && MI->getOperand(1).isRegister() && 
             isMem(MI, 2))
-           && "Bad format for MRMDestReg!");
+           && "Bad format for MRMSrcMem!");
     if (MI->getNumOperands() == 2+4 &&
         MI->getOperand(0).getReg() != MI->getOperand(1).getReg())
       O << "**";

lib/Target/X86/X86CodeEmitter.cpp

@@ -548,10 +548,10 @@ void Emitter::emitInstruction(MachineInstr &MI) {
 
   case X86II::MRMDestReg: {
     MCE.emitByte(BaseOpcode);
-    MachineOperand &SrcOp = MI.getOperand(1+II->isTwoAddrInstr(Opcode));
+    emitRegModRMByte(MI.getOperand(0).getReg(),
-    emitRegModRMByte(MI.getOperand(0).getReg(), getX86RegNum(SrcOp.getReg()));
+                     getX86RegNum(MI.getOperand(1).getReg()));
-    if (MI.getNumOperands() == 4)
+    if (MI.getNumOperands() == 3)
-      emitConstant(MI.getOperand(3).getImmedValue(), sizeOfPtr(Desc));
+      emitConstant(MI.getOperand(2).getImmedValue(), sizeOfPtr(Desc));
     break;
   }
   case X86II::MRMDestMem:
@@ -562,18 +562,10 @@ void Emitter::emitInstruction(MachineInstr &MI) {
   case X86II::MRMSrcReg:
     MCE.emitByte(BaseOpcode);
 
-    if (MI.getNumOperands() == 2) {
+    emitRegModRMByte(MI.getOperand(1).getReg(),
-      emitRegModRMByte(MI.getOperand(MI.getNumOperands()-1).getReg(),
+                     getX86RegNum(MI.getOperand(0).getReg()));
-                       getX86RegNum(MI.getOperand(0).getReg()));
+    if (MI.getNumOperands() == 3)
-    } else if (MI.getOperand(2).isImmediate()) {
-      emitRegModRMByte(MI.getOperand(1).getReg(),
-                       getX86RegNum(MI.getOperand(0).getReg()));
-
       emitConstant(MI.getOperand(2).getImmedValue(), sizeOfPtr(Desc));
-    } else {
-      emitRegModRMByte(MI.getOperand(2).getReg(),
-                       getX86RegNum(MI.getOperand(0).getReg()));
-    }
     break;
 
   case X86II::MRMSrcMem:

lib/Target/X86/X86PeepholeOpt.cpp

@@ -67,16 +67,36 @@ bool PH::PeepholeOptimize(MachineBasicBlock &MBB,
     // immediate despite the fact that the operands are 16 or 32 bits.  Because
     // this can save three bytes of code size (and icache space), we want to
     // shrink them if possible.
-  case X86::ADDri16:  case X86::ADDri32:
-  case X86::SUBri16:  case X86::SUBri32:
   case X86::IMULri16: case X86::IMULri32:
-  case X86::ANDri16:  case X86::ANDri32:
-  case X86::ORri16:   case X86::ORri32:
-  case X86::XORri16:  case X86::XORri32:
     assert(MI->getNumOperands() == 3 && "These should all have 3 operands!");
     if (MI->getOperand(2).isImmediate()) {
       int Val = MI->getOperand(2).getImmedValue();
       // If the value is the same when signed extended from 8 bits...
+      if (Val == (signed int)(signed char)Val) {
+        unsigned Opcode;
+        switch (MI->getOpcode()) {
+        default: assert(0 && "Unknown opcode value!");
+        case X86::IMULri16: Opcode = X86::IMULri16b; break;
+        case X86::IMULri32: Opcode = X86::IMULri32b; break;
+        }
+        unsigned R0 = MI->getOperand(0).getReg();
+        unsigned R1 = MI->getOperand(1).getReg();
+        *I = BuildMI(Opcode, 2, R0).addReg(R1).addZImm((char)Val);
+        delete MI;
+        return true;
+      }
+    }
+    return false;
+
+  case X86::ADDri16:  case X86::ADDri32:
+  case X86::SUBri16:  case X86::SUBri32:
+  case X86::ANDri16:  case X86::ANDri32:
+  case X86::ORri16:   case X86::ORri32:
+  case X86::XORri16:  case X86::XORri32:
+    assert(MI->getNumOperands() == 2 && "These should all have 2 operands!");
+    if (MI->getOperand(1).isImmediate()) {
+      int Val = MI->getOperand(1).getImmedValue();
+      // If the value is the same when signed extended from 8 bits...
       if (Val == (signed int)(signed char)Val) {
         unsigned Opcode;
         switch (MI->getOpcode()) {
@@ -85,8 +105,6 @@ bool PH::PeepholeOptimize(MachineBasicBlock &MBB,
         case X86::ADDri32:  Opcode = X86::ADDri32b; break;
         case X86::SUBri16:  Opcode = X86::SUBri16b; break;
         case X86::SUBri32:  Opcode = X86::SUBri32b; break;
-        case X86::IMULri16: Opcode = X86::IMULri16b; break;
-        case X86::IMULri32: Opcode = X86::IMULri32b; break;
         case X86::ANDri16:  Opcode = X86::ANDri16b; break;
         case X86::ANDri32:  Opcode = X86::ANDri32b; break;
         case X86::ORri16:   Opcode = X86::ORri16b; break;
@@ -95,8 +113,7 @@ bool PH::PeepholeOptimize(MachineBasicBlock &MBB,
         case X86::XORri32:  Opcode = X86::XORri32b; break;
         }
         unsigned R0 = MI->getOperand(0).getReg();
-        unsigned R1 = MI->getOperand(1).getReg();
+        *I = BuildMI(Opcode, 1, R0, MOTy::UseAndDef).addZImm((char)Val);
-        *I = BuildMI(Opcode, 2, R0).addReg(R1).addZImm((char)Val);
         delete MI;
         return true;
       }

lib/Target/X86/X86RegisterInfo.cpp

@@ -110,10 +110,10 @@ int X86RegisterInfo::eliminateCallFramePseudoInstr(MachineFunction &MF,
       Amount = (Amount+Align-1)/Align*Align;
 
       if (Old->getOpcode() == X86::ADJCALLSTACKDOWN) {
-	New=BuildMI(X86::SUBri32, 2, X86::ESP).addReg(X86::ESP).addZImm(Amount);
+	New=BuildMI(X86::SUBri32, 1, X86::ESP, MOTy::UseAndDef).addZImm(Amount);
       } else {
 	assert(Old->getOpcode() == X86::ADJCALLSTACKUP);
-	New=BuildMI(X86::ADDri32, 2, X86::ESP).addReg(X86::ESP).addZImm(Amount);
+	New=BuildMI(X86::ADDri32, 1, X86::ESP, MOTy::UseAndDef).addZImm(Amount);
       }
     }
   }
@@ -181,7 +181,7 @@ int X86RegisterInfo::emitPrologue(MachineFunction &MF) const {
     int EBPOffset = MFI->getObjectOffset(MFI->getObjectIndexEnd()-1)+4;
 
     if (NumBytes) {   // adjust stack pointer: ESP -= numbytes
-      MI= BuildMI(X86::SUBri32, 2, X86::ESP).addReg(X86::ESP).addZImm(NumBytes);
+      MI= BuildMI(X86::SUBri32, 1, X86::ESP, MOTy::UseAndDef).addZImm(NumBytes);
       MBBI = MBB.insert(MBBI, MI)+1;
     }
 
@@ -215,7 +215,7 @@ int X86RegisterInfo::emitPrologue(MachineFunction &MF) const {
 
     if (NumBytes) {
       // adjust stack pointer: ESP -= numbytes
-      MI= BuildMI(X86::SUBri32, 2, X86::ESP).addReg(X86::ESP).addZImm(NumBytes);
+      MI= BuildMI(X86::SUBri32, 1, X86::ESP, MOTy::UseAndDef).addZImm(NumBytes);
       MBB.insert(MBBI, MI);
     }
   }
@@ -248,7 +248,7 @@ int X86RegisterInfo::emitEpilogue(MachineFunction &MF,
     unsigned NumBytes = MFI->getStackSize();
 
     if (NumBytes) {    // adjust stack pointer back: ESP += numbytes
-      MI =BuildMI(X86::ADDri32, 2, X86::ESP).addReg(X86::ESP).addZImm(NumBytes);
+      MI =BuildMI(X86::ADDri32, 1, X86::ESP, MOTy::UseAndDef).addZImm(NumBytes);
       MBBI = 1+MBB.insert(MBBI, MI);
     }
   }