Add missing dependencies to mayHaveNonDefUseDependency
authorPhilip Reames <listmail@philipreames.com>
Mon, 21 Mar 2022 17:15:36 +0000 (10:15 -0700)
committerPhilip Reames <listmail@philipreames.com>
Mon, 21 Mar 2022 17:15:36 +0000 (10:15 -0700)
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.

llvm/lib/Analysis/ValueTracking.cpp

index 021e5d9..67fe15f 100644 (file)
@@ -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.