[PATCH] D16986: [LICM] Don't assert on volatile accesses

[Nuno Lopes via llvm-commits]

nlopes added a comment.

In http://reviews.llvm.org/D16986#359209, @reames wrote:

> In http://reviews.llvm.org/D16986#359180, @nlopes wrote:
>
> > But then store forwarding happens and LICM realizes (or doesn't) that two of the alias sets that were said to be disjoint by TBAA in fact alias.
>
>
> Can you clarify this point?  AFAIK, LICM does not do store forwarding.  How did store forwarding come into the picture at all?


It does.  see here: http://www.llvm.org/docs/doxygen/html/LICM_8cpp_source.html#l00790
LICM does store forwarding of loop-invariant stores. Plus it moves the store to the loop exit.
So yes, it's this mechanism that is breaking the assumption that an alias set not marked as volatile cannot become volatile later (which it can if UB is exploited, like in this case).

> Also, if we ever discover two pointers mustalias, then by definition they must be the same alias set.  If not, we have constructed *invalid, and incorrect* alias sets.  Oh, I think I see the problem.  You're saying that they're both noalias and mustalias discovered through two different mechanism.  Ouch, ouch, ouch.


If add UB to the picture, then 2 pointers that must alias may end up in different alias sets. That's ok; it's just LICM doesn't know how to handle this specific volatile case.

I second my case that we should change TBAA instead, such that it handles volatile stuff more conservatively.


http://reviews.llvm.org/D16986