[ConstraintElim] NE implies SLT if SLE and SGT if SGE

[citymarina]

https://alive2.llvm.org/ce/z/bsa7t5

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Marina Taylor (citymarina)

Changes

https://alive2.llvm.org/ce/z/bsa7t5

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/Scalar/ConstraintElimination.cpp (+26-14)
• (modified) llvm/test/Transforms/ConstraintElimination/ne.ll (+81)

diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index 6dd26910f6846..48a7a58ec9e9c 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -1086,6 +1086,20 @@ void State::addInfoForInductions(BasicBlock &BB) {
 void State::addInfoFor(BasicBlock &BB) {
   addInfoForInductions(BB);
 
+  auto addConditionFact = [&](DomTreeNode *DTN, CmpPredicate Pred, Value *Op0,
+                              Value *Op1) {
+    WorkList.emplace_back(FactOrCheck::getConditionFact(DTN, Pred, Op0, Op1));
+    // In the case of NE zero, we can deduce SLT if SLE and SGT if SGE.
+    if (Pred == CmpInst::ICMP_NE && match(Op1, m_Zero())) {
+      ConditionTy Precond = {CmpInst::ICMP_SLE, Op0, Op1};
+      WorkList.emplace_back(FactOrCheck::getConditionFact(
+          DTN, CmpInst::ICMP_SLT, Op0, Op1, Precond));
+      Precond = {CmpInst::ICMP_SGE, Op0, Op1};
+      WorkList.emplace_back(FactOrCheck::getConditionFact(
+          DTN, CmpInst::ICMP_SGT, Op0, Op1, Precond));
+    }
+  };
+
   // True as long as long as the current instruction is guaranteed to execute.
   bool GuaranteedToExecute = true;
   // Queue conditions and assumes.
@@ -1112,8 +1126,7 @@ void State::addInfoFor(BasicBlock &BB) {
       if (GuaranteedToExecute) {
         // The assume is guaranteed to execute when BB is entered, hence Cond
         // holds on entry to BB.
-        WorkList.emplace_back(FactOrCheck::getConditionFact(
-            DT.getNode(I.getParent()), Pred, A, B));
+        addConditionFact(DT.getNode(I.getParent()), Pred, A, B);
       } else {
         WorkList.emplace_back(
             FactOrCheck::getInstFact(DT.getNode(I.getParent()), &I));
@@ -1154,8 +1167,8 @@ void State::addInfoFor(BasicBlock &BB) {
       Value *V = Case.getCaseValue();
       if (!canAddSuccessor(BB, Succ))
         continue;
-      WorkList.emplace_back(FactOrCheck::getConditionFact(
-          DT.getNode(Succ), CmpInst::ICMP_EQ, Switch->getCondition(), V));
+      addConditionFact(DT.getNode(Succ), CmpInst::ICMP_EQ,
+                       Switch->getCondition(), V);
     }
     return;
   }
@@ -1191,10 +1204,10 @@ void State::addInfoFor(BasicBlock &BB) {
       while (!CondWorkList.empty()) {
         Value *Cur = CondWorkList.pop_back_val();
         if (auto *Cmp = dyn_cast<ICmpInst>(Cur)) {
-          WorkList.emplace_back(FactOrCheck::getConditionFact(
-              DT.getNode(Successor),
-              IsOr ? Cmp->getInverseCmpPredicate() : Cmp->getCmpPredicate(),
-              Cmp->getOperand(0), Cmp->getOperand(1)));
+          addConditionFact(DT.getNode(Successor),
+                           IsOr ? Cmp->getInverseCmpPredicate()
+                                : Cmp->getCmpPredicate(),
+                           Cmp->getOperand(0), Cmp->getOperand(1));
           continue;
         }
         if (IsOr && match(Cur, m_LogicalOr(m_Value(Op0), m_Value(Op1)))) {
@@ -1216,13 +1229,12 @@ void State::addInfoFor(BasicBlock &BB) {
   if (!CmpI)
     return;
   if (canAddSuccessor(BB, Br->getSuccessor(0)))
-    WorkList.emplace_back(FactOrCheck::getConditionFact(
-        DT.getNode(Br->getSuccessor(0)), CmpI->getCmpPredicate(),
-        CmpI->getOperand(0), CmpI->getOperand(1)));
+    addConditionFact(DT.getNode(Br->getSuccessor(0)), CmpI->getCmpPredicate(),
+                     CmpI->getOperand(0), CmpI->getOperand(1));
   if (canAddSuccessor(BB, Br->getSuccessor(1)))
-    WorkList.emplace_back(FactOrCheck::getConditionFact(
-        DT.getNode(Br->getSuccessor(1)), CmpI->getInverseCmpPredicate(),
-        CmpI->getOperand(0), CmpI->getOperand(1)));
+    addConditionFact(DT.getNode(Br->getSuccessor(1)),
+                     CmpI->getInverseCmpPredicate(), CmpI->getOperand(0),
+                     CmpI->getOperand(1));
 }
 
 #ifndef NDEBUG
diff --git a/llvm/test/Transforms/ConstraintElimination/ne.ll b/llvm/test/Transforms/ConstraintElimination/ne.ll
index 4753860db2851..6413b398e70f5 100644
--- a/llvm/test/Transforms/ConstraintElimination/ne.ll
+++ b/llvm/test/Transforms/ConstraintElimination/ne.ll
@@ -423,3 +423,84 @@ define i1 @assume_4b(i64 %a, i64 %b) {
   %ret = icmp sle i64 %a, %b
   ret i1 %ret
 }
+
+define i1 @sle_and_ne_imply_slt(i8 %a) {
+; CHECK-LABEL: @sle_and_ne_imply_slt(
+; CHECK-NEXT:    [[SLE:%.*]] = icmp sle i8 [[A:%.*]], 0
+; CHECK-NEXT:    br i1 [[SLE]], label [[LESS_OR_EQUAL:%.*]], label [[EXIT:%.*]]
+; CHECK:       less_or_equal:
+; CHECK-NEXT:    [[NE:%.*]] = icmp ne i8 [[A]], 0
+; CHECK-NEXT:    br i1 [[NE]], label [[NOT_EQUAL:%.*]], label [[EXIT]]
+; CHECK:       not_equal:
+; CHECK-NEXT:    ret i1 true
+; CHECK:       exit:
+; CHECK-NEXT:    ret i1 false
+;
+  %sle = icmp sle i8 %a, 0
+  br i1 %sle, label %less_or_equal, label %exit
+
+less_or_equal:
+  %ne = icmp ne i8 %a, 0
+  br i1 %ne, label %not_equal, label %exit
+
+not_equal:
+  %slt = icmp slt i8 %a, 0
+  ret i1 %slt
+
+exit:
+  ret i1 0
+}
+
+define i1 @sge_and_ne_imply_sgt(i8 %a) {
+; CHECK-LABEL: @sge_and_ne_imply_sgt(
+; CHECK-NEXT:    [[SGE:%.*]] = icmp sge i8 [[A:%.*]], 0
+; CHECK-NEXT:    br i1 [[SGE]], label [[GREATER_OR_EQUAL:%.*]], label [[EXIT:%.*]]
+; CHECK:       greater_or_equal:
+; CHECK-NEXT:    [[NE:%.*]] = icmp ne i8 [[A]], 0
+; CHECK-NEXT:    br i1 [[NE]], label [[NOT_EQUAL:%.*]], label [[EXIT]]
+; CHECK:       not_equal:
+; CHECK-NEXT:    ret i1 true
+; CHECK:       exit:
+; CHECK-NEXT:    ret i1 false
+;
+  %sge = icmp sge i8 %a, 0
+  br i1 %sge, label %greater_or_equal, label %exit
+
+greater_or_equal:
+  %ne = icmp ne i8 %a, 0
+  br i1 %ne, label %not_equal, label %exit
+
+not_equal:
+  %sgt = icmp sgt i8 %a, 0
+  ret i1 %sgt
+
+exit:
+  ret i1 0
+}
+
+define i1 @sge_and_ne_imply_sgt_opposite_successor(i8 %a) {
+; CHECK-LABEL: @sge_and_ne_imply_sgt_opposite_successor(
+; CHECK-NEXT:    [[SLT:%.*]] = icmp slt i8 [[A:%.*]], 0
+; CHECK-NEXT:    br i1 [[SLT]], label [[EXIT:%.*]], label [[GREATER_OR_EQUAL:%.*]]
+; CHECK:       greater_or_equal:
+; CHECK-NEXT:    [[EQ:%.*]] = icmp eq i8 [[A]], 0
+; CHECK-NEXT:    br i1 [[EQ]], label [[EXIT]], label [[NOT_EQUAL:%.*]]
+; CHECK:       not_equal:
+; CHECK-NEXT:    ret i1 true
+; CHECK:       exit:
+; CHECK-NEXT:    ret i1 false
+;
+  %slt = icmp slt i8 %a, 0
+  br i1 %slt, label %exit, label %greater_or_equal
+
+greater_or_equal:
+  %eq = icmp eq i8 %a, 0
+  br i1 %eq, label %exit, label %not_equal
+
+not_equal:
+  %sgt = icmp sgt i8 %a, 0
+  ret i1 %sgt
+
+exit:
+  ret i1 0
+}

[nikic]

This has compile-time overhead: https://llvm-compile-time-tracker.com/compare.php?from=2cb5241c7724cbf4fa02b25547e098bfc635aba9&to=517f1757636ca8e5aa2c7f3f61e5b9672527886e&stat=instructions:u

[dtcxzyw]

The motivating case has been handled by CVP/SCCP: https://godbolt.org/z/qE9srErGx
I am drafting a patch in LVI to perform missed optimizations exposed by this PR.

[citymarina]

Thank you both for the input. It sounds like we don't need this patch for now. I've opened #128168 to cover a different use case that we have.