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

[Nuno Lopes via llvm-commits]

nlopes added a comment.

In http://reviews.llvm.org/D16986#363895, @dberlin wrote:

> "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)


I agree with your points.
I was only trying to make the case that maybe TBAA should be less aggressive in the presence of volatile stuff, i.e., it could switch off the analysis of metadata for volatile stuff.
My concern is that in the past there's been pain in handling volatile stuff, resulting in miscompilations.  Given that the perf of volatile stuff is probably a non-issue (I don't have any data; just guessing), I would prefer to go the conservative path.
Otherwise, LICM just needs a very simple patch.


http://reviews.llvm.org/D16986