[ValueTracking] Handle chain of single-pred blocks in willNotFreeBetween()

[nikic]

willNotFreeBetween() currently handles the case where both instructions are in the same block, or one is in the single predecessor of the other. This patch extends this to handle a chain of single predecessor blocks.

The budget now applies to all checked instructions, rather than per block. Also increase the budget by a factor of two (which means that new budget interpretation should never regress relative to the previous).

[llvmorg-github-actions]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-analysis

Author: Nikita Popov (nikic)

Changes

willNotFreeBetween() currently handles the case where both instructions are in the same block, or one is in the single predecessor of the other. This patch extends this to handle a chain of single predecessor blocks.

The budget now applies to all checked instructions, rather than per block. Also increase the budget by a factor of two (which means that new budget interpretation should never regress relative to the previous).

---

Full diff: https://github.com/llvm/llvm-project/pull/202308.diff

3 Files Affected:

• (modified) llvm/lib/Analysis/ValueTracking.cpp (+21-15)
• (modified) llvm/test/Transforms/LICM/hoist-deref-load.ll (+1)
• (modified) llvm/test/Transforms/SLPVectorizer/AArch64/masked-loads-side-effects-after-vec.ll (+1)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 1261664c5b986..3efb7c64c78e7 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -93,7 +93,7 @@ static cl::opt<unsigned> DomConditionsMaxUses("dom-conditions-max-uses",
 
 /// Maximum number of instructions to check between assume and context
 /// instruction.
-static constexpr unsigned MaxInstrsToCheckForFree = 16;
+static constexpr unsigned MaxInstrsToCheckForFree = 32;
 
 /// Returns the bitwidth of the given scalar or pointer type. For vector types,
 /// returns the element type's bitwidth.
@@ -704,9 +704,10 @@ bool llvm::isValidAssumeForContext(const Instruction *Inv,
 bool llvm::willNotFreeBetween(const Instruction *Assume,
                               const Instruction *CtxI) {
   // Helper to check if there are any calls in the range that may free memory.
-  auto hasNoFreeInRange = [](auto Range) {
+  unsigned NumChecked = 0;
+  auto hasNoFreeInRange = [&NumChecked](auto Range) {
     for (const auto &[Idx, I] : enumerate(Range)) {
-      if (Idx > MaxInstrsToCheckForFree)
+      if (NumChecked++ > MaxInstrsToCheckForFree)
         return false;
 
       if (auto *CB = dyn_cast<CallBase>(&I)) {
@@ -718,27 +719,32 @@ bool llvm::willNotFreeBetween(const Instruction *Assume,
     return true;
   };
 
-  // Handle cross-block case: CtxI in a successor of Assume's block.
   const BasicBlock *CtxBB = CtxI->getParent();
   const BasicBlock *AssumeBB = Assume->getParent();
   BasicBlock::const_iterator CtxIter = CtxI->getIterator();
-  if (CtxBB != AssumeBB) {
-    if (CtxBB->getSinglePredecessor() != AssumeBB)
+  if (CtxBB == AssumeBB) {
+    // Same block case: check that Assume comes before CtxI.
+    if (Assume != CtxI && !Assume->comesBefore(CtxI))
       return false;
+    return hasNoFreeInRange(make_range(Assume->getIterator(), CtxIter));
+  }
+
+  // Handle chain of single-predecessor blocks.
+  const BasicBlock *CurBB = CtxBB;
+  while (true) {
+    if (CurBB == AssumeBB)
+      return hasNoFreeInRange(
+          make_range(Assume->getIterator(), AssumeBB->end()));
 
-    if (!hasNoFreeInRange(make_range(CtxBB->begin(), CtxIter)))
+    const BasicBlock *PredBB = CurBB->getSinglePredecessor();
+    if (!PredBB)
       return false;
 
-    CtxIter = AssumeBB->end();
-  } else {
-    // Same block case: check that Assume comes before CtxI.
-    if (Assume != CtxI && !Assume->comesBefore(CtxI))
+    if (!hasNoFreeInRange(make_range(CurBB->begin(),
+                                     CurBB == CtxBB ? CtxIter : CurBB->end())))
       return false;
+    CurBB = PredBB;
   }
-
-  // Check if there are any calls between Assume and CtxIter that may free
-  // memory.
-  return hasNoFreeInRange(make_range(Assume->getIterator(), CtxIter));
 }
 
 // TODO: cmpExcludesZero misses many cases where `RHS` is non-constant but
diff --git a/llvm/test/Transforms/LICM/hoist-deref-load.ll b/llvm/test/Transforms/LICM/hoist-deref-load.ll
index c498e85ddd6c2..4ff21a34afa3b 100644
--- a/llvm/test/Transforms/LICM/hoist-deref-load.ll
+++ b/llvm/test/Transforms/LICM/hoist-deref-load.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S -passes=licm -verify-memoryssa < %s | FileCheck %s
+; RUN: opt -S -passes=licm -verify-memoryssa -use-dereferenceable-at-point-semantics < %s | FileCheck %s
 ; RUN: opt -aa-pipeline=basic-aa -passes='require<opt-remark-emit>,loop-mssa(loop-simplifycfg,licm)' -verify-memoryssa -S < %s | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
diff --git a/llvm/test/Transforms/SLPVectorizer/AArch64/masked-loads-side-effects-after-vec.ll b/llvm/test/Transforms/SLPVectorizer/AArch64/masked-loads-side-effects-after-vec.ll
index ca3c8bbac6366..df4c527aa216a 100644
--- a/llvm/test/Transforms/SLPVectorizer/AArch64/masked-loads-side-effects-after-vec.ll
+++ b/llvm/test/Transforms/SLPVectorizer/AArch64/masked-loads-side-effects-after-vec.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
 ; RUN: opt -S --passes=slp-vectorizer -mtriple=aarch64-unknown-linux-gnu < %s | FileCheck %s
+; RUN: opt -S --passes=slp-vectorizer -mtriple=aarch64-unknown-linux-gnu -use-dereferenceable-at-point-semantics < %s | FileCheck %s
 
 declare noalias ptr @malloc()
 

[dtcxzyw]

LGTM

[antoniofrighetto]

LGTM

[fhahn]

LGTM, thanks.

Might be good to add an explicit test to something like llvm/test/Transforms/LoopVectorize/dereferenceable-info-from-assumption-constant-size.ll