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

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

Responding to the thread local discussion, but leaving the rest until you've separated the various pieces and landed them.



================
Comment at: test/Transforms/LICM/scalar_promote.ll:208
+
+loop:
+  %j = phi i32 [ 0, %entry ], [ %next, %loop ]
----------------
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.


https://reviews.llvm.org/D28147