[PATCH] D152281: [Transforms][LICM] Add the ability to undo unprofitable reassociation

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2708
+      return false;
+    if (!match(VOp, m_FAdd(m_BinOp(Op), m_BinOp(OpNext)))) {
+      Op = VOp;
----------------
Do we need to require that the fadd is reassoc as well?


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2742
+    IRBuilder<> Builder(Preheader->getTerminator());
+    U->set(Builder.CreateFMulFMF(U->get(), Factor, &I, "factor.op.fmul"));
+  }
----------------
This copies the FMF flags from the top-level fmul, but is that correct? We have multiple fadds and fmuls involved in the transform, why is it safe to use the flags from that one in particular?

(The test coverage for this is not great, because you just use "fast" everywhere.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152281