[PATCH] D74058: [ReachingDefs] Make isSafeToMove more strict.

[Sam Parker via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0a8cae10feb2: [ReachingDefs] Make isSafeToMove more strict. (authored by samparker).

Changed prior to commit:
  https://reviews.llvm.org/D74058?vs=242838&id=242888#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74058/new/

https://reviews.llvm.org/D74058

Files:
  llvm/lib/CodeGen/ReachingDefAnalysis.cpp
  llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
  llvm/test/CodeGen/Thumb2/LowOverheadLoops/move-def-before-start.mir


Index: llvm/test/CodeGen/Thumb2/LowOverheadLoops/move-def-before-start.mir
===================================================================
--- llvm/test/CodeGen/Thumb2/LowOverheadLoops/move-def-before-start.mir
+++ 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 {
Index: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
===================================================================
--- llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp
+++ 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)) {
Index: llvm/lib/CodeGen/ReachingDefAnalysis.cpp
===================================================================
--- llvm/lib/CodeGen/ReachingDefAnalysis.cpp
+++ 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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74058.242888.patch
Type: text/x-patch
Size: 3310 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200206/91e745be/attachment.bin>