[SCCP] Consider provenance when propagating constant ptrs

[antoniofrighetto]

Similarly to what it is being already done in GVN (fb632ed), make sure pointers equalities derived via PredicatedInfo may be propagated, taking into account their provenance as well.

Fixes: #159565.

[llvmbot]

@llvm/pr-subscribers-function-specialization
@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-analysis

Author: Antonio Frighetto (antoniofrighetto)

Changes

Similarly to what it is being already done in GVN (fb632ed), make sure pointers equalities derived via PredicatedInfo may be propagated, taking into account their provenance as well.

Fixes: #159565.

---

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

4 Files Affected:

• (modified) llvm/lib/Analysis/Loads.cpp (+2-2)
• (modified) llvm/lib/Transforms/Utils/SCCPSolver.cpp (+22-1)
• (modified) llvm/test/Transforms/SCCP/assume.ll (+11)
• (modified) llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll (+2-2)

diff --git a/llvm/lib/Analysis/Loads.cpp b/llvm/lib/Analysis/Loads.cpp
index 0c4e3a2e3b233..bb8724d8af9d1 100644
--- a/llvm/lib/Analysis/Loads.cpp
+++ b/llvm/lib/Analysis/Loads.cpp
@@ -799,7 +799,7 @@ Value *llvm::FindAvailableLoadedValue(LoadInst *Load, BatchAAResults &AA,
 
 // Returns true if a use is either in an ICmp/PtrToInt or a Phi/Select that only
 // feeds into them.
-static bool isPointerUseReplacable(const Use &U) {
+static bool isPointerUseReplaceable(const Use &U) {
   unsigned Limit = 40;
   SmallVector<const User *> Worklist({U.getUser()});
   SmallPtrSet<const User *, 8> Visited;
@@ -847,7 +847,7 @@ bool llvm::canReplacePointersInUseIfEqual(const Use &U, const Value *To,
 
   if (isPointerAlwaysReplaceable(&*U, To, DL))
     return true;
-  return isPointerUseReplacable(U);
+  return isPointerUseReplaceable(U);
 }
 
 bool llvm::canReplacePointersIfEqual(const Value *From, const Value *To,
diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
index af216cd9214bf..27e285fd74bf6 100644
--- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
+++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
@@ -16,6 +16,7 @@
 #include "llvm/ADT/SetVector.h"
 #include "llvm/Analysis/ConstantFolding.h"
 #include "llvm/Analysis/InstructionSimplify.h"
+#include "llvm/Analysis/Loads.h"
 #include "llvm/Analysis/ValueLattice.h"
 #include "llvm/Analysis/ValueLatticeUtils.h"
 #include "llvm/Analysis/ValueTracking.h"
@@ -82,6 +83,26 @@ bool SCCPSolver::tryToReplaceWithConstant(Value *V) {
     return false;
   }
 
+  // Perform constant pointer propagation as long as assuming PredicateInfo
+  // derived equality between the two holds, and their provenance is the same.
+  if (auto *I = dyn_cast<Instruction>(V); I && I->getType()->isPointerTy()) {
+    if (getPredicateInfoFor(I)) {
+      bool MadeChange = false;
+      const auto &DL = I->getDataLayout();
+
+      I->replaceUsesWithIf(Const, [&](Use &U) {
+        bool CanReplace = canReplacePointersInUseIfEqual(U, Const, DL);
+        if (CanReplace)
+          LLVM_DEBUG(dbgs() << "  Constant pointer: " << *Const << " = " << *V
+                            << '\n');
+
+        MadeChange |= CanReplace;
+        return CanReplace;
+      });
+      return MadeChange;
+    }
+  }
+
   LLVM_DEBUG(dbgs() << "  Constant: " << *Const << " = " << *V << '\n');
 
   // Replaces all of the uses of a variable with uses of the constant.
@@ -350,7 +371,7 @@ bool SCCPSolver::simplifyInstsInBlock(BasicBlock &BB,
     if (Inst.getType()->isVoidTy())
       continue;
     if (tryToReplaceWithConstant(&Inst)) {
-      if (wouldInstructionBeTriviallyDead(&Inst))
+      if (isInstructionTriviallyDead(&Inst))
         Inst.eraseFromParent();
 
       MadeChanges = true;
diff --git a/llvm/test/Transforms/SCCP/assume.ll b/llvm/test/Transforms/SCCP/assume.ll
index 9beee934bb509..28df0cc6ce5f1 100644
--- a/llvm/test/Transforms/SCCP/assume.ll
+++ b/llvm/test/Transforms/SCCP/assume.ll
@@ -119,3 +119,14 @@ define void @neg_trunc(i8 %v) {
   call void @use(i1 %c4)
   ret void
 }
+
+define ptr @assume_pointers_equality_maybe_different_provenance(ptr %x) {
+; CHECK-LABEL: @assume_pointers_equality_maybe_different_provenance(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq ptr [[X:%.*]], inttoptr (i64 12345678 to ptr)
+; CHECK-NEXT:    call void @llvm.assume(i1 [[CMP]])
+; CHECK-NEXT:    ret ptr [[X]]
+;
+  %cmp = icmp eq ptr %x, inttoptr (i64 12345678 to ptr)
+  call void @llvm.assume(i1 %cmp)
+  ret ptr %x
+}
diff --git a/llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll b/llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll
index 39af513a4506c..f336fcbe70c77 100644
--- a/llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll
+++ b/llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll
@@ -11,7 +11,7 @@ define i32 @eq_undereferenceable(ptr %p) {
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp eq ptr [[P:%.*]], getelementptr inbounds (i32, ptr @x, i64 1)
 ; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
 ; CHECK:       if.then:
-; CHECK-NEXT:    store i32 2, ptr getelementptr inbounds (i32, ptr @x, i64 1), align 4
+; CHECK-NEXT:    store i32 2, ptr [[P]], align 4
 ; CHECK-NEXT:    br label [[IF_END]]
 ; CHECK:       if.end:
 ; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr @y, align 4
@@ -65,7 +65,7 @@ define i1 @eq_undereferenceable_cmp_simp(ptr %p) {
 ; CHECK-NEXT:    [[CMP_0:%.*]] = icmp eq ptr [[P:%.*]], getelementptr inbounds (i32, ptr @x, i64 1)
 ; CHECK-NEXT:    br i1 [[CMP_0]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
 ; CHECK:       if.then:
-; CHECK-NEXT:    store i32 2, ptr getelementptr inbounds (i32, ptr @x, i64 1), align 4
+; CHECK-NEXT:    store i32 2, ptr [[P]], align 4
 ; CHECK-NEXT:    ret i1 true
 ; CHECK:       if.end:
 ; CHECK-NEXT:    ret i1 false

[nikic]

Won't think not work if there any intermediate copies of the value? Like through a phi/select or in the IPSCCP case a function return value etc.

[antoniofrighetto]

Hm, right. Any suggestions on whether calling getPredicateInfoFor() over instruction operands may be a possibility here? Alternatively, after materializing the new copy and walking the uses to be renamed in PredicateInfo, could keep (and expose) a small map copy -> user's new use be an option?

[antoniofrighetto]

On second thought, I believe there's not much we can do but visit the transitive closure of an instruction's operands, see whether the copy is part of it.

[antoniofrighetto]

(Sorry for extra ping on this, somehow I didn't receive push notification while on draft)