Skip to content

[LVI][CVP] Treat undef like a full range on abs(x, false) #68711

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 15, 2023

Conversation

dianqk
Copy link
Member

@dianqk dianqk commented Oct 10, 2023

Fixes #68682.

@llvmbot
Copy link
Member

llvmbot commented Oct 10, 2023

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-llvm-transforms

Author: DianQK (DianQK)

Changes

Fixes #68682.


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

4 Files Affected:

  • (modified) llvm/include/llvm/Analysis/LazyValueInfo.h (+4-2)
  • (modified) llvm/lib/Analysis/LazyValueInfo.cpp (+18-12)
  • (modified) llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp (+6-3)
  • (modified) llvm/test/Transforms/CorrelatedValuePropagation/abs.ll (+98)
diff --git a/llvm/include/llvm/Analysis/LazyValueInfo.h b/llvm/include/llvm/Analysis/LazyValueInfo.h
index f013a4a75d3d6ad..755d682d071272f 100644
--- a/llvm/include/llvm/Analysis/LazyValueInfo.h
+++ b/llvm/include/llvm/Analysis/LazyValueInfo.h
@@ -70,14 +70,16 @@ namespace llvm {
     /// predicate.
     Tristate getPredicateOnEdge(unsigned Pred, Value *V, Constant *C,
                                 BasicBlock *FromBB, BasicBlock *ToBB,
-                                Instruction *CxtI = nullptr);
+                                Instruction *CxtI = nullptr,
+                                bool UndefAllowed = true);
 
     /// Determine whether the specified value comparison with a constant is
     /// known to be true or false at the specified instruction. \p Pred is a
     /// CmpInst predicate. If \p UseBlockValue is true, the block value is also
     /// taken into account.
     Tristate getPredicateAt(unsigned Pred, Value *V, Constant *C,
-                            Instruction *CxtI, bool UseBlockValue);
+                            Instruction *CxtI, bool UseBlockValue,
+                            bool UndefAllowed = true);
 
     /// Determine whether the specified value comparison is known to be true
     /// or false at the specified instruction. While this takes two Value's,
diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp
index 789a02ed329c909..d7055198b0402a0 100644
--- a/llvm/lib/Analysis/LazyValueInfo.cpp
+++ b/llvm/lib/Analysis/LazyValueInfo.cpp
@@ -1750,7 +1750,8 @@ ConstantRange LazyValueInfo::getConstantRangeOnEdge(Value *V,
 
 static LazyValueInfo::Tristate
 getPredicateResult(unsigned Pred, Constant *C, const ValueLatticeElement &Val,
-                   const DataLayout &DL, TargetLibraryInfo *TLI) {
+                   const DataLayout &DL, TargetLibraryInfo *TLI,
+                   bool UndefAllowed = true) {
   // If we know the value is a constant, evaluate the conditional.
   Constant *Res = nullptr;
   if (Val.isConstant()) {
@@ -1760,7 +1761,7 @@ getPredicateResult(unsigned Pred, Constant *C, const ValueLatticeElement &Val,
     return LazyValueInfo::Unknown;
   }
 
-  if (Val.isConstantRange()) {
+  if (Val.isConstantRange(UndefAllowed)) {
     ConstantInt *CI = dyn_cast<ConstantInt>(C);
     if (!CI) return LazyValueInfo::Unknown;
 
@@ -1818,17 +1819,20 @@ getPredicateResult(unsigned Pred, Constant *C, const ValueLatticeElement &Val,
 LazyValueInfo::Tristate
 LazyValueInfo::getPredicateOnEdge(unsigned Pred, Value *V, Constant *C,
                                   BasicBlock *FromBB, BasicBlock *ToBB,
-                                  Instruction *CxtI) {
+                                  Instruction *CxtI, bool UndefAllowed) {
   Module *M = FromBB->getModule();
   ValueLatticeElement Result =
       getOrCreateImpl(M).getValueOnEdge(V, FromBB, ToBB, CxtI);
 
-  return getPredicateResult(Pred, C, Result, M->getDataLayout(), TLI);
+  return getPredicateResult(Pred, C, Result, M->getDataLayout(), TLI,
+                            UndefAllowed);
 }
 
-LazyValueInfo::Tristate
-LazyValueInfo::getPredicateAt(unsigned Pred, Value *V, Constant *C,
-                              Instruction *CxtI, bool UseBlockValue) {
+LazyValueInfo::Tristate LazyValueInfo::getPredicateAt(unsigned Pred, Value *V,
+                                                      Constant *C,
+                                                      Instruction *CxtI,
+                                                      bool UseBlockValue,
+                                                      bool UndefAllowed) {
   // Is or is not NonNull are common predicates being queried. If
   // isKnownNonZero can tell us the result of the predicate, we can
   // return it quickly. But this is only a fastpath, and falling
@@ -1847,7 +1851,7 @@ LazyValueInfo::getPredicateAt(unsigned Pred, Value *V, Constant *C,
   ValueLatticeElement Result =
       UseBlockValue ? Impl.getValueInBlock(V, CxtI->getParent(), CxtI)
                     : Impl.getValueAt(V, CxtI);
-  Tristate Ret = getPredicateResult(Pred, C, Result, DL, TLI);
+  Tristate Ret = getPredicateResult(Pred, C, Result, DL, TLI, UndefAllowed);
   if (Ret != Unknown)
     return Ret;
 
@@ -1892,8 +1896,8 @@ LazyValueInfo::getPredicateAt(unsigned Pred, Value *V, Constant *C,
         Value *Incoming = PHI->getIncomingValue(i);
         BasicBlock *PredBB = PHI->getIncomingBlock(i);
         // Note that PredBB may be BB itself.
-        Tristate Result =
-            getPredicateOnEdge(Pred, Incoming, C, PredBB, BB, CxtI);
+        Tristate Result = getPredicateOnEdge(Pred, Incoming, C, PredBB, BB,
+                                             CxtI, UndefAllowed);
 
         // Keep going as long as we've seen a consistent known result for
         // all inputs.
@@ -1914,11 +1918,13 @@ LazyValueInfo::getPredicateAt(unsigned Pred, Value *V, Constant *C,
     // For predecessor edge, determine if the comparison is true or false
     // on that edge. If they're all true or all false, we can conclude
     // the value of the comparison in this block.
-    Tristate Baseline = getPredicateOnEdge(Pred, V, C, *PI, BB, CxtI);
+    Tristate Baseline =
+        getPredicateOnEdge(Pred, V, C, *PI, BB, CxtI, UndefAllowed);
     if (Baseline != Unknown) {
       // Check that all remaining incoming values match the first one.
       while (++PI != PE) {
-        Tristate Ret = getPredicateOnEdge(Pred, V, C, *PI, BB, CxtI);
+        Tristate Ret =
+            getPredicateOnEdge(Pred, V, C, *PI, BB, CxtI, UndefAllowed);
         if (Ret != Baseline)
           break;
       }
diff --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
index 48b27a1ea0a2984..d276f5ea03e668e 100644
--- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -479,7 +479,8 @@ static bool processAbsIntrinsic(IntrinsicInst *II, LazyValueInfo *LVI) {
 
   // Is X in [0, IntMin]?  NOTE: INT_MIN is fine!
   Result = LVI->getPredicateAt(CmpInst::Predicate::ICMP_ULE, X, IntMin, II,
-                               /*UseBlockValue=*/true);
+                               /*UseBlockValue=*/true,
+                               /*UndefAllowed*/ IsIntMinPoison);
   if (Result == LazyValueInfo::True) {
     ++NumAbs;
     II->replaceAllUsesWith(X);
@@ -490,7 +491,8 @@ static bool processAbsIntrinsic(IntrinsicInst *II, LazyValueInfo *LVI) {
   // Is X in [IntMin, 0]?  NOTE: INT_MIN is fine!
   Constant *Zero = ConstantInt::getNullValue(Ty);
   Result = LVI->getPredicateAt(CmpInst::Predicate::ICMP_SLE, X, Zero, II,
-                               /*UseBlockValue=*/true);
+                               /*UseBlockValue=*/true,
+                               /*UndefAllowed*/ IsIntMinPoison);
   assert(Result != LazyValueInfo::False && "Should have been handled already.");
 
   if (Result == LazyValueInfo::Unknown) {
@@ -499,7 +501,8 @@ static bool processAbsIntrinsic(IntrinsicInst *II, LazyValueInfo *LVI) {
     if (!IsIntMinPoison) {
       // Can we at least tell that the argument is never INT_MIN?
       Result = LVI->getPredicateAt(CmpInst::Predicate::ICMP_NE, X, IntMin, II,
-                                   /*UseBlockValue=*/true);
+                                   /*UseBlockValue=*/true,
+                                   /*UndefAllowed*/ IsIntMinPoison);
       if (Result == LazyValueInfo::True) {
         ++NumNSW;
         ++NumSubNSW;
diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/abs.ll b/llvm/test/Transforms/CorrelatedValuePropagation/abs.ll
index 6231b05a851cb3f..4f3e0e977113101 100644
--- a/llvm/test/Transforms/CorrelatedValuePropagation/abs.ll
+++ b/llvm/test/Transforms/CorrelatedValuePropagation/abs.ll
@@ -2,6 +2,7 @@
 ; RUN: opt < %s -passes=correlated-propagation -S | FileCheck %s
 
 declare void @llvm.assume(i1)
+declare i32 @llvm.abs.i32(i32, i1)
 declare i8 @llvm.abs.i8(i8, i1)
 declare i1 @llvm.abs.i1(i1, i1)
 
@@ -387,3 +388,100 @@ define i1 @pr59887(i1 %x, i1 %c) {
   %res = select i1 %c, i1 %abs, i1 false
   ret i1 %res
 }
+
+; Because of `undef`, We can't delete `abs`.
+; We can't replace the `abs` argument with true either.
+define i32 @pr68381_undef_abs_false(i1 %c0, i1 %c1, i8 %v1) {
+; CHECK-LABEL: @pr68381_undef_abs_false(
+; CHECK-NEXT:  start:
+; CHECK-NEXT:    br i1 [[C0:%.*]], label [[BB0:%.*]], label [[BB1:%.*]]
+; CHECK:       bb0:
+; CHECK-NEXT:    [[V1_I32:%.*]] = zext i8 [[V1:%.*]] to i32
+; CHECK-NEXT:    br label [[BB1]]
+; CHECK:       bb1:
+; CHECK-NEXT:    [[X:%.*]] = phi i32 [ [[V1_I32]], [[BB0]] ], [ undef, [[START:%.*]] ]
+; CHECK-NEXT:    br i1 [[C1:%.*]], label [[BB0]], label [[BB2:%.*]]
+; CHECK:       bb2:
+; CHECK-NEXT:    [[Z:%.*]] = call i32 @llvm.abs.i32(i32 [[X]], i1 false)
+; CHECK-NEXT:    ret i32 [[Z]]
+;
+start:
+  br i1 %c0, label %bb0, label %bb1
+
+bb0:
+  %v1_i32 = zext i8 %v1 to i32
+  br label %bb1
+
+bb1:
+  %x = phi i32 [ %v1_i32, %bb0 ], [ undef, %start ]
+  br i1 %c1, label %bb0, label %bb2
+
+bb2:
+  %z = call i32 @llvm.abs.i32(i32 %x, i1 false)
+  ret i32 %z
+}
+
+; Because of `undef`, we can't replace `abs` with `sub`.
+define i32 @pr68381_undef_abs_false_sub(i1 %c0, i1 %c1, i32 %v1, i32 %v2) {
+; CHECK-LABEL: @pr68381_undef_abs_false_sub(
+; CHECK-NEXT:  start:
+; CHECK-NEXT:    br i1 [[C0:%.*]], label [[BB0:%.*]], label [[BB1:%.*]]
+; CHECK:       bb0:
+; CHECK-NEXT:    [[V3:%.*]] = add i32 [[V1:%.*]], [[V2:%.*]]
+; CHECK-NEXT:    [[LIM:%.*]] = icmp sle i32 [[V3]], -1
+; CHECK-NEXT:    call void @llvm.assume(i1 [[LIM]])
+; CHECK-NEXT:    br label [[BB1]]
+; CHECK:       bb1:
+; CHECK-NEXT:    [[X:%.*]] = phi i32 [ [[V3]], [[BB0]] ], [ undef, [[START:%.*]] ]
+; CHECK-NEXT:    br i1 [[C1:%.*]], label [[BB0]], label [[BB2:%.*]]
+; CHECK:       bb2:
+; CHECK-NEXT:    [[Z:%.*]] = call i32 @llvm.abs.i32(i32 [[X]], i1 false)
+; CHECK-NEXT:    ret i32 [[Z]]
+;
+start:
+  br i1 %c0, label %bb0, label %bb1
+
+bb0:
+  %v3 = add i32 %v1, %v2
+  %lim = icmp sle i32 %v3, -1
+  call void @llvm.assume(i1 %lim)
+  br label %bb1
+
+bb1:
+  %x = phi i32 [ %v3, %bb0 ], [ undef, %start ]
+  br i1 %c1, label %bb0, label %bb2
+
+bb2:
+  %z = call i32 @llvm.abs.i32(i32 %x, i1 false)
+  ret i32 %z
+}
+
+; We can delete `abs`.
+define i32 @pr68381_undef_abs_true(i1 %c0, i1 %c1, i8 %v1) {
+; CHECK-LABEL: @pr68381_undef_abs_true(
+; CHECK-NEXT:  start:
+; CHECK-NEXT:    br i1 [[C0:%.*]], label [[BB0:%.*]], label [[BB1:%.*]]
+; CHECK:       bb0:
+; CHECK-NEXT:    [[V1_I32:%.*]] = zext i8 [[V1:%.*]] to i32
+; CHECK-NEXT:    br label [[BB1]]
+; CHECK:       bb1:
+; CHECK-NEXT:    [[X:%.*]] = phi i32 [ [[V1_I32]], [[BB0]] ], [ undef, [[START:%.*]] ]
+; CHECK-NEXT:    br i1 [[C1:%.*]], label [[BB0]], label [[BB2:%.*]]
+; CHECK:       bb2:
+; CHECK-NEXT:    ret i32 [[X]]
+;
+start:
+  br i1 %c0, label %bb0, label %bb1
+
+bb0:
+  %v1_i32 = zext i8 %v1 to i32
+  br label %bb1
+
+bb1:
+  %x = phi i32 [ %v1_i32, %bb0 ], [ undef, %start ]
+  br i1 %c1, label %bb0, label %bb2
+
+bb2:
+  %z = call i32 @llvm.abs.i32(i32 %x, i1 true)
+  ret i32 %z
+}

@dianqk dianqk changed the title [LVI][CVP] Treat undef as INT_MIN on abs [LVI][CVP] Treat undef as Unknown on abs Oct 11, 2023
@dianqk dianqk changed the title [LVI][CVP] Treat undef as Unknown on abs [LVI][CVP] Treat undef like a full range on abs(x, false) Oct 15, 2023
@dianqk dianqk requested a review from nikic October 15, 2023 13:56
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dianqk dianqk merged commit 2ad9a65 into llvm:main Oct 15, 2023
@dianqk dianqk deleted the cvp-undef-abs branch October 15, 2023 22:25
tru pushed a commit that referenced this pull request Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[LVI][CVP] CVP error deleted the abs function
3 participants