Skip to content

[InstCombine] Factorise add/sub and max/min using distributivity #101507

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jf-botto
Copy link
Contributor

@jf-botto jf-botto commented Aug 1, 2024

This PR fixes part of Issue 92433.

The alive proof sometimes times out. I have to add noundef to certain variables to make it not time out more consistently. Given how min/max/add preserve undef, I believe these optimisations to be correct when undef values are passed through.

Proofs:

  1. https://alive2.llvm.org/ce/z/9N_cAE (1 - 4 | umin_of_umax, umin_of_umax_comm, smin_of_smax, smin_of_smax_comm)
  2. https://alive2.llvm.org/ce/z/DchqXW (5 - 8 | umax_of_umin, umax_of_umin_comm, smax_of_smin, smax_of_smin_comm)
  3. https://alive2.llvm.org/ce/z/2j3P-- (9 - 10 | umax_of_uadd_sat, umax_of_uadd_sat_comm)
  4. https://alive2.llvm.org/ce/z/vR4v7c (11 | umin_of_uadd_sat)
  5. https://alive2.llvm.org/ce/z/dQjJT4 (12 | umin_of_uadd_sat_comm)
  6. https://alive2.llvm.org/ce/z/MskzQc (13 | smax_of_sadd_sat)
  7. https://alive2.llvm.org/ce/z/kLys_n (14 | smax_of_sadd_sat_comm)
  8. https://alive2.llvm.org/ce/z/b2KQeS (15 | smin_of_sadd_sat)
  9. https://alive2.llvm.org/ce/z/R4JrTN (16 | smin_of_sadd_sat_comm)

@jf-botto jf-botto requested a review from nikic as a code owner August 1, 2024 15:55
Copy link

github-actions bot commented Aug 1, 2024

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Member

llvmbot commented Aug 1, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Jorge Botto (jf-botto)

Changes

This PR fixes part of Issue 92433.
https://alive2.llvm.org/ce/z/ZaXf7L

The alive proof sometimes times out. I have to add noundef to certain variables to make it not time out more consistently. Given how min/max/add preserve undef, I believe these optimisations to be correct when undef values are passed through.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp (+93-1)
  • (added) llvm/test/Transforms/InstCombine/intrinsic-distributive.ll (+210)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index cc68fd4cf1c1b..3267d27d703e3 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -1505,6 +1505,80 @@ foldMinimumOverTrailingOrLeadingZeroCount(Value *I0, Value *I1,
       ConstantInt::getTrue(ZeroUndef->getType()));
 }
 
