[PATCH] D28147: [LICM] Allow promotion of some stores that are not guaranteed to execute

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper added inline comments.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:288
+  // loop used in the outer loop.
+  // FIXME: This is really heavy handed. It would be a bit better to use an
+  // SSAUpdater strategy during promotion that was LCSSA aware and reformed
----------------
mkuper wrote:
> reames wrote:
> > Use of the Changed variable here is overly conservative.  We actually only need to run this if *store promotion* changed the loop.  If you wanted to do that as a a separate change, it would be a useful cleanup and compile time win.
> I think that's actually wrong (and the comment is misleading, I should fix it)- see above for explanation.
> 
> I mean, I'm not saying that we *need* to run it in other cases (although I'm pretty sure we do), I'm saying we *did* run it in other cases, and I'd prefer this part to be NFC.
And looks like I was wrong, apparently this really is fine to run only for promotions, I guess the other transformations preserve LCSSA.
I'll sort all of that out first, then update this patch.


https://reviews.llvm.org/D28147