[PATCH] D117302: [InstCombine] Simplify addends reordering logic

Mon Jan 17 09:33:32 PST 2022

kovdan01 added a comment.

In D117302#3248293 <https://reviews.llvm.org/D117302#3248293>, @spatel wrote:

> I already drafted a patch for it while looking at this, so I can clean that up and post it (the harder part is adding a pile of tests to check FMF propagation!).

OK, please let me know if you need any help with that. If I understood you correctly, you plan to submit the patch by yourself. If so, could you please mention me in revision to keep me informed? Thanks!

================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:540
+    // desirable to reside at the top of the resulting expression tree. Placing
+    // constant close to supper-expr(s) will potentially reveal some
+    // optimization opportunities in super-expr(s). Here we do not implement
----------------
spatel wrote:
> typo: supper -> super
Fixed

================
Comment at: llvm/test/Transforms/Reassociate/fast-basictest.ll:2
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -reassociate -gvn -instcombine -S | FileCheck %s

----------------
spatel wrote:
> I missed that this test is under Reassociate. It's generally wrong for a regression test to run 3 different passes, but it can be a separate cleanup patch.
Got it, will submit a separate patch to fix this

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117302