+/// Return whether "X LOp (Y ROp Z)" is always equal to
+/// "(X LOp Y) ROp (X LOp Z)".
+static bool leftDistributesOverRightIntrinsic(Intrinsic::ID LOp,
+                                              Intrinsic::ID ROp) {
+  switch (LOp) {
+  case Intrinsic::umax:
+    return ROp == Intrinsic::umin;
+  case Intrinsic::smax:
+    return ROp == Intrinsic::smin;
+  case Intrinsic::umin:
+    return ROp == Intrinsic::umax;
+  case Intrinsic::smin:
+    return ROp == Intrinsic::smax;
+  case Intrinsic::uadd_sat:
+    return ROp == Intrinsic::umax || ROp == Intrinsic::umin;
+  case Intrinsic::sadd_sat:
+    return ROp == Intrinsic::smax || ROp == Intrinsic::smin;
+  default:
+    return false;
+  }
+}
+
+// Attempts to factorise a common term
+// in an instruction that has the form "(A op' B) op (C op' D)
+static Instruction *
+foldCallUsingDistributiveLaws(CallInst *II, InstCombiner::BuilderTy &Builder) {
+  Value *LHS = II->getOperand(0), *RHS = II->getOperand(1);
+  Intrinsic::ID TopLevelOpcode = II->getCalledFunction()->getIntrinsicID();
+
+  if (LHS && RHS) {
+    CallInst *Op0 = dyn_cast<CallInst>(LHS);
+    CallInst *Op1 = dyn_cast<CallInst>(RHS);
+
+    if (!Op0 || !Op1)
+      return nullptr;
+
+    if (Op0->getCalledFunction()->getIntrinsicID() !=
+        Op1->getCalledFunction()->getIntrinsicID())
+      return nullptr;
+
+    Intrinsic::ID InnerOpcode = Op0->getCalledFunction()->getIntrinsicID();
+
+    bool InnerCommutative = Op0->isCommutative();
+    bool Distributive =
+        leftDistributesOverRightIntrinsic(InnerOpcode, TopLevelOpcode);
+
+    Value *A = Op0->getOperand(0);
+    Value *B = Op0->getOperand(1);
+    Value *C = Op1->getOperand(0);
+    Value *D = Op1->getOperand(1);
+
+    if (Distributive && (A == C || (InnerCommutative && A == D))) {
+      if (A != C)
+        std::swap(C, D);
+
+      Value *NewIntrinsic = Builder.CreateBinaryIntrinsic(TopLevelOpcode, B, D);
+      Function *F = Intrinsic::getDeclaration(II->getModule(), InnerOpcode,
+                                              II->getType());
+      return CallInst::Create(F, {NewIntrinsic, A});
+    }
+
+    if (Distributive && InnerCommutative && (B == D || B == C)) {
+      if (B != D)
+        std::swap(C, D);
+
+      Value *NewIntrinsic = Builder.CreateBinaryIntrinsic(TopLevelOpcode, A, C);
+      Function *F = Intrinsic::getDeclaration(II->getModule(), InnerOpcode,
+                                              II->getType());
+      return CallInst::Create(F, {NewIntrinsic, B});
+    }
+  }
+  return nullptr;
+}
+
 /// CallInst simplification. This mostly only handles folding of intrinsic
 /// instructions. For normal calls, it allows visitCallBase to do the heavy
 /// lifting.
@@ -1731,6 +1805,11 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
             foldMinimumOverTrailingOrLeadingZeroCount<Intrinsic::ctlz>(
                 I0, I1, DL, Builder))
       return replaceInstUsesWith(*II, FoldedCtlz);
+
+    if (Instruction *I = foldCallUsingDistributiveLaws(II, Builder)) {
+      return I;
+    }
+
     [[fallthrough]];
   }
   case Intrinsic::umax: {
@@ -1751,9 +1830,18 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
     }
     // If both operands of unsigned min/max are sign-extended, it is still ok
     // to narrow the operation.
+
+    if (Instruction *I = foldCallUsingDistributiveLaws(II, Builder))
+      return I;
+
+    [[fallthrough]];
+  }
+  case Intrinsic::smax: {
+    if (Instruction *I = foldCallUsingDistributiveLaws(II, Builder))
+      return I;
+
     [[fallthrough]];
   }
-  case Intrinsic::smax:
   case Intrinsic::smin: {
     Value *I0 = II->getArgOperand(0), *I1 = II->getArgOperand(1);
     Value *X, *Y;
@@ -1929,6 +2017,10 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
       }
     }
 
