[PATCH] D88287: [NARY-REASSOCIATE] Support reassociation of min/max

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/NaryReassociate.cpp:319
+      return dyn_cast_or_null<Instruction>(
+          tryReassociateMinOrMax(I, SMaxMatch, LHS, RHS));
+    }
----------------
ebrevnov wrote:
> mkazantsev wrote:
> > Can we factor out code of `if(match) { ... }` into a lambda that takes a matcher and returns instruction and avoid this copy-paste?
> Not sure how many lines of the code it will save.... let me try that... will see if it's any better
I still think it would be useful, these lines copy-paste over and over.


================
Comment at: llvm/test/Transforms/NaryReassociate/nary-smax.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -nary-reassociate -S | FileCheck %s
----------------
ebrevnov wrote:
> mkazantsev wrote:
> > Please commit these tests with auto-generated checks without your patch and rebase on top of it to see what your patch changes.
> Ok
Still actual.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88287