[PATCH] D57967: [LICM&MSSA] Limit store hoisting.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea marked 2 inline comments as done.
asbirlea added inline comments.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:121
 
+static bool LocalDisablePromotion = false;
+
----------------
george.burgess.iv wrote:
> I don't think that this will play nicely in threaded environments (ThinLTO?)
> 
> An alternative might be to stick it into LICM as a member, then pass it to canSinkOrHoistInst as a param, though I don't know this code well enough to say whether that's the best approach
Ack, passed as param (renamed for clarity of it's meaning).


================
Comment at: lib/Transforms/Scalar/LICM.cpp:1194
+              if (const auto *MU = dyn_cast<MemoryUse>(&MA)) {
+                if (!MU->isOptimized())
+                  return false;
----------------
george.burgess.iv wrote:
> Why does optimizedness matter here? `MemoryUse`s should always have valid defining accesses; I imagine that a valid-but-not-fully-optimal one would be good enough as long as it points outside of the loop
Indeed, removed.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57967