+    if (Instruction *I = foldCallUsingDistributiveLaws(II, Builder)) {
+      return I;
+    }
+
     break;
   }
   case Intrinsic::bitreverse: {
diff --git a/llvm/test/Transforms/InstCombine/intrinsic-distributive.ll b/llvm/test/Transforms/InstCombine/intrinsic-distributive.ll
new file mode 100644
index 0000000000000..10f4d8bbc7a0d
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/intrinsic-distributive.ll
@@ -0,0 +1,210 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=instcombine < %s 2>&1 | FileCheck %s
+
+define i32 @umin_of_umax(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @umin_of_umax(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.umin.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MIN:%.*]] = call i32 @llvm.umax.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MIN]]
+;
+  %max1 = call i32 @llvm.umax.i32(i32 %x, i32 %z)
+  %max2 = call i32 @llvm.umax.i32(i32 %y, i32 %z)
+  %min = call i32 @llvm.umin.i32(i32 %max1, i32 %max2)
+  ret i32 %min
+}
+
+define i32 @umin_of_umax_comm(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @umin_of_umax_comm(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.umin.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MIN:%.*]] = call i32 @llvm.umax.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MIN]]
+;
+  %max1 = call i32 @llvm.umax.i32(i32 %z, i32 %x)
+  %max2 = call i32 @llvm.umax.i32(i32 %z, i32 %y)
+  %min = call i32 @llvm.umin.i32(i32 %max1, i32 %max2)
+  ret i32 %min
+}
+
+define i32 @smin_of_smax(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @smin_of_smax(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.smin.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MIN:%.*]] = call i32 @llvm.smax.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MIN]]
+;
+  %max1 = call i32 @llvm.smax.i32(i32 %x, i32 %z)
+  %max2 = call i32 @llvm.smax.i32(i32 %y, i32 %z)
+  %min = call i32 @llvm.smin.i32(i32 %max1, i32 %max2)
+  ret i32 %min
+}
+
+define i32 @smin_of_smax_comm(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @smin_of_smax_comm(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.smin.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MIN:%.*]] = call i32 @llvm.smax.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MIN]]
+;
+  %max1 = call i32 @llvm.smax.i32(i32 %z, i32 %x)
+  %max2 = call i32 @llvm.smax.i32(i32 %z, i32 %y)
+  %min = call i32 @llvm.smin.i32(i32 %max1, i32 %max2)
+  ret i32 %min
+}
+
+define i32 @umax_of_umin(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @umax_of_umin(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.umax.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MAX:%.*]] = call i32 @llvm.umin.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MAX]]
+;
+  %min1 = call i32 @llvm.umin.i32(i32 %x, i32 %z)
+  %min2 = call i32 @llvm.umin.i32(i32 %y, i32 %z)
+  %max = call i32 @llvm.umax.i32(i32 %min1, i32 %min2)
+  ret i32 %max
+}
+
+define i32 @umax_of_umin_comm(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @umax_of_umin_comm(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.umax.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MAX:%.*]] = call i32 @llvm.umin.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MAX]]
+;
+  %min1 = call i32 @llvm.umin.i32(i32 %z, i32 %x)
+  %min2 = call i32 @llvm.umin.i32(i32 %z, i32 %y)
+  %max = call i32 @llvm.umax.i32(i32 %min1, i32 %min2)
+  ret i32 %max
+}
+
+define i32 @smax_of_smin(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @smax_of_smin(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.smax.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MAX:%.*]] = call i32 @llvm.smin.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MAX]]
+;
+  %min1 = call i32 @llvm.smin.i32(i32 %x, i32 %z)
+  %min2 = call i32 @llvm.smin.i32(i32 %y, i32 %z)
+  %max = call i32 @llvm.smax.i32(i32 %min1, i32 %min2)
+  ret i32 %max
+}
+
+define i32 @smax_of_smin_comm(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @smax_of_smin_comm(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.smax.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MAX:%.*]] = call i32 @llvm.smin.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MAX]]
+;
+  %min1 = call i32 @llvm.smin.i32(i32 %z, i32 %x)
+  %min2 = call i32 @llvm.smin.i32(i32 %z, i32 %y)
+  %max = call i32 @llvm.smax.i32(i32 %min1, i32 %min2)
+  ret i32 %max
+}
+
+define i32 @umax_of_uadd_sat(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @umax_of_uadd_sat(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.umax.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MAX:%.*]] = call i32 @llvm.uadd.sat.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MAX]]
+;
+  %add1 = call i32 @llvm.uadd.sat.i32(i32 %x, i32 %z)
+  %add2 = call i32 @llvm.uadd.sat.i32(i32 %y, i32 %z)
+  %max = call i32 @llvm.umax.i32(i32 %add1, i32 %add2)
+  ret i32 %max
+}
+
+define i32 @umax_of_uadd_sat_comm(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @umax_of_uadd_sat_comm(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.umax.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MAX:%.*]] = call i32 @llvm.uadd.sat.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MAX]]
+;
+  %add1 = call i32 @llvm.uadd.sat.i32(i32 %z, i32 %x)
+  %add2 = call i32 @llvm.uadd.sat.i32(i32 %z, i32 %y)
+  %max = call i32 @llvm.umax.i32(i32 %add1, i32 %add2)
+  ret i32 %max
+}
+
+define i32 @umin_of_uadd_sat(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @umin_of_uadd_sat(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.umin.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MIN:%.*]] = call i32 @llvm.uadd.sat.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MIN]]
+;
+  %add1 = call i32 @llvm.uadd.sat.i32(i32 %x, i32 %z)
+  %add2 = call i32 @llvm.uadd.sat.i32(i32 %y, i32 %z)
+  %min = call i32 @llvm.umin.i32(i32 %add1, i32 %add2)
+  ret i32 %min
+}
+
+define i32 @umin_of_uadd_sat_comm(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @umin_of_uadd_sat_comm(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.umin.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MIN:%.*]] = call i32 @llvm.uadd.sat.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MIN]]
+;
+  %add1 = call i32 @llvm.uadd.sat.i32(i32 %z, i32 %x)
+  %add2 = call i32 @llvm.uadd.sat.i32(i32 %z, i32 %y)
+  %min = call i32 @llvm.umin.i32(i32 %add1, i32 %add2)
+  ret i32 %min
+}
+
+define i32 @smax_of_sadd_sat(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @smax_of_sadd_sat(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.smax.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MAX:%.*]] = call i32 @llvm.sadd.sat.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MAX]]
+;
+  %add1 = call i32 @llvm.sadd.sat.i32(i32 %x, i32 %z)
+  %add2 = call i32 @llvm.sadd.sat.i32(i32 %y, i32 %z)
+  %max = call i32 @llvm.smax.i32(i32 %add1, i32 %add2)
+  ret i32 %max
+}
+
+define i32 @smax_of_sadd_sat_comm(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @smax_of_sadd_sat_comm(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.smax.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MAX:%.*]] = call i32 @llvm.sadd.sat.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MAX]]
+;
+  %add1 = call i32 @llvm.sadd.sat.i32(i32 %z, i32 %x)
+  %add2 = call i32 @llvm.sadd.sat.i32(i32 %z, i32 %y)
+  %max = call i32 @llvm.smax.i32(i32 %add1, i32 %add2)
+  ret i32 %max
+}
+
+define i32 @smin_of_sadd_sat(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @smin_of_sadd_sat(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.smin.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MIN:%.*]] = call i32 @llvm.sadd.sat.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MIN]]
+;
+  %add1 = call i32 @llvm.sadd.sat.i32(i32 %x, i32 %z)
+  %add2 = call i32 @llvm.sadd.sat.i32(i32 %y, i32 %z)
+  %min = call i32 @llvm.smin.i32(i32 %add1, i32 %add2)
+  ret i32 %min
+}
+
+define i32 @smin_of_sadd_sat_comm(i32 %x, i32 %y, i32 %z) {
+; CHECK-LABEL: define i32 @smin_of_sadd_sat_comm(
+; CHECK-SAME: i32 [[X:%.*]], i32 [[Y:%.*]], i32 [[Z:%.*]]) {
+; CHECK-NEXT:    [[TMP1:%.*]] = call i32 @llvm.smin.i32(i32 [[X]], i32 [[Y]])
+; CHECK-NEXT:    [[MIN:%.*]] = call i32 @llvm.sadd.sat.i32(i32 [[TMP1]], i32 [[Z]])
+; CHECK-NEXT:    ret i32 [[MIN]]
+;
+  %add1 = call i32 @llvm.sadd.sat.i32(i32 %z, i32 %x)
+  %add2 = call i32 @llvm.sadd.sat.i32(i32 %z, i32 %y)
+  %min = call i32 @llvm.smin.i32(i32 %add1, i32 %add2)
+  ret i32 %min
+}

@nikic
Copy link
Contributor

nikic commented Aug 1, 2024

The alive proof sometimes times out. I have to add noundef to certain variables to make it not time out more consistently. Given how min/max/add preserve undef, I believe these optimisations to be correct when undef values are passed through.

Try replacing all the i32s with i8. That usually helps with timeouts.

@goldsteinn
Copy link
Contributor

The alive proof sometimes times out. I have to add noundef to certain variables to make it not time out more consistently. Given how min/max/add preserve undef, I believe these optimisations to be correct when undef values are passed through.

Try replacing all the i32s with i8. That usually helps with timeouts.

In this case even i4 times out: https://alive2.llvm.org/ce/z/UPjvkJ

@jf-botto
Copy link
Contributor Author

jf-botto commented Aug 1, 2024

The alive proof sometimes times out. I have to add noundef to certain variables to make it not time out more consistently. Given how min/max/add preserve undef, I believe these optimisations to be correct when undef values are passed through.

Try replacing all the i32s with i8. That usually helps with timeouts.

Thank you! That really helps. I will update the alive link. I may have to separate the tests into 3-4 links.

@goldsteinn
Copy link
Contributor

Two of the proofs are buggy: https://alive2.llvm.org/ce/z/Xz2-Vq (src5 -> tgt5 and src6 -> tgt6).

@jf-botto
Copy link
Contributor Author

jf-botto commented Aug 1, 2024

The alive proof sometimes times out. I have to add noundef to certain variables to make it not time out more consistently. Given how min/max/add preserve undef, I believe these optimisations to be correct when undef values are passed through.

Try replacing all the i32s with i8. That usually helps with timeouts.

In this case even i4 times out: https://alive2.llvm.org/ce/z/UPjvkJ

It works for individual tests.

Two of the proofs are buggy: https://alive2.llvm.org/ce/z/Xz2-Vq (src5 -> tgt5 and src6 -> tgt6).

Thank you. Just noticed that. Will fix it.

case Intrinsic::smax: {
if (Instruction *I = foldCallUsingDistributiveLaws(II, Builder))
return I;

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think you need the call in the umin/umax/smax cases, they all fallthrough to smin (and it seems a bit silly to potentially fail 4x times on the same call).

@jf-botto
Copy link
Contributor Author

jf-botto commented Aug 1, 2024

Thank you @goldsteinn for the time and effort you've put into reviewing my code. I've addressed all of your points.
The two test cases that were buggy, was simply a mistake I made when copying and adapting the tests into alive. I've corrected the Alive tests in the top comment.

@jf-botto jf-botto requested a review from goldsteinn August 1, 2024 19:12
@nikic nikic requested a review from dtcxzyw August 1, 2024 19:13
dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Aug 2, 2024
dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Aug 2, 2024
@dtcxzyw
Copy link
Member

dtcxzyw commented Aug 2, 2024

With one-use check, this patch doesn't change anything :(

I think we should handle the interesting cases I listed in #92433 (comment) first.

@jf-botto
Copy link
Contributor Author

jf-botto commented Aug 2, 2024

@dtcxzyw, I was planning on making a second PR built upon this to handle the cases you mention. Would you rather me use this PR just for those cases you mention?

@dtcxzyw
Copy link
Member

dtcxzyw commented Aug 2, 2024

@dtcxzyw, I was planning on making a second PR built upon this to handle the cases you mention. Would you rather me use this PR just for those cases you mention?

Please file a separate PR.

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.

5 participants