diff --git a/llvm/lib/CodeGen/ReachingDefAnalysis.cpp b/llvm/lib/CodeGen/ReachingDefAnalysis.cpp --- a/llvm/lib/CodeGen/ReachingDefAnalysis.cpp +++ b/llvm/lib/CodeGen/ReachingDefAnalysis.cpp @@ -371,6 +371,12 @@ return Def < 0 ? nullptr : getInstFromId(MBB, Def); } +static bool mayHaveSideEffects(MachineInstr &MI) { + return MI.mayLoadOrStore() || MI.mayRaiseFPException() || + MI.hasUnmodeledSideEffects() || MI.isTerminator() || + MI.isCall() || MI.isBarrier() || MI.isBranch() || MI.isReturn(); +} + // Can we safely move 'From' to just before 'To'? To satisfy this, 'From' must // not define a register that is used by any instructions, after and including, // 'To'. These instructions also must not redefine any of Froms operands. @@ -392,10 +398,13 @@ } // Now walk checking that the rest of the instructions will compute the same - // value. + // value and that we're not overwriting anything. Don't move the instruction + // past any memory, control-flow or other ambigious instructions. for (auto I = ++Iterator(From), E = Iterator(To); I != E; ++I) { + if (mayHaveSideEffects(*I)) + return false; for (auto &MO : I->operands()) - if (MO.isReg() && MO.getReg() && MO.isUse() && Defs.count(MO.getReg())) + if (MO.isReg() && MO.getReg() && Defs.count(MO.getReg())) return false; } return true; @@ -430,8 +439,7 @@ InstSet &ToRemove, InstSet &Ignore) const { if (Visited.count(MI) || Ignore.count(MI)) return true; - else if (MI->mayLoadOrStore() || MI->hasUnmodeledSideEffects() || - MI->isBranch() || MI->isTerminator() || MI->isReturn()) { + else if (mayHaveSideEffects(*MI)) { // Unless told to ignore the instruction, don't remove anything which has // side effects. return false; diff --git a/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp b/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp --- a/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp +++ b/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp @@ -390,6 +390,8 @@ // The element count register maybe defined after InsertPt, in which case we // need to try to move either InsertPt or the def so that the [w|d]lstp can // use the value. + // TODO: On failing to move an instruction, check if the count is provided by + // a mov and whether we can use the mov operand directly. MachineBasicBlock *InsertBB = StartInsertPt->getParent(); if (!RDA->isReachingDefLiveOut(StartInsertPt, NumElements)) { if (auto *ElemDef = RDA->getLocalLiveOutMIDef(InsertBB, NumElements)) { diff --git a/llvm/test/CodeGen/Thumb2/LowOverheadLoops/move-def-before-start.mir b/llvm/test/CodeGen/Thumb2/LowOverheadLoops/move-def-before-start.mir --- a/llvm/test/CodeGen/Thumb2/LowOverheadLoops/move-def-before-start.mir +++ b/llvm/test/CodeGen/Thumb2/LowOverheadLoops/move-def-before-start.mir @@ -3,8 +3,6 @@ # Test that, though the vctp operand is defined at the end of the block, # that the correct value is used for the dlstp. -# TODO: The pass currently just bails instead of finding the correct -# value. --- | define dso_local arm_aapcs_vfpcc void @start_before_elems(i32* noalias nocapture %a, i8* nocapture readonly %b, i8* nocapture readonly %c, i32 %N) local_unnamed_addr #0 {