From: Bob Wilson Date: Thu, 9 Apr 2009 17:16:43 +0000 (+0000) Subject: Fix pr3954. The register scavenger asserts for inline assembly with X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=d9df5017040489303acb57bdd8697ef0f8bafc08;p=oota-llvm.git 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). git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@68714 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/CodeGen/MachineInstr.h b/include/llvm/CodeGen/MachineInstr.h index 5c646c6874a..3caf7acbae8 100644 --- a/include/llvm/CodeGen/MachineInstr.h +++ b/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 diff --git a/include/llvm/Target/TargetInstrDesc.h b/include/llvm/Target/TargetInstrDesc.h index 6a0a3969753..b389a4f746c 100644 --- a/include/llvm/Target/TargetInstrDesc.h +++ b/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; diff --git a/lib/CodeGen/LiveIntervalAnalysis.cpp b/lib/CodeGen/LiveIntervalAnalysis.cpp index cb08fe759f5..3bc05a9b9a6 100644 --- a/lib/CodeGen/LiveIntervalAnalysis.cpp +++ b/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 diff --git a/lib/CodeGen/MachineInstr.cpp b/lib/CodeGen/MachineInstr.cpp index ade8683ec79..d3b2e9a91c8 100644 --- a/lib/CodeGen/MachineInstr.cpp +++ b/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; } diff --git a/lib/CodeGen/PostRASchedulerList.cpp b/lib/CodeGen/PostRASchedulerList.cpp index 311ad281103..f4e958c1077 100644 --- a/lib/CodeGen/PostRASchedulerList.cpp +++ b/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; diff --git a/lib/CodeGen/PreAllocSplitting.cpp b/lib/CodeGen/PreAllocSplitting.cpp index d9775f9f0d8..c4bda4862e1 100644 --- a/lib/CodeGen/PreAllocSplitting.cpp +++ b/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; } diff --git a/lib/CodeGen/RegAllocLocal.cpp b/lib/CodeGen/RegAllocLocal.cpp index b965dc9f0f6..14346e90ed7 100644 --- a/lib/CodeGen/RegAllocLocal.cpp +++ b/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 = diff --git a/lib/CodeGen/RegAllocSimple.cpp b/lib/CodeGen/RegAllocSimple.cpp index 5e5290ce3e3..447e54cf790 100644 --- a/lib/CodeGen/RegAllocSimple.cpp +++ b/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 diff --git a/lib/CodeGen/RegisterScavenging.cpp b/lib/CodeGen/RegisterScavenging.cpp index 9447ff2c410..944468ea11b 100644 --- a/lib/CodeGen/RegisterScavenging.cpp +++ b/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, 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(); diff --git a/lib/CodeGen/Spiller.cpp b/lib/CodeGen/Spiller.cpp index 7b2a32f1c76..354ff0a6492 100644 --- a/lib/CodeGen/Spiller.cpp +++ b/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); diff --git a/lib/Target/TargetInstrInfo.cpp b/lib/Target/TargetInstrInfo.cpp index 10a5cdb6247..1bdeef40097 100644 --- a/lib/Target/TargetInstrInfo.cpp +++ b/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) { diff --git a/test/CodeGen/ARM/2009-04-09-RegScavengerAsm.ll b/test/CodeGen/ARM/2009-04-09-RegScavengerAsm.ll new file mode 100644 index 00000000000..223fa0f435c --- /dev/null +++ b/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 ; [#uses=2] + %0 = load i32* %rr ; [#uses=1] + %1 = call i32 asm "nop", "=r,0"(i32 %0) nounwind ; [#uses=1] + store i32 %1, i32* %rr + br label %return + +return: ; preds = %entry + ret void +}