[llvm] 0a8cae1 - [ReachingDefs] Make isSafeToMove more strict.
Sam Parker via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 6 06:06:26 PST 2020
Author: Sam Parker
Date: 2020-02-06T14:06:08Z
New Revision: 0a8cae10feb283b7a0886c5b52055de3db8b0e10
URL: https://github.com/llvm/llvm-project/commit/0a8cae10feb283b7a0886c5b52055de3db8b0e10
DIFF: https://github.com/llvm/llvm-project/commit/0a8cae10feb283b7a0886c5b52055de3db8b0e10.diff
LOG: [ReachingDefs] Make isSafeToMove more strict.
Test that we're not moving the instruction through instructions with
side-effects.
Differential Revision: https://reviews.llvm.org/D74058
Added:
Modified:
llvm/lib/CodeGen/ReachingDefAnalysis.cpp
llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
llvm/test/CodeGen/Thumb2/LowOverheadLoops/move-def-before-start.mir
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/ReachingDefAnalysis.cpp b/llvm/lib/CodeGen/ReachingDefAnalysis.cpp
index 4483a85cf8fb..62adaaf419df 100644
--- a/llvm/lib/CodeGen/ReachingDefAnalysis.cpp
+++ b/llvm/lib/CodeGen/ReachingDefAnalysis.cpp
@@ -371,6 +371,12 @@ MachineInstr* ReachingDefAnalysis::getLocalLiveOutMIDef(MachineBasicBlock *MBB,
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 @@ bool ReachingDefAnalysis::isSafeToMove(MachineInstr *From,
}
// 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 @@ ReachingDefAnalysis::isSafeToRemove(MachineInstr *MI, InstSet &Visited,
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
index 7fc5e1297ae2..cdd1cb2efc1c 100644
--- a/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
+++ b/llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
@@ -390,6 +390,8 @@ bool LowOverheadLoop::ValidateTailPredicate(MachineInstr *StartInsertPt) {
// 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
index 8297f830bc5b..165452b534d2 100644
--- 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 {
More information about the llvm-commits
mailing list