[llvm] 4d192f2 - [InstSimplify][CaptureTracking] Reduce scope of special case

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2023-02-21T09:01:55+01:00
New Revision: 4d192f2d2a545c8cd51ef86f9e83209eccbe229e

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

LOG: [InstSimplify][CaptureTracking] Reduce scope of special case

As described in PR54002, the "icmp of load from global" special
case in CaptureTracking is incorrect, as is the InstSimplify code
that makes use of it.

This moves the special case from CaptureTracking into InstSimplify
to limit its scope to the buggy transform only, and not affect
other code using CaptureTracking as well.

Added: 
    

Modified: 
    llvm/lib/Analysis/CaptureTracking.cpp
    llvm/lib/Analysis/InstructionSimplify.cpp
    llvm/test/Transforms/InstCombine/compare-alloca.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/CaptureTracking.cpp b/llvm/lib/Analysis/CaptureTracking.cpp
index 7f3a2b49aca94..34a320435c0a7 100644
--- a/llvm/lib/Analysis/CaptureTracking.cpp
+++ b/llvm/lib/Analysis/CaptureTracking.cpp
@@ -403,12 +403,7 @@ UseCaptureKind llvm::DetermineUseCaptureKind(
           return UseCaptureKind::NO_CAPTURE;
       }
     }
-    // Comparison against value stored in global variable. Given the pointer
-    // does not escape, its value cannot be guessed and stored separately in a
-    // global variable.
-    auto *LI = dyn_cast<LoadInst>(I->getOperand(OtherIdx));
-    if (LI && isa<GlobalVariable>(LI->getPointerOperand()))
-      return UseCaptureKind::NO_CAPTURE;
+
     // Otherwise, be conservative. There are crazy ways to capture pointers
     // using comparisons.
     return UseCaptureKind::MAY_CAPTURE;

diff  --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index afeb80b5ff7a4..0cd51fcac8ced 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -2850,11 +2850,35 @@ static Constant *computePointerICmp(CmpInst::Predicate Pred, Value *LHS,
     else if (isAllocLikeFn(RHS, TLI) &&
              llvm::isKnownNonZero(LHS, DL, 0, nullptr, CxtI, DT))
       MI = RHS;
-    // FIXME: We should also fold the compare when the pointer escapes, but the
-    // compare dominates the pointer escape
-    if (MI && !PointerMayBeCaptured(MI, true, true))
-      return ConstantInt::get(getCompareTy(LHS),
-                              CmpInst::isFalseWhenEqual(Pred));
+    if (MI) {
+      // FIXME: This is incorrect, see PR54002. While we can assume that the
+      // allocation is at an address that makes the comparison false, this
+      // requires that *all* comparisons to that address be false, which
+      // InstSimplify cannot guarantee.
+      struct CustomCaptureTracker : public CaptureTracker {
+        bool Captured = false;
+        void tooManyUses() override { Captured = true; }
+        bool captured(const Use *U) override {
+          if (auto *ICmp = dyn_cast<ICmpInst>(U->getUser())) {
+            // Comparison against value stored in global variable. Given the
+            // pointer does not escape, its value cannot be guessed and stored
+            // separately in a global variable.
+            unsigned OtherIdx = 1 - U->getOperandNo();
+            auto *LI = dyn_cast<LoadInst>(ICmp->getOperand(OtherIdx));
+            if (LI && isa<GlobalVariable>(LI->getPointerOperand()))
+              return false;
+          }
+
+          Captured = true;
+          return true;
+        }
+      };
+      CustomCaptureTracker Tracker;
+      PointerMayBeCaptured(MI, &Tracker);
+      if (!Tracker.Captured)
+        return ConstantInt::get(getCompareTy(LHS),
+                                CmpInst::isFalseWhenEqual(Pred));
+    }
   }
 
   // Otherwise, fail.

diff  --git a/llvm/test/Transforms/InstCombine/compare-alloca.ll b/llvm/test/Transforms/InstCombine/compare-alloca.ll
index 463e6a6192f59..4b5ab1bab9af5 100644
--- a/llvm/test/Transforms/InstCombine/compare-alloca.ll
+++ b/llvm/test/Transforms/InstCombine/compare-alloca.ll
@@ -193,12 +193,14 @@ define void @neg_consistent_fold2() {
   ret void
 }
 
-; FIXME: The end result is correct, but the fold happens for the wrong
-; reason (incorrect icmp GlobalValue special case in CaptureTracking).
-define void @consistent_fold3() {
-; CHECK-LABEL: @consistent_fold3(
+define void @neg_consistent_fold3() {
+; CHECK-LABEL: @neg_consistent_fold3(
+; CHECK-NEXT:    [[M1:%.*]] = alloca [4 x i8], align 1
+; CHECK-NEXT:    [[LGP:%.*]] = load ptr, ptr @gp, align 8
 ; CHECK-NEXT:    [[RHS2:%.*]] = call ptr @hidden_inttoptr()
-; CHECK-NEXT:    call void @witness(i1 false, i1 false)
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp eq ptr [[M1]], [[LGP]]
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp eq ptr [[M1]], [[RHS2]]
+; CHECK-NEXT:    call void @witness(i1 [[CMP1]], i1 [[CMP2]])
 ; CHECK-NEXT:    ret void
 ;
   %m = alloca i8, i32 4