Fix pr3954. The register scavenger asserts for inline assembly with

register destinations that are tied to source operands.  The
TargetInstrDescr::findTiedToSrcOperand method silently fails for inline
assembly.  The existing MachineInstr::isRegReDefinedByTwoAddr was very
close to doing what is needed, so this revision makes a few changes to
that method and also renames it to isRegTiedToUseOperand (for consistency
with the very similar isRegTiedToDefOperand and because it handles both
two-address instructions and inline assembly with tied registers).

llvm-svn: 68714

include/llvm/CodeGen/MachineInstr.h

@@ -241,9 +241,11 @@ public:
   /// none is found.
   int findFirstPredOperandIdx() const;
   
-  /// isRegReDefinedByTwoAddr - Given the index of a register def operand,
-  /// check if the register def is a re-definition due to two addr elimination.
-  bool isRegReDefinedByTwoAddr(unsigned DefIdx) const;
+  /// isRegTiedToUseOperand - Given the index of a register def operand,
+  /// check if the register def is tied to a source operand, due to either
+  /// two-address elimination or inline assembly constraints. Returns the
+  /// first tied use operand index by reference is UseOpIdx is not null.
+  bool isRegTiedToUseOperand(unsigned DefOpIdx, unsigned *UseOpIdx = 0);
 
   /// isRegTiedToDefOperand - Return true if the use operand of the specified
   /// index is tied to an def operand. It also returns the def operand index by

include/llvm/Target/TargetInstrDesc.h

@@ -131,10 +131,6 @@ public:
     return -1;
   }
 
