[PATCH] D149132: [LICM] Reassociate & hoist add expressions

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2564
+  }
+  if (L.isLoopInvariant(LHS) || !L.isLoopInvariant(RHS) || !LHS->hasOneUse())
+    return false;
----------------
Any comments?
Why LHS invariant is not interested to you? Covered in other place? Assert instead of if?

Is hasOneUse a cost model? We want to remove original add?


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2575
+    std::swap(VariantOp, InvariantOp);
+  if (L.isLoopInvariant(VariantOp) || !L.isLoopInvariant(InvariantOp))
+    return false;
----------------
Comment?


================
Comment at: llvm/lib/Transforms/Scalar/LICM.cpp:2583
+  // <Pred> RHS - InvariantOp). It is only legal if we can prove that the
+  // subtraction won't overflow.
+  const SCEV *RHSS = SE.getSCEV(RHS);
----------------
Some words about, why it is legal if both add and sub are nsw?


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

https://reviews.llvm.org/D149132