VT: teach isImpliedCondMatchingOperands about samesign #122474

artagnon · 2025-01-10T15:29:31Z

Move isImplied{True,False}ByMatchingCmp from CmpInst to ICmpInst, so that it can operate on CmpPredicate instead of CmpInst::Predicate, and teach it about samesign. There are two callers of this function, and we choose to migrate the one in ValueTracking, namely isImpliedCondMatchingOperands to CmpPredicate, hence teaching it about samesign, with visible test impact.

llvmbot · 2025-01-10T15:30:07Z

@llvm/pr-subscribers-llvm-analysis
@llvm/pr-subscribers-llvm-ir

@llvm/pr-subscribers-llvm-transforms

Author: Ramkumar Ramachandra (artagnon)

Changes

Move isImplied{True,False}ByMatchingCmp from CmpInst to ICmpInst, so that it can operate on CmpPredicate instead of CmpInst::Predicate, and teach it about samesign. There are two callers of this function, and we choose to migrate the one in ValueTracking, namely isImpliedCondMatchingOperands to CmpPredicate, hence teaching it about samesign, with visible test impact.

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

7 Files Affected:

(modified) llvm/include/llvm/IR/InstrTypes.h (-8)
(modified) llvm/include/llvm/IR/Instructions.h (+10)
(modified) llvm/include/llvm/SandboxIR/Instruction.h (+9-7)
(modified) llvm/lib/Analysis/ValueTracking.cpp (+4-5)
(modified) llvm/lib/IR/Instructions.cpp (+10-3)
(modified) llvm/lib/Transforms/Scalar/NewGVN.cpp (+4-4)
(modified) llvm/test/Analysis/ValueTracking/implied-condition-samesign.ll (+2-5)

diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h
index e6332a16df7d5f..7ad34e4f223394 100644
--- a/llvm/include/llvm/IR/InstrTypes.h
+++ b/llvm/include/llvm/IR/InstrTypes.h
@@ -967,14 +967,6 @@ class CmpInst : public Instruction {
   /// Determine if the predicate is false when comparing a value with itself.
   static bool isFalseWhenEqual(Predicate predicate);
 
-  /// Determine if Pred1 implies Pred2 is true when two compares have matching
-  /// operands.
-  static bool isImpliedTrueByMatchingCmp(Predicate Pred1, Predicate Pred2);
-
-  /// Determine if Pred1 implies Pred2 is false when two compares have matching
-  /// operands.
-  static bool isImpliedFalseByMatchingCmp(Predicate Pred1, Predicate Pred2);
-
   /// Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const Instruction *I) {
     return I->getOpcode() == Instruction::ICmp ||
diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
index a8df12a1282fcc..59eb5040988378 100644
--- a/llvm/include/llvm/IR/Instructions.h
+++ b/llvm/include/llvm/IR/Instructions.h
@@ -1266,6 +1266,16 @@ class ICmpInst: public CmpInst {
     return getFlippedSignednessPredicate(getPredicate());
   }
 
+  /// Determine if Pred1 implies Pred2 is true when two compares have matching
+  /// operands.
+  static bool isImpliedTrueByMatchingCmp(CmpPredicate Pred1,
+                                         CmpPredicate Pred2);
+
+  /// Determine if Pred1 implies Pred2 is false when two compares have matching
+  /// operands.
+  static bool isImpliedFalseByMatchingCmp(CmpPredicate Pred1,
+                                          CmpPredicate Pred2);
+
   void setSameSign(bool B = true) {
     SubclassOptionalData = (SubclassOptionalData & ~SameSign) | (B * SameSign);
   }
diff --git a/llvm/include/llvm/SandboxIR/Instruction.h b/llvm/include/llvm/SandboxIR/Instruction.h
index 4d21c4d3da3556..d7c1eda81c0060 100644
--- a/llvm/include/llvm/SandboxIR/Instruction.h
+++ b/llvm/include/llvm/SandboxIR/Instruction.h
@@ -2511,13 +2511,6 @@ class CmpInst : public SingleLLVMInstructionImpl<llvm::CmpInst> {
   WRAP_STATIC_PREDICATE(isOrdered);
   WRAP_STATIC_PREDICATE(isUnordered);
 
-  static bool isImpliedTrueByMatchingCmp(Predicate Pred1, Predicate Pred2) {
-    return llvm::CmpInst::isImpliedTrueByMatchingCmp(Pred1, Pred2);
-  }
-  static bool isImpliedFalseByMatchingCmp(Predicate Pred1, Predicate Pred2) {
-    return llvm::CmpInst::isImpliedFalseByMatchingCmp(Pred1, Pred2);
-  }
-
   /// Method for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const Value *From) {
     return From->getSubclassID() == ClassID::ICmp ||
@@ -2554,6 +2547,15 @@ class ICmpInst : public CmpInst {
   WRAP_STATIC_PREDICATE(isGE);
   WRAP_STATIC_PREDICATE(isLE);
 
+  static bool isImpliedTrueByMatchingCmp(CmpPredicate Pred1,
+                                         CmpPredicate Pred2) {
+    return llvm::ICmpInst::isImpliedTrueByMatchingCmp(Pred1, Pred2);
+  }
+  static bool isImpliedFalseByMatchingCmp(CmpPredicate Pred1,
+                                          CmpPredicate Pred2) {
+    return llvm::ICmpInst::isImpliedFalseByMatchingCmp(Pred1, Pred2);
+  }
+
   static auto predicates() { return llvm::ICmpInst::predicates(); }
   static bool compare(const APInt &LHS, const APInt &RHS,
                       ICmpInst::Predicate Pred) {
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 9a61b36efa51da..31792dcb17df95 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -9374,12 +9374,11 @@ isImpliedCondOperands(CmpInst::Predicate Pred, const Value *ALHS,
 /// Return true if "icmp1 LPred X, Y" implies "icmp2 RPred X, Y" is true.
 /// Return false if "icmp1 LPred X, Y" implies "icmp2 RPred X, Y" is false.
 /// Otherwise, return std::nullopt if we can't infer anything.
-static std::optional<bool>
-isImpliedCondMatchingOperands(CmpInst::Predicate LPred,
-                              CmpInst::Predicate RPred) {
-  if (CmpInst::isImpliedTrueByMatchingCmp(LPred, RPred))
+static std::optional<bool> isImpliedCondMatchingOperands(CmpPredicate LPred,
+                                                         CmpPredicate RPred) {
+  if (ICmpInst::isImpliedTrueByMatchingCmp(LPred, RPred))
     return true;
-  if (CmpInst::isImpliedFalseByMatchingCmp(LPred, RPred))
+  if (ICmpInst::isImpliedFalseByMatchingCmp(LPred, RPred))
     return false;
 
   return std::nullopt;
diff --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp
index 2d6fe40f4c1de0..49c148bb68a4d3 100644
--- a/llvm/lib/IR/Instructions.cpp
+++ b/llvm/lib/IR/Instructions.cpp
@@ -3886,12 +3886,18 @@ bool CmpInst::isFalseWhenEqual(Predicate predicate) {
   }
 }
 
-bool CmpInst::isImpliedTrueByMatchingCmp(Predicate Pred1, Predicate Pred2) {
+bool ICmpInst::isImpliedTrueByMatchingCmp(CmpPredicate Pred1,
+                                          CmpPredicate Pred2) {
   // If the predicates match, then we know the first condition implies the
   // second is true.
-  if (Pred1 == Pred2)
+  if (CmpPredicate::getMatching(Pred1, Pred2))
     return true;
 
+  if (Pred1.hasSameSign() && CmpInst::isSigned(Pred2))
+    Pred1 = ICmpInst::getFlippedSignednessPredicate(Pred1);
+  else if (Pred2.hasSameSign() && CmpInst::isSigned(Pred1))
+    Pred2 = ICmpInst::getFlippedSignednessPredicate(Pred2);
+
   switch (Pred1) {
   default:
     break;
@@ -3911,7 +3917,8 @@ bool CmpInst::isImpliedTrueByMatchingCmp(Predicate Pred1, Predicate Pred2) {
   return false;
 }
 
-bool CmpInst::isImpliedFalseByMatchingCmp(Predicate Pred1, Predicate Pred2) {
+bool ICmpInst::isImpliedFalseByMatchingCmp(CmpPredicate Pred1,
+                                           CmpPredicate Pred2) {
   return isImpliedTrueByMatchingCmp(Pred1, getInversePredicate(Pred2));
 }
 
diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp
index 0cba8739441bcb..3812e99508f738 100644
--- a/llvm/lib/Transforms/Scalar/NewGVN.cpp
+++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp
@@ -1964,15 +1964,15 @@ NewGVN::ExprResult NewGVN::performSymbolicCmpEvaluation(Instruction *I) const {
         if (PBranch->TrueEdge) {
           // If we know the previous predicate is true and we are in the true
           // edge then we may be implied true or false.
-          if (CmpInst::isImpliedTrueByMatchingCmp(BranchPredicate,
-                                                  OurPredicate)) {
+          if (ICmpInst::isImpliedTrueByMatchingCmp(BranchPredicate,
+                                                   OurPredicate)) {
             return ExprResult::some(
                 createConstantExpression(ConstantInt::getTrue(CI->getType())),
                 PI);
           }
 
-          if (CmpInst::isImpliedFalseByMatchingCmp(BranchPredicate,
-                                                   OurPredicate)) {
+          if (ICmpInst::isImpliedFalseByMatchingCmp(BranchPredicate,
+                                                    OurPredicate)) {
             return ExprResult::some(
                 createConstantExpression(ConstantInt::getFalse(CI->getType())),
                 PI);
diff --git a/llvm/test/Analysis/ValueTracking/implied-condition-samesign.ll b/llvm/test/Analysis/ValueTracking/implied-condition-samesign.ll
index 042155ae2bb79b..546ff2d77d86e3 100644
--- a/llvm/test/Analysis/ValueTracking/implied-condition-samesign.ll
+++ b/llvm/test/Analysis/ValueTracking/implied-condition-samesign.ll
@@ -118,8 +118,7 @@ define i1 @sgt_implies_ge_via_assume(i32 %i, i32 %j) {
 ; CHECK-SAME: i32 [[I:%.*]], i32 [[J:%.*]]) {
 ; CHECK-NEXT:    [[I_SGT_J:%.*]] = icmp sgt i32 [[I]], [[J]]
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[I_SGT_J]])
-; CHECK-NEXT:    [[I_GE_J:%.*]] = icmp samesign uge i32 [[I]], [[J]]
-; CHECK-NEXT:    ret i1 [[I_GE_J]]
+; CHECK-NEXT:    ret i1 true
 ;
   %i.sgt.j = icmp sgt i32 %i, %j
   call void @llvm.assume(i1 %i.sgt.j)
@@ -134,9 +133,7 @@ define i32 @gt_implies_sge_dominating(i32 %a, i32 %len) {
 ; CHECK-NEXT:    [[A_GT_LEN:%.*]] = icmp samesign ugt i32 [[A]], [[LEN]]
 ; CHECK-NEXT:    br i1 [[A_GT_LEN]], label %[[TAKEN:.*]], label %[[END:.*]]
 ; CHECK:       [[TAKEN]]:
-; CHECK-NEXT:    [[A_SGE_LEN:%.*]] = icmp sge i32 [[A]], [[LEN]]
-; CHECK-NEXT:    [[RES:%.*]] = select i1 [[A_SGE_LEN]], i32 30, i32 0
-; CHECK-NEXT:    ret i32 [[RES]]
+; CHECK-NEXT:    ret i32 30
 ; CHECK:       [[END]]:
 ; CHECK-NEXT:    ret i32 -1
 ;

dtcxzyw

LGTM. Please wait for additional approval from other reviewers :)

dtcxzyw · 2025-01-11T05:43:05Z

llvm/lib/Analysis/ValueTracking.cpp

-  if (CmpInst::isImpliedTrueByMatchingCmp(LPred, RPred))
+static std::optional<bool> isImpliedCondMatchingOperands(CmpPredicate LPred,
+                                                         CmpPredicate RPred) {
+  if (ICmpInst::isImpliedTrueByMatchingCmp(LPred, RPred))


Seems like isImpliedTrueByMatchingCmp and isImpliedFalseByMatchingCmp are always called together. We can replace them with a helper std::optional<bool> isImpliedByMatchingCmp in the future.

nikic

LGTM

Move isImplied{True,False}ByMatchingCmp from CmpInst to ICmpInst, so that it can operate on CmpPredicate instead of CmpInst::Predicate, and teach it about samesign. There are two callers of this function, and we choose to migrate the one in ValueTracking, namely isImpliedCondMatchingOperands to CmpPredicate, hence teaching it about samesign, with visible test impact.

artagnon requested a review from dtcxzyw January 10, 2025 15:29

artagnon requested a review from nikic as a code owner January 10, 2025 15:29

llvmbot added llvm:ir llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms llvm:SandboxIR labels Jan 10, 2025

nikic mentioned this pull request Jan 10, 2025

Task submission dtcxzyw/llvm-opt-benchmark#1312

Open

This was referenced Jan 10, 2025

pre-commit: PR122474 dtcxzyw/llvm-opt-benchmark#1943

Closed

Fuzz PR122474 dtcxzyw/llvm-fuzz-service#51

Closed

dtcxzyw approved these changes Jan 11, 2025

View reviewed changes

nikic approved these changes Jan 11, 2025

View reviewed changes

artagnon merged commit f38c40b into llvm:main Jan 11, 2025
13 checks passed

artagnon deleted the vt-samesign-matching-operands branch January 11, 2025 09:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

VT: teach isImpliedCondMatchingOperands about samesign #122474

VT: teach isImpliedCondMatchingOperands about samesign #122474

Uh oh!

artagnon commented Jan 10, 2025

Uh oh!

llvmbot commented Jan 10, 2025 •

edited

Loading

Uh oh!

dtcxzyw left a comment

Uh oh!

dtcxzyw Jan 11, 2025

Uh oh!

nikic left a comment

Uh oh!

Uh oh!

Uh oh!

VT: teach isImpliedCondMatchingOperands about samesign #122474

VT: teach isImpliedCondMatchingOperands about samesign #122474

Uh oh!

Conversation

artagnon commented Jan 10, 2025

Uh oh!

llvmbot commented Jan 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dtcxzyw left a comment

Choose a reason for hiding this comment

Uh oh!

dtcxzyw Jan 11, 2025

Choose a reason for hiding this comment

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Jan 10, 2025 •

edited

Loading