-  /// findTiedToSrcOperand - Returns the operand that is tied to the specified
-  /// dest operand. Returns -1 if there isn't one.
-  int findTiedToSrcOperand(unsigned OpNum) const;
-  
   /// getOpcode - Return the opcode number for this descriptor.
   unsigned getOpcode() const {
     return Opcode;

lib/CodeGen/LiveIntervalAnalysis.cpp

@@ -469,7 +469,7 @@ void LiveIntervals::handleVirtualRegisterDef(MachineBasicBlock *mbb,
     // must be due to phi elimination or two addr elimination.  If this is
     // the result of two address elimination, then the vreg is one of the
     // def-and-use register operand.
-    if (mi->isRegReDefinedByTwoAddr(MOIdx)) {
+    if (mi->isRegTiedToUseOperand(MOIdx)) {
       // If this is a two-address definition, then we have already processed
       // the live range.  The only problem is that we didn't realize there
       // are actually two values in the live interval.  Because of this we

lib/CodeGen/MachineInstr.cpp

@@ -690,12 +690,14 @@ int MachineInstr::findFirstPredOperandIdx() const {
   return -1;
 }
   
-/// isRegReDefinedByTwoAddr - Given the index of a register operand,
-/// check if the register def is a re-definition due to two addr elimination.
-bool MachineInstr::isRegReDefinedByTwoAddr(unsigned DefIdx) const{
+/// isRegTiedToUseOperand - Given the index of a register def operand,
+/// check if the register def is tied to a source operand, due to either
+/// two-address elimination or inline assembly constraints. Returns the
+/// first tied use operand index by reference is UseOpIdx is not null.
+bool MachineInstr::isRegTiedToUseOperand(unsigned DefOpIdx, unsigned *UseOpIdx){
   if (getOpcode() == TargetInstrInfo::INLINEASM) {
-    assert(DefIdx >= 2);
-    const MachineOperand &MO = getOperand(DefIdx);
+    assert(DefOpIdx >= 2);
+    const MachineOperand &MO = getOperand(DefOpIdx);
     if (!MO.isReg() || !MO.isDef())
       return false;
     // Determine the actual operand no corresponding to this index.
@@ -705,7 +707,7 @@ bool MachineInstr::isRegReDefinedByTwoAddr(unsigned DefIdx) const{
       assert(FMO.isImm());
       // Skip over this def.
       i += InlineAsm::getNumOperandRegisters(FMO.getImm()) + 1;
-      if (i > DefIdx)
+      if (i > DefOpIdx)
         break;
       ++DefNo;
     }
@@ -717,18 +719,24 @@ bool MachineInstr::isRegReDefinedByTwoAddr(unsigned DefIdx) const{
         continue;
       unsigned Idx;
       if (InlineAsm::isUseOperandTiedToDef(FMO.getImm(), Idx) && 
-          Idx == DefNo)
+          Idx == DefNo) {
+        if (UseOpIdx)
+          *UseOpIdx = (unsigned)i + 1;
         return true;
+      }
     }
   }
 
-  assert(getOperand(DefIdx).isDef() && "DefIdx is not a def!");
+  assert(getOperand(DefOpIdx).isDef() && "DefOpIdx is not a def!");
   const TargetInstrDesc &TID = getDesc();
   for (unsigned i = 0, e = TID.getNumOperands(); i != e; ++i) {
     const MachineOperand &MO = getOperand(i);
     if (MO.isReg() && MO.isUse() &&
-        TID.getOperandConstraint(i, TOI::TIED_TO) == (int)DefIdx)
+        TID.getOperandConstraint(i, TOI::TIED_TO) == (int)DefOpIdx) {
+      if (UseOpIdx)
+        *UseOpIdx = (unsigned)i;
       return true;
+    }
   }
   return false;
 }

lib/CodeGen/PostRASchedulerList.cpp

@@ -500,7 +500,7 @@ void SchedulePostRATDList::ScanInstruction(MachineInstr *MI,
     if (Reg == 0) continue;
     if (!MO.isDef()) continue;
     // Ignore two-addr defs.
-    if (MI->isRegReDefinedByTwoAddr(i)) continue;
+    if (MI->isRegTiedToUseOperand(i)) continue;
 
     DefIndices[Reg] = Count;
     KillIndices[Reg] = ~0u;

lib/CodeGen/PreAllocSplitting.cpp

@@ -803,7 +803,7 @@ void PreAllocSplitting::RenumberValno(VNInfo* VN) {
       MachineInstr* MI = LIs->getInstructionFromIndex(*KI);
       unsigned DefIdx = MI->findRegisterDefOperandIdx(CurrLI->reg);
       if (DefIdx == ~0U) continue;
-      if (MI->isRegReDefinedByTwoAddr(DefIdx)) {
+      if (MI->isRegTiedToUseOperand(DefIdx)) {
         VNInfo* NextVN =
                      CurrLI->findDefinedVNInfo(LiveIntervals::getDefIndex(*KI));
         if (NextVN == OldVN) continue;
@@ -1214,7 +1214,7 @@ unsigned PreAllocSplitting::getNumberOfNonSpills(
       NonSpills++;
     
     int DefIdx = (*UI)->findRegisterDefOperandIdx(Reg);
-    if (DefIdx != -1 && (*UI)->isRegReDefinedByTwoAddr(DefIdx))
+    if (DefIdx != -1 && (*UI)->isRegTiedToUseOperand(DefIdx))
       FeedsTwoAddr = true;
   }
   

lib/CodeGen/RegAllocLocal.cpp

@@ -633,7 +633,7 @@ void RALocal::ComputeLocalLiveness(MachineBasicBlock& MBB) {
           // Check if this is a two address instruction.  If so, then
           // the def does not kill the use.
           if (last->second.first == I &&
-              I->isRegReDefinedByTwoAddr(i))
+              I->isRegTiedToUseOperand(i))
             continue;
           
           MachineOperand& lastUD =

lib/CodeGen/RegAllocSimple.cpp

@@ -204,8 +204,8 @@ void RegAllocSimple::AllocateBasicBlock(MachineBasicBlock &MBB) {
         unsigned physReg = Virt2PhysRegMap[virtualReg];
         if (physReg == 0) {
           if (MO.isDef()) {
-            int TiedOp = Desc.findTiedToSrcOperand(i);
-            if (TiedOp == -1) {
+            unsigned TiedOp;
+            if (!MI->isRegTiedToUseOperand(i, &TiedOp)) {
               physReg = getFreeReg(virtualReg);
             } else {
               // must be same register number as the source operand that is 

lib/CodeGen/RegisterScavenging.cpp

@@ -188,7 +188,6 @@ void RegScavenger::forward() {
 
   MachineInstr *MI = MBBI;
   DistanceMap.insert(std::make_pair(MI, CurrDist++));
-  const TargetInstrDesc &TID = MI->getDesc();
 
   if (MI == ScavengeRestore) {
     ScavengedReg = 0;
@@ -256,7 +255,7 @@ void RegScavenger::forward() {
     }
 
     // Skip two-address destination operand.
-    if (TID.findTiedToSrcOperand(Idx) != -1) {
+    if (MI->isRegTiedToUseOperand(Idx)) {
       assert(isUsed(Reg) && "Using an undefined register!");
       continue;
     }
@@ -284,7 +283,6 @@ void RegScavenger::backward() {
   MachineInstr *MI = MBBI;
   DistanceMap.erase(MI);
   --CurrDist;
-  const TargetInstrDesc &TID = MI->getDesc();
 
   // Separate register operands into 3 classes: uses, defs, earlyclobbers.
   SmallVector<std::pair<const MachineOperand*,unsigned>, 4> UseMOs;
@@ -313,7 +311,7 @@ void RegScavenger::backward() {
       ? DefMOs[i].second : EarlyClobberMOs[i-NumDefs].second;
 
     // Skip two-address destination operand.
-    if (TID.findTiedToSrcOperand(Idx) != -1)
+    if (MI->isRegTiedToUseOperand(Idx))
       continue;
 
     unsigned Reg = MO.getReg();

lib/CodeGen/Spiller.cpp

@@ -1589,8 +1589,8 @@ void LocalSpiller::RewriteMBB(MachineBasicBlock &MBB, VirtRegMap &VRM,
       // If this def is part of a two-address operand, make sure to execute
       // the store from the correct physical register.
       unsigned PhysReg;
-      int TiedOp = MI.getDesc().findTiedToSrcOperand(i);
-      if (TiedOp != -1) {
+      unsigned TiedOp;
+      if (MI.isRegTiedToUseOperand(i, &TiedOp)) {
         PhysReg = MI.getOperand(TiedOp).getReg();
         if (SubIdx) {
           unsigned SuperReg = findSuperReg(RC, PhysReg, SubIdx, TRI);

lib/Target/TargetInstrInfo.cpp

@@ -16,19 +16,6 @@
 #include "llvm/DerivedTypes.h"
 using namespace llvm;
 
-/// findTiedToSrcOperand - Returns the operand that is tied to the specified
-/// dest operand. Returns -1 if there isn't one.
-int TargetInstrDesc::findTiedToSrcOperand(unsigned OpNum) const {
-  for (unsigned i = 0, e = getNumOperands(); i != e; ++i) {
-    if (i == OpNum)
-      continue;
-    if (getOperandConstraint(i, TOI::TIED_TO) == (int)OpNum)
-      return i;
-  }
-  return -1;
-}
-
-
 TargetInstrInfo::TargetInstrInfo(const TargetInstrDesc* Desc,
                                  unsigned numOpcodes)
   : Descriptors(Desc), NumOpcodes(numOpcodes) {

test/CodeGen/ARM/2009-04-09-RegScavengerAsm.ll

@@ -0,0 +1,14 @@
+; RUN: llvm-as < %s | llc -march=arm
+; PR3954
+
+define void @foo(...) nounwind {
+entry:
+	%rr = alloca i32		; <i32*> [#uses=2]
+	%0 = load i32* %rr		; <i32> [#uses=1]
+	%1 = call i32 asm "nop", "=r,0"(i32 %0) nounwind		; <i32> [#uses=1]
+	store i32 %1, i32* %rr
+	br label %return
+
+return:		; preds = %entry
+	ret void
+}