[llvm] b880bde - Add missing dependencies to mayHaveNonDefUseDependency

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2022-03-21T10:15:36-07:00
New Revision: b880bde92b4a95c7b73dfbab8f04c90704381898

URL: https://github.com/llvm/llvm-project/commit/b880bde92b4a95c7b73dfbab8f04c90704381898
DIFF: https://github.com/llvm/llvm-project/commit/b880bde92b4a95c7b73dfbab8f04c90704381898.diff

LOG: Add missing dependencies to mayHaveNonDefUseDependency

Two interesting ommissions:
* When reordering in either direction, reordering two calls which both
  contain inf-loops is illegal.  This one is possibly a change in behavior
  for certain callers (e.g. fixes a latent bug.)
* When moving down, control dependence must be respected by checking the
  inverse of isSafeToSpeculativeExecute.  Current callers all seem to
  handle this case - though admitted, I did not do an exhaustive audit.
  Most seem to be only interested in moving upwards within a block.  This
  is mostly a case of future proofing an API so that it implements what
  the comments says, not just what current callers need.

Noticed via inspection.  I don't have a test case.

Added: 
    

Modified: 
    llvm/lib/Analysis/ValueTracking.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 021e5d907aa1a..67fe15fd23b36 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -4644,7 +4644,19 @@ bool llvm::isSafeToSpeculativelyExecute(const Value *V,
 }
 
 bool llvm::mayHaveNonDefUseDependency(const Instruction &I) {
-  return I.mayReadOrWriteMemory() || !isSafeToSpeculativelyExecute(&I);
+  if (I.mayReadOrWriteMemory())
+    // Memory dependency possible
+    return true;
+  if (!isSafeToSpeculativelyExecute(&I))
+    // Can't move above a maythrow call or infinite loop.  Or if an
+    // inalloca alloca, above a stacksave call.
+    return true;
+  if (!isGuaranteedToTransferExecutionToSuccessor(&I))
+    // 1) Can't reorder two inf-loop calls, even if readonly
+    // 2) Also can't reorder an inf-loop call below a instruction which isn't
+    //    safe to speculative execute.  (Inverse of above)
+    return true;
+  return false;
 }
 
 /// Convert ConstantRange OverflowResult into ValueTracking OverflowResult.