[ValueTracking] Use `SimplifyQuery` in `isKnownNonEqual` #124942

dtcxzyw · 2025-01-29T16:38:04Z

It is needed by #117442.

llvmbot · 2025-01-29T16:38:41Z

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-analysis

Author: Yingwei Zheng (dtcxzyw)

Changes

It is needed by #117442.

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

6 Files Affected:

(modified) llvm/include/llvm/Analysis/ValueTracking.h (+2-5)
(modified) llvm/lib/Analysis/BasicAliasAnalysis.cpp (+2-2)
(modified) llvm/lib/Analysis/InstructionSimplify.cpp (+1-1)
(modified) llvm/lib/Analysis/ValueTracking.cpp (+3-6)
(modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+1-1)
(modified) llvm/test/Transforms/InstCombine/known-bits.ll (+1-5)

diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
index b4918c2d1e8a18..dba54be4c92f83 100644
--- a/llvm/include/llvm/Analysis/ValueTracking.h
+++ b/llvm/include/llvm/Analysis/ValueTracking.h
@@ -164,11 +164,8 @@ bool isKnownNegative(const Value *V, const SimplifyQuery &SQ,
 
 /// Return true if the given values are known to be non-equal when defined.
 /// Supports scalar integer types only.
-bool isKnownNonEqual(const Value *V1, const Value *V2, const DataLayout &DL,
-                     AssumptionCache *AC = nullptr,
-                     const Instruction *CxtI = nullptr,
-                     const DominatorTree *DT = nullptr,
-                     bool UseInstrInfo = true);
+bool isKnownNonEqual(const Value *V1, const Value *V2, const SimplifyQuery &SQ,
+                     unsigned Depth = 0);
 
 /// Return true if 'V & Mask' is known to be zero. We use this predicate to
 /// simplify operations downstream. Mask is known to be zero for bits that V
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index b2a3f3390e000d..893a8f7c1207b4 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1349,8 +1349,8 @@ AliasResult BasicAAResult::aliasGEP(
     const VariableGEPIndex &Var1 = DecompGEP1.VarIndices[1];
     if (Var0.hasNegatedScaleOf(Var1) && Var0.Val.TruncBits == 0 &&
         Var0.Val.hasSameCastsAs(Var1.Val) && !AAQI.MayBeCrossIteration &&
-        isKnownNonEqual(Var0.Val.V, Var1.Val.V, DL, &AC, /* CxtI */ nullptr,
-                        DT))
+        isKnownNonEqual(Var0.Val.V, Var1.Val.V,
+                        SimplifyQuery(DL, DT, &AC, /*CxtI=*/nullptr)))
       MinAbsVarIndex = Var0.Scale.abs();
   }
 
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index d69747e30f884d..f76bfa9c5d58c5 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -3994,7 +3994,7 @@ static Value *simplifyICmpInst(CmpPredicate Pred, Value *LHS, Value *RHS,
   // This is potentially expensive, and we have already computedKnownBits for
   // compares with 0 above here, so only try this for a non-zero compare.
   if (ICmpInst::isEquality(Pred) && !match(RHS, m_Zero()) &&
-      isKnownNonEqual(LHS, RHS, Q.DL, Q.AC, Q.CxtI, Q.DT, Q.IIQ.UseInstrInfo)) {
+      isKnownNonEqual(LHS, RHS, Q)) {
     return Pred == ICmpInst::ICMP_NE ? getTrue(ITy) : getFalse(ITy);
   }
 
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index b63a0a07f7de29..649581741e0e24 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -316,18 +316,15 @@ static bool isKnownNonEqual(const Value *V1, const Value *V2,
                             const SimplifyQuery &Q);
 
 bool llvm::isKnownNonEqual(const Value *V1, const Value *V2,
-                           const DataLayout &DL, AssumptionCache *AC,
-                           const Instruction *CxtI, const DominatorTree *DT,
-                           bool UseInstrInfo) {
+                           const SimplifyQuery &Q, unsigned Depth) {
   // We don't support looking through casts.
   if (V1 == V2 || V1->getType() != V2->getType())
     return false;
   auto *FVTy = dyn_cast<FixedVectorType>(V1->getType());
   APInt DemandedElts =
       FVTy ? APInt::getAllOnes(FVTy->getNumElements()) : APInt(1, 1);
-  return ::isKnownNonEqual(
-      V1, V2, DemandedElts, 0,
-      SimplifyQuery(DL, DT, AC, safeCxtI(V2, V1, CxtI), UseInstrInfo));
+  return ::isKnownNonEqual(V1, V2, DemandedElts, Depth,
+                           Q.getWithInstruction(safeCxtI(V2, V1, Q.CxtI)));
 }
 
 bool llvm::MaskedValueIsZero(const Value *V, const APInt &Mask,
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 5a4791870ac77b..a7e70cf09d2393 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -5412,7 +5412,7 @@ Instruction *InstCombinerImpl::foldICmpBinOp(ICmpInst &I,
         if (ICmpInst::isEquality(Pred)) {
           // If X != Y, fold (X *nw Z) eq/ne (Y *nw Z) -> Z eq/ne 0
           if (((Op0HasNSW && Op1HasNSW) || (Op0HasNUW && Op1HasNUW)) &&
-              isKnownNonEqual(X, Y, DL, &AC, &I, &DT))
+              isKnownNonEqual(X, Y, SQ))
             return new ICmpInst(Pred, Z, Constant::getNullValue(Z->getType()));
 
           KnownBits ZKnown = computeKnownBits(Z, 0, &I);
diff --git a/llvm/test/Transforms/InstCombine/known-bits.ll b/llvm/test/Transforms/InstCombine/known-bits.ll
index 9467f507cd6306..bd6b2f015145e5 100644
--- a/llvm/test/Transforms/InstCombine/known-bits.ll
+++ b/llvm/test/Transforms/InstCombine/known-bits.ll
@@ -1609,17 +1609,13 @@ if.else:
   ret i16 0
 }
 
-; TODO: %cmp always evaluates to false
-
 define i1 @test_simplify_icmp2(double %x) {
 ; CHECK-LABEL: @test_simplify_icmp2(
 ; CHECK-NEXT:    [[ABS:%.*]] = tail call double @llvm.fabs.f64(double [[X:%.*]])
 ; CHECK-NEXT:    [[COND:%.*]] = fcmp oeq double [[ABS]], 0x7FF0000000000000
 ; CHECK-NEXT:    br i1 [[COND]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
 ; CHECK:       if.then:
-; CHECK-NEXT:    [[CAST:%.*]] = bitcast double [[X]] to i64
-; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i64 [[CAST]], 3458764513820540928
-; CHECK-NEXT:    ret i1 [[CMP]]
+; CHECK-NEXT:    ret i1 false
 ; CHECK:       if.else:
 ; CHECK-NEXT:    ret i1 false
 ;

nikic · 2025-01-29T16:49:49Z

llvm/lib/Analysis/ValueTracking.cpp

-      V1, V2, DemandedElts, 0,
-      SimplifyQuery(DL, DT, AC, safeCxtI(V2, V1, CxtI), UseInstrInfo));
+  return ::isKnownNonEqual(V1, V2, DemandedElts, Depth,
+                           Q.getWithInstruction(safeCxtI(V2, V1, Q.CxtI)));


I don't think we should do safeCxtI adjustment when an explicit SimplifyQuery has been provided. We don't do this for the other functions either.

Done. I set CxtI in BasicAAResult::aliasGEP to avoid a regression in llvm/test/Analysis/BasicAA/sequential-gep.ll:add_non_zero_assume.

goldsteinn · 2025-01-31T02:31:56Z

llvm/test/Transforms/InstCombine/known-bits.ll

-; CHECK-NEXT:    [[CAST:%.*]] = bitcast double [[X]] to i64
-; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i64 [[CAST]], 3458764513820540928
-; CHECK-NEXT:    ret i1 [[CMP]]
+; CHECK-NEXT:    ret i1 false


I don't quite understand why this change isn't NFC

With DC we will have a more precise KnownBits:

llvm-project/llvm/lib/Analysis/ValueTracking.cpp

Lines 3830 to 3840 in 8af24ce

if (V1->getType()->isIntOrIntVectorTy()) {

// Are any known bits in V1 contradictory to known bits in V2? If V1

// has a known zero where V2 has a known one, they must not be equal.

KnownBits Known1 = computeKnownBits(V1, DemandedElts, Depth, Q);

if (!Known1.isUnknown()) {

KnownBits Known2 = computeKnownBits(V2, DemandedElts, Depth, Q);

if (Known1.Zero.intersects(Known2.One) ||

Known2.Zero.intersects(Known1.One))

return true;

}

}

nikic

LGTM

nikic · 2025-01-31T08:59:40Z

llvm/lib/Analysis/BasicAliasAnalysis.cpp

+        isKnownNonEqual(Var0.Val.V, Var1.Val.V,
+                        SimplifyQuery(DL, DT, &AC, /*CxtI=*/Var0.CxtI
+                                                       ? Var0.CxtI
+                                                       : Var1.CxtI)))


This was actually using a null context instruction intentionally, to avoid order-dependence. But due to the implicit context, it ended up not mattering, and now things rely on it... It's okay for now, keeping symmetry in this code is not super important.

artagnon · 2025-01-31T20:22:06Z

llvm/lib/Analysis/ValueTracking.cpp

-  return ::isKnownNonEqual(
-      V1, V2, DemandedElts, 0,
-      SimplifyQuery(DL, DT, AC, safeCxtI(V2, V1, CxtI), UseInstrInfo));
+  return ::isKnownNonEqual(V1, V2, DemandedElts, Depth, Q);


Why did you add the Depth argument? Is the plan to support depth > 0 in follow-ups?

It may be set to MaxAnalysisRecursionDepth - 1 by the caller to avoid recursion.

artagnon

Very nice!

llvm-ci · 2025-02-01T07:21:01Z

LLVM Buildbot has detected a new failure on builder sanitizer-aarch64-linux running on sanitizer-buildbot8 while building llvm at step 2 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/10173