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

[Daniel Berlin via llvm-commits]

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

Volatileness and aliasing are different properties.
It's completely reasonable for TBAA (and other alias analysis) to say two
pointers may or may not access different memory.
It may use any way it feels like to achieve that goal.

The volatileness of a given access is something the optimizer needs to take
into account, not TBAA.

(and yes, you can get completely inconsistent results between different
alias analysis providers, where some will say mustalias and some will say
noalias for the same thing).


TL;DR You should fix LICM or AST, depending on your viewpoint.
You also have to take into account that TBAA does not obey  transitiveness
or other various nice properties,not to mention different AA levels giving
inconsistent results.

Only one result is "most correct" at the language semantic level, but both
results are correct at the IR level (ie both noalias and mustalias).  It
can use or not use whatever metadata it likes.


(apropos of nothing, the lack of transitivity/etc in TBAA is one of the
reasons effective partitioning schemes for aliasing information don't
usually work out :P.  Since AST is basically a partitioning scheme, you are
feeling the pain of the edge cases of TBAA)



On Sun, Feb 28, 2016 at 5:12 PM, Nuno Lopes via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> 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
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160228/892906de/attachment.html>