From e67bd6c248e21e9b55a7d86b8ccfaef1ed70bf8a Mon Sep 17 00:00:00 2001 From: Matthias Braun Date: Fri, 29 May 2015 02:56:46 +0000 Subject: [PATCH] CodeGen: Use mop_iterator instead of MIOperands/ConstMIOperands MIOperands/ConstMIOperands are classes iterating over the MachineOperand of a MachineInstr, however MachineInstr::mop_iterator does the same thing. I assume these two iterators exist to have a uniform interface to iterate over the operands of a machine instruction bundle and a single machine instruction. However in practice I find it more confusing to have 2 different iterator classes, so this patch transforms (nearly all) the code to use mop_iterators. The only exception being MIOperands::anlayzePhysReg() and MIOperands::analyzeVirtReg() still needing an equivalent, I leave that as an exercise for the next patch. Differential Revision: http://reviews.llvm.org/D9932 This version is slightly modified from the proposed revision in that it introduces MachineInstr::getOperandNo to avoid the extra counting variable in the few loops that previously used MIOperands::getOperandNo. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@238539 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/CodeGen/MachineInstr.h | 5 +++ lib/CodeGen/EarlyIfConversion.cpp | 22 ++++++------- lib/CodeGen/LiveIntervalAnalysis.cpp | 20 ++++++------ lib/CodeGen/MachineInstr.cpp | 5 ++- lib/CodeGen/MachineLICM.cpp | 6 ++-- lib/CodeGen/MachineTraceMetrics.cpp | 49 ++++++++++++++++------------- lib/CodeGen/ProcessImplicitDefs.cpp | 14 ++++----- lib/CodeGen/RegisterCoalescer.cpp | 26 +++++++-------- lib/CodeGen/ScheduleDAGInstrs.cpp | 12 +++---- lib/Target/ARM/ARMLoadStoreOptimizer.cpp | 6 ++-- lib/Target/ARM/Thumb2ITBlockPass.cpp | 8 ++--- lib/Target/Hexagon/HexagonFrameLowering.cpp | 8 ++--- lib/Target/X86/X86FastISel.cpp | 14 +++++---- 13 files changed, 104 insertions(+), 91 deletions(-) diff --git a/include/llvm/CodeGen/MachineInstr.h b/include/llvm/CodeGen/MachineInstr.h index d04ea567bbc..564b8da37dc 100644 --- a/include/llvm/CodeGen/MachineInstr.h +++ b/include/llvm/CodeGen/MachineInstr.h @@ -331,6 +331,11 @@ public: operands_begin() + getDesc().getNumDefs(), operands_end()); } + /// Returns the number of the operand iterator \p I points to. + unsigned getOperandNo(const_mop_iterator I) const { + return I - operands_begin(); + } + /// Access to memory operands of the instruction mmo_iterator memoperands_begin() const { return MemRefs; } mmo_iterator memoperands_end() const { return MemRefs + NumMemRefs; } diff --git a/lib/CodeGen/EarlyIfConversion.cpp b/lib/CodeGen/EarlyIfConversion.cpp index 092b7f804e4..99509513761 100644 --- a/lib/CodeGen/EarlyIfConversion.cpp +++ b/lib/CodeGen/EarlyIfConversion.cpp @@ -226,21 +226,21 @@ bool SSAIfConv::canSpeculateInstrs(MachineBasicBlock *MBB) { } // Check for any dependencies on Head instructions. - for (MIOperands MO(I); MO.isValid(); ++MO) { - if (MO->isRegMask()) { + for (const MachineOperand MO : I->operands()) { + if (MO.isRegMask()) { DEBUG(dbgs() << "Won't speculate regmask: " << *I); return false; } - if (!MO->isReg()) + if (!MO.isReg()) continue; - unsigned Reg = MO->getReg(); + unsigned Reg = MO.getReg(); // Remember clobbered regunits. - if (MO->isDef() && TargetRegisterInfo::isPhysicalRegister(Reg)) + if (MO.isDef() && TargetRegisterInfo::isPhysicalRegister(Reg)) for (MCRegUnitIterator Units(Reg, TRI); Units.isValid(); ++Units) ClobberedRegUnits.set(*Units); - if (!MO->readsReg() || !TargetRegisterInfo::isVirtualRegister(Reg)) + if (!MO.readsReg() || !TargetRegisterInfo::isVirtualRegister(Reg)) continue; MachineInstr *DefMI = MRI->getVRegDef(Reg); if (!DefMI || DefMI->getParent() != Head) @@ -284,19 +284,19 @@ bool SSAIfConv::findInsertionPoint() { } // Update live regunits. - for (MIOperands MO(I); MO.isValid(); ++MO) { + for (const MachineOperand &MO : I->operands()) { // We're ignoring regmask operands. That is conservatively correct. - if (!MO->isReg()) + if (!MO.isReg()) continue; - unsigned Reg = MO->getReg(); + unsigned Reg = MO.getReg(); if (!TargetRegisterInfo::isPhysicalRegister(Reg)) continue; // I clobbers Reg, so it isn't live before I. - if (MO->isDef()) + if (MO.isDef()) for (MCRegUnitIterator Units(Reg, TRI); Units.isValid(); ++Units) LiveRegUnits.erase(*Units); // Unless I reads Reg. - if (MO->readsReg()) + if (MO.readsReg()) Reads.push_back(Reg); } // Anything read by I is live before I. diff --git a/lib/CodeGen/LiveIntervalAnalysis.cpp b/lib/CodeGen/LiveIntervalAnalysis.cpp index adca4cc738e..c00b010e763 100644 --- a/lib/CodeGen/LiveIntervalAnalysis.cpp +++ b/lib/CodeGen/LiveIntervalAnalysis.cpp @@ -223,11 +223,11 @@ void LiveIntervals::computeRegMasks() { RMB.first = RegMaskSlots.size(); for (MachineBasicBlock::iterator MI = MBB->begin(), ME = MBB->end(); MI != ME; ++MI) - for (MIOperands MO(MI); MO.isValid(); ++MO) { - if (!MO->isRegMask()) + for (const MachineOperand &MO : MI->operands()) { + if (!MO.isRegMask()) continue; RegMaskSlots.push_back(Indexes->getInstructionIndex(MI).getRegSlot()); - RegMaskBits.push_back(MO->getRegMask()); + RegMaskBits.push_back(MO.getRegMask()); } // Compute the number of register mask instructions in this block. RMB.second = RegMaskSlots.size() - RMB.first; @@ -927,23 +927,23 @@ public: void updateAllRanges(MachineInstr *MI) { DEBUG(dbgs() << "handleMove " << OldIdx << " -> " << NewIdx << ": " << *MI); bool hasRegMask = false; - for (MIOperands MO(MI); MO.isValid(); ++MO) { - if (MO->isRegMask()) + for (MachineOperand &MO : MI->operands()) { + if (MO.isRegMask()) hasRegMask = true; - if (!MO->isReg()) + if (!MO.isReg()) continue; // Aggressively clear all kill flags. // They are reinserted by VirtRegRewriter. - if (MO->isUse()) - MO->setIsKill(false); + if (MO.isUse()) + MO.setIsKill(false); - unsigned Reg = MO->getReg(); + unsigned Reg = MO.getReg(); if (!Reg) continue; if (TargetRegisterInfo::isVirtualRegister(Reg)) { LiveInterval &LI = LIS.getInterval(Reg); if (LI.hasSubRanges()) { - unsigned SubReg = MO->getSubReg(); + unsigned SubReg = MO.getSubReg(); unsigned LaneMask = TRI.getSubRegIndexLaneMask(SubReg); for (LiveInterval::SubRange &S : LI.subranges()) { if ((S.LaneMask & LaneMask) == 0) diff --git a/lib/CodeGen/MachineInstr.cpp b/lib/CodeGen/MachineInstr.cpp index 205032f9297..e67102865bf 100644 --- a/lib/CodeGen/MachineInstr.cpp +++ b/lib/CodeGen/MachineInstr.cpp @@ -1092,9 +1092,8 @@ const TargetRegisterClass *MachineInstr::getRegClassConstraintEffectForVReg( OpndIt.getOperandNo(), Reg, CurRC, TII, TRI); else // Otherwise, just check the current operands. - for (ConstMIOperands OpndIt(this); OpndIt.isValid() && CurRC; ++OpndIt) - CurRC = getRegClassConstraintEffectForVRegImpl(OpndIt.getOperandNo(), Reg, - CurRC, TII, TRI); + for (unsigned i = 0, e = NumOperands; i < e && CurRC; ++i) + CurRC = getRegClassConstraintEffectForVRegImpl(i, Reg, CurRC, TII, TRI); return CurRC; } diff --git a/lib/CodeGen/MachineLICM.cpp b/lib/CodeGen/MachineLICM.cpp index 3967a2fbf8c..cce590c6dc5 100644 --- a/lib/CodeGen/MachineLICM.cpp +++ b/lib/CodeGen/MachineLICM.cpp @@ -1012,10 +1012,10 @@ bool MachineLICM::HasLoopPHIUse(const MachineInstr *MI) const { SmallVector Work(1, MI); do { MI = Work.pop_back_val(); - for (ConstMIOperands MO(MI); MO.isValid(); ++MO) { - if (!MO->isReg() || !MO->isDef()) + for (const MachineOperand &MO : MI->operands()) { + if (!MO.isReg() || !MO.isDef()) continue; - unsigned Reg = MO->getReg(); + unsigned Reg = MO.getReg(); if (!TargetRegisterInfo::isVirtualRegister(Reg)) continue; for (MachineInstr &UseMI : MRI->use_instructions(Reg)) { diff --git a/lib/CodeGen/MachineTraceMetrics.cpp b/lib/CodeGen/MachineTraceMetrics.cpp index e07250bf4f3..34ac9d5b0ed 100644 --- a/lib/CodeGen/MachineTraceMetrics.cpp +++ b/lib/CodeGen/MachineTraceMetrics.cpp @@ -627,10 +627,12 @@ static bool getDataDeps(const MachineInstr *UseMI, SmallVectorImpl &Deps, const MachineRegisterInfo *MRI) { bool HasPhysRegs = false; - for (ConstMIOperands MO(UseMI); MO.isValid(); ++MO) { - if (!MO->isReg()) + for (MachineInstr::const_mop_iterator I = UseMI->operands_begin(), + E = UseMI->operands_end(); I != E; ++I) { + const MachineOperand &MO = *I; + if (!MO.isReg()) continue; - unsigned Reg = MO->getReg(); + unsigned Reg = MO.getReg(); if (!Reg) continue; if (TargetRegisterInfo::isPhysicalRegister(Reg)) { @@ -638,8 +640,8 @@ static bool getDataDeps(const MachineInstr *UseMI, continue; } // Collect virtual register reads. - if (MO->readsReg()) - Deps.push_back(DataDep(MRI, Reg, MO.getOperandNo())); + if (MO.readsReg()) + Deps.push_back(DataDep(MRI, Reg, UseMI->getOperandNo(I))); } return HasPhysRegs; } @@ -690,28 +692,30 @@ static void updatePhysDepsDownwards(const MachineInstr *UseMI, SmallVector Kills; SmallVector LiveDefOps; - for (ConstMIOperands MO(UseMI); MO.isValid(); ++MO) { - if (!MO->isReg()) + for (MachineInstr::const_mop_iterator MI = UseMI->operands_begin(), + ME = UseMI->operands_end(); MI != ME; ++MI) { + const MachineOperand &MO = *MI; + if (!MO.isReg()) continue; - unsigned Reg = MO->getReg(); + unsigned Reg = MO.getReg(); if (!TargetRegisterInfo::isPhysicalRegister(Reg)) continue; // Track live defs and kills for updating RegUnits. - if (MO->isDef()) { - if (MO->isDead()) + if (MO.isDef()) { + if (MO.isDead()) Kills.push_back(Reg); else - LiveDefOps.push_back(MO.getOperandNo()); - } else if (MO->isKill()) + LiveDefOps.push_back(UseMI->getOperandNo(MI)); + } else if (MO.isKill()) Kills.push_back(Reg); // Identify dependencies. - if (!MO->readsReg()) + if (!MO.readsReg()) continue; for (MCRegUnitIterator Units(Reg, TRI); Units.isValid(); ++Units) { SparseSet::iterator I = RegUnits.find(*Units); if (I == RegUnits.end()) continue; - Deps.push_back(DataDep(I->MI, I->Op, MO.getOperandNo())); + Deps.push_back(DataDep(I->MI, I->Op, UseMI->getOperandNo(MI))); break; } } @@ -864,15 +868,18 @@ static unsigned updatePhysDepsUpwards(const MachineInstr *MI, unsigned Height, const TargetInstrInfo *TII, const TargetRegisterInfo *TRI) { SmallVector ReadOps; - for (ConstMIOperands MO(MI); MO.isValid(); ++MO) { - if (!MO->isReg()) + + for (MachineInstr::const_mop_iterator MOI = MI->operands_begin(), + MOE = MI->operands_end(); MOI != MOE; ++MOI) { + const MachineOperand &MO = *MOI; + if (!MO.isReg()) continue; - unsigned Reg = MO->getReg(); + unsigned Reg = MO.getReg(); if (!TargetRegisterInfo::isPhysicalRegister(Reg)) continue; - if (MO->readsReg()) - ReadOps.push_back(MO.getOperandNo()); - if (!MO->isDef()) + if (MO.readsReg()) + ReadOps.push_back(MI->getOperandNo(MOI)); + if (!MO.isDef()) continue; // This is a def of Reg. Remove corresponding entries from RegUnits, and // update MI Height to consider the physreg dependencies. @@ -885,7 +892,7 @@ static unsigned updatePhysDepsUpwards(const MachineInstr *MI, unsigned Height, // We may not know the UseMI of this dependency, if it came from the // live-in list. SchedModel can handle a NULL UseMI. DepHeight += SchedModel - .computeOperandLatency(MI, MO.getOperandNo(), I->MI, I->Op); + .computeOperandLatency(MI, MI->getOperandNo(MOI), I->MI, I->Op); } Height = std::max(Height, DepHeight); // This regunit is dead above MI. diff --git a/lib/CodeGen/ProcessImplicitDefs.cpp b/lib/CodeGen/ProcessImplicitDefs.cpp index b1538006e72..5f819498348 100644 --- a/lib/CodeGen/ProcessImplicitDefs.cpp +++ b/lib/CodeGen/ProcessImplicitDefs.cpp @@ -68,8 +68,8 @@ bool ProcessImplicitDefs::canTurnIntoImplicitDef(MachineInstr *MI) { !MI->isRegSequence() && !MI->isPHI()) return false; - for (MIOperands MO(MI); MO.isValid(); ++MO) - if (MO->isReg() && MO->isUse() && MO->readsReg()) + for (const MachineOperand &MO : MI->operands()) + if (MO.isReg() && MO.isUse() && MO.readsReg()) return false; return true; } @@ -100,17 +100,17 @@ void ProcessImplicitDefs::processImplicitDef(MachineInstr *MI) { MachineBasicBlock::instr_iterator UserE = MI->getParent()->instr_end(); bool Found = false; for (++UserMI; UserMI != UserE; ++UserMI) { - for (MIOperands MO(UserMI); MO.isValid(); ++MO) { - if (!MO->isReg()) + for (MachineOperand &MO : UserMI->operands()) { + if (!MO.isReg()) continue; - unsigned UserReg = MO->getReg(); + unsigned UserReg = MO.getReg(); if (!TargetRegisterInfo::isPhysicalRegister(UserReg) || !TRI->regsOverlap(Reg, UserReg)) continue; // UserMI uses or redefines Reg. Set flags on all uses. Found = true; - if (MO->isUse()) - MO->setIsUndef(); + if (MO.isUse()) + MO.setIsUndef(); } if (Found) break; diff --git a/lib/CodeGen/RegisterCoalescer.cpp b/lib/CodeGen/RegisterCoalescer.cpp index ac7d98f258b..e513a4f1ccf 100644 --- a/lib/CodeGen/RegisterCoalescer.cpp +++ b/lib/CodeGen/RegisterCoalescer.cpp @@ -1834,12 +1834,12 @@ public: unsigned JoinVals::computeWriteLanes(const MachineInstr *DefMI, bool &Redef) const { unsigned L = 0; - for (ConstMIOperands MO(DefMI); MO.isValid(); ++MO) { - if (!MO->isReg() || MO->getReg() != Reg || !MO->isDef()) + for (const MachineOperand &MO : DefMI->operands()) { + if (!MO.isReg() || MO.getReg() != Reg || !MO.isDef()) continue; L |= TRI->getSubRegIndexLaneMask( - TRI->composeSubRegIndices(SubIdx, MO->getSubReg())); - if (MO->readsReg()) + TRI->composeSubRegIndices(SubIdx, MO.getSubReg())); + if (MO.readsReg()) Redef = true; } return L; @@ -2224,13 +2224,13 @@ bool JoinVals::usesLanes(const MachineInstr *MI, unsigned Reg, unsigned SubIdx, unsigned Lanes) const { if (MI->isDebugValue()) return false; - for (ConstMIOperands MO(MI); MO.isValid(); ++MO) { - if (!MO->isReg() || MO->isDef() || MO->getReg() != Reg) + for (const MachineOperand &MO : MI->operands()) { + if (!MO.isReg() || MO.isDef() || MO.getReg() != Reg) continue; - if (!MO->readsReg()) + if (!MO.readsReg()) continue; if (Lanes & TRI->getSubRegIndexLaneMask( - TRI->composeSubRegIndices(SubIdx, MO->getSubReg()))) + TRI->composeSubRegIndices(SubIdx, MO.getSubReg()))) return true; } return false; @@ -2339,11 +2339,11 @@ void JoinVals::pruneValues(JoinVals &Other, // Remove flags. This def is now a partial redef. // Also remove flags since the joined live range will // continue past this instruction. - for (MIOperands MO(Indexes->getInstructionFromIndex(Def)); - MO.isValid(); ++MO) { - if (MO->isReg() && MO->isDef() && MO->getReg() == Reg) { - MO->setIsUndef(EraseImpDef); - MO->setIsDead(false); + for (MachineOperand &MO : + Indexes->getInstructionFromIndex(Def)->operands()) { + if (MO.isReg() && MO.isDef() && MO.getReg() == Reg) { + MO.setIsUndef(EraseImpDef); + MO.setIsDead(false); } } } diff --git a/lib/CodeGen/ScheduleDAGInstrs.cpp b/lib/CodeGen/ScheduleDAGInstrs.cpp index c60c5183b42..e8e47b764dd 100644 --- a/lib/CodeGen/ScheduleDAGInstrs.cpp +++ b/lib/CodeGen/ScheduleDAGInstrs.cpp @@ -1106,25 +1106,25 @@ static void toggleBundleKillFlag(MachineInstr *MI, unsigned Reg, MachineBasicBlock::instr_iterator Begin = MI; MachineBasicBlock::instr_iterator End = getBundleEnd(MI); while (Begin != End) { - for (MIOperands MO(--End); MO.isValid(); ++MO) { - if (!MO->isReg() || MO->isDef() || Reg != MO->getReg()) + for (MachineOperand &MO : (--End)->operands()) { + if (!MO.isReg() || MO.isDef() || Reg != MO.getReg()) continue; // DEBUG_VALUE nodes do not contribute to code generation and should // always be ignored. Failure to do so may result in trying to modify // KILL flags on DEBUG_VALUE nodes, which is distressing. - if (MO->isDebug()) + if (MO.isDebug()) continue; // If the register has the internal flag then it could be killing an // internal def of the register. In this case, just skip. We only want // to toggle the flag on operands visible outside the bundle. - if (MO->isInternalRead()) + if (MO.isInternalRead()) continue; - if (MO->isKill() == NewKillState) + if (MO.isKill() == NewKillState) continue; - MO->setIsKill(NewKillState); + MO.setIsKill(NewKillState); if (NewKillState) return; } diff --git a/lib/Target/ARM/ARMLoadStoreOptimizer.cpp b/lib/Target/ARM/ARMLoadStoreOptimizer.cpp index 5b62a21706c..019261edb5b 100644 --- a/lib/Target/ARM/ARMLoadStoreOptimizer.cpp +++ b/lib/Target/ARM/ARMLoadStoreOptimizer.cpp @@ -762,10 +762,10 @@ void ARMLoadStoreOpt::MergeOpsUpdate(MachineBasicBlock &MBB, Regs.push_back(std::make_pair(Reg, isKill)); // Collect any implicit defs of super-registers. They must be preserved. - for (MIOperands MO(memOps[i].MBBI); MO.isValid(); ++MO) { - if (!MO->isReg() || !MO->isDef() || !MO->isImplicit() || MO->isDead()) + for (const MachineOperand &MO : memOps[i].MBBI->operands()) { + if (!MO.isReg() || !MO.isDef() || !MO.isImplicit() || MO.isDead()) continue; - unsigned DefReg = MO->getReg(); + unsigned DefReg = MO.getReg(); if (std::find(ImpDefs.begin(), ImpDefs.end(), DefReg) == ImpDefs.end()) ImpDefs.push_back(DefReg); diff --git a/lib/Target/ARM/Thumb2ITBlockPass.cpp b/lib/Target/ARM/Thumb2ITBlockPass.cpp index b62ae2e3429..68736bc1dec 100644 --- a/lib/Target/ARM/Thumb2ITBlockPass.cpp +++ b/lib/Target/ARM/Thumb2ITBlockPass.cpp @@ -94,12 +94,12 @@ static void TrackDefUses(MachineInstr *MI, /// conservatively remove more kill flags than are necessary, but removing them /// is safer than incorrect kill flags remaining on instructions. static void ClearKillFlags(MachineInstr *MI, SmallSet &Uses) { - for (MIOperands MO(MI); MO.isValid(); ++MO) { - if (!MO->isReg() || MO->isDef() || !MO->isKill()) + for (MachineOperand &MO : MI->operands()) { + if (!MO.isReg() || MO.isDef() || !MO.isKill()) continue; - if (!Uses.count(MO->getReg())) + if (!Uses.count(MO.getReg())) continue; - MO->setIsKill(false); + MO.setIsKill(false); } } diff --git a/lib/Target/Hexagon/HexagonFrameLowering.cpp b/lib/Target/Hexagon/HexagonFrameLowering.cpp index 0885a794a7b..868f87e1841 100644 --- a/lib/Target/Hexagon/HexagonFrameLowering.cpp +++ b/lib/Target/Hexagon/HexagonFrameLowering.cpp @@ -201,17 +201,17 @@ namespace { break; } // Check individual operands. - for (ConstMIOperands Mo(MI); Mo.isValid(); ++Mo) { + for (const MachineOperand &MO : MI->operands()) { // While the presence of a frame index does not prove that a stack // frame will be required, all frame indexes should be within alloc- // frame/deallocframe. Otherwise, the code that translates a frame // index into an offset would have to be aware of the placement of // the frame creation/destruction instructions. - if (Mo->isFI()) + if (MO.isFI()) return true; - if (!Mo->isReg()) + if (!MO.isReg()) continue; - unsigned R = Mo->getReg(); + unsigned R = MO.getReg(); // Virtual registers will need scavenging, which then may require // a stack slot. if (TargetRegisterInfo::isVirtualRegister(R)) diff --git a/lib/Target/X86/X86FastISel.cpp b/lib/Target/X86/X86FastISel.cpp index 9af0aebea23..d1d053ae36b 100644 --- a/lib/Target/X86/X86FastISel.cpp +++ b/lib/Target/X86/X86FastISel.cpp @@ -3541,16 +3541,18 @@ bool X86FastISel::tryToFoldLoadIntoMI(MachineInstr *MI, unsigned OpNo, // to just look at OpNo + the offset to the index reg. We actually need to // scan the instruction to find the index reg and see if its the correct reg // class. - for (MIOperands MO(Result); MO.isValid(); ++MO) { - if (!MO->isReg() || MO->isDef() || MO->getReg() != AM.IndexReg) + unsigned OperandNo = 0; + for (MachineInstr::mop_iterator I = Result->operands_begin(), + E = Result->operands_end(); I != E; ++I, ++OperandNo) { + MachineOperand &MO = *I; + if (!MO.isReg() || MO.isDef() || MO.getReg() != AM.IndexReg) continue; // Found the index reg, now try to rewrite it. - unsigned OpNo = MO.getOperandNo(); unsigned IndexReg = constrainOperandRegClass(Result->getDesc(), - MO->getReg(), OpNo); - if (IndexReg == MO->getReg()) + MO.getReg(), OperandNo); + if (IndexReg == MO.getReg()) continue; - MO->setReg(IndexReg); + MO.setReg(IndexReg); } Result->addMemOperand(*FuncInfo.MF, createMachineMemOperandFor(LI)); -- 2.11.0