[PATCH] D50925: [LICM] Hoist stores of invariant values to invariant addresses out of loops

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: lib/Transforms/Scalar/LICM.cpp:745
+    // We can only hoist a store that we can prove writes a value which is not
+    // read or overwritten within the loop.  For those cases, we fallback to
+    // load store promotion instead.
----------------
mkazantsev wrote:
> reames wrote:
> > mkazantsev wrote:
> > > We can hoist stores to locations that are read within the loop if we can prove that no read may happen before the store. It doesn't matter if this location is then stored again or not in this case.
> > > 
> > > Please add it as a TODO item, may be interesting in the future.
> > The particular case mentioned could be done, but the motivating case you described offline w/a readonly call in the loop is much harder than it first sounds.  The basic problem is the AST doesn't actually keep track of the *instructions* it only tracks memory locations.  So, for a large read set, we don't have an efficient way to determine if there's a single mod.  
> > 
> > I think the transform you're looking for is probably better done w/MemorySSA once we get around to that (larger) rewrite.  
> > 
> > I'd prefer not to put a todo here if you don't mind.
> It only means that we might want to give up using AST here. The mention that it can be generalized still makes sense.
Add the todo and corresponding tests:
Sending        lib/Transforms/Scalar/LICM.cpp
Sending        test/Transforms/LICM/store-hoisting.ll
Transmitting file data ..done
Committing transaction...
Committed revision 340978.

When writing the tests, I realized we can handle a lot of common cases, just not the alias set collapse one.  Most of tests can be implemented easily.


Repository:
  rL LLVM

https://reviews.llvm.org/D50925