[PATCH] D147296: [InstCombine] Add support for max(a, b) + min(a, b) => a + b.

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added a comment.

Please add alive2 links for the proofs for {mul, add} x {smin/smax, umin/umax}.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:1596
 
+  // min(A, B) + max(A, B) => A + B.
+  if (match(&I, match_combine_or(
----------------
Can you move this to before nuw/nsw flags are set (L1557).


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:1602
+                            m_c_UMin(m_Deferred(A), m_Deferred(B)))))) {
+    return BinaryOperator::CreateAdd(A, B);
+  }
----------------
I think you can also preserve `nuw` and `nsw` flags.
https://alive2.llvm.org/ce/z/EaiNjB

likewise in the mul case:
https://alive2.llvm.org/ce/z/sV3tnG


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:486
+  // min(X, Y) * max(X, Y) => X * Y.
+  if (match(&I, match_combine_or(
+                    m_c_Mul(m_SMax(m_Value(X), m_Value(Y)),
----------------
Can you move this before `I` is potentially modified and `Changed` is set (before L474).


================
Comment at: llvm/test/Transforms/InstCombine/add-min-max.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
----------------
Can you split the tests into a seperate patch so we can see the diff this patch results in?


================
Comment at: llvm/test/Transforms/InstCombine/mul-min-max.ll:9
+
+define i32 @uadd_min_max(i32 %a, i32 %b) {
+; CHECK-LABEL: @uadd_min_max(
----------------
This (along with other tests) are misnamed. ('add' -> 'mul')


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147296/new/

https://reviews.llvm.org/D147296