[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: test/Transforms/LICM/scalar_promote.ll:208
+
+loop:
+  %j = phi i32 [ 0, %entry ], [ %next, %loop ]
----------------
reames wrote:
> mkuper wrote:
> > reames wrote:
> > > Looking at this test, it really should be handled by the thread local code we already have.  Have you looked to see why it isn't?
> > > 
> > > In fact, looking at *all* of your tests, the thread local case should handle it...?
> > > 
> > > (Note: This is a very important question to that I'll want an answer to before we commit the semantics parts of this code.)
> > Why would it be handled by the thread-local code?
> > %local is an alloca, so it's not thread-local memory.
> > 
> An alloca is thread local until the first time it escapes and become visible to another thread.  This is the same as a call to malloc().  
> 
> I think the isAllocLikeFn check in the thread local case should become (isAllocLikeFn || isa<AllocaInst>).  I believe that will handle the cases you've mentioned.
> An alloca is thread local until the first time it escapes and become visible to another thread. This is the same as a call to malloc().
Argh, right. 

> I think the isAllocLikeFn check in the thread local case should become (isAllocLikeFn || isa<AllocaInst>). I believe that will handle the cases you've mentioned.
I guess you're right, that will solve most issues. Thanks for the (failed :-) ) sanity check.
I think we may want both things, though - this can fire even if the alloca is captured (or if it's a global or a function argument instead of the alloca), if bar is argmemonly/readnone. 


https://reviews.llvm.org/D28147