Re: Patch: New implementation of -Wstrict-aliasing

[Silvius Rus <rus at google dot com>]

Andrew Pinski wrote:
> > Patch Description
> > =================
> >
> > The proposed patch contains a new implementation of -Wstrict-aliasing.
> > The proposed implementation lives in the backend and is based on 
> > flow-sensitive
> > points-to information, which is computed by analyzing the entire source of
> > each function.  It is not perfect (the problem is undecidable), but it 
> > improves
> > in all three directions: it checks whether pointers get dereferenced,
> > it detects aliasing across multiple statements and it produces more
> > meaningful messages.
> >     
>
> The only problem with this implementation I can see is that once
> we start folding *(int*)&float_var into using VIEW_CONVERT_EXPR, we
> are no longer going to warn about this specific aliasing issue (even
> though after that, we will get it correct, we should warn about it).
>
> Yes I am working on getting that specific optimization implemented.
> I mentioned about aliasing in
> http://gcc.gnu.org/ml/gcc-patches/2006-09/msg00117.html
>
> And Diego responed with:
> If we've warned then I have no problems in letting the folder ignore the
> alias sets.  Undefined code is fair game.  Wait for stage 1 for this
> one, though.
> -------------------------------------------------------------------------
> So I think we still need a warning in the front-end for the obvious
> aliasing issues.
>   
I have thought about leaving the warning in the front-end (perhaps 
improving a little on the current implementation) and use a different 
option level for the one in the backend.  Including your review, I have 
got three reasons for keeping it in the frontend:
1. You get some warnings at -O0.
2. You get warnings about the original source rather than after 
transformations such as folding.  Even if GCC happens to fix nonstandard 
code, users should be warned just in case GCC changes or they use other 
compilers.
3. You might be able to generate better warnings because you have direct 
access to language specific information.  In my experience, users got 
frustrated by two types of warnings: false positive and vague descriptions.

My reason for removing it from the frontend was:
1. Tried to keep it in just one place for simplicity.
2. It is hard to detect false positives in the front end.

Let me know what you think of this:
Keep the current check in the front-end, but give it two flavors:
1. Only warn when the converted pointer is dereferenced immediately: 
*(int*)&float_var.
2. The original, which warns even if it is not dereferenced: 
(int*)&float_var.
One of the two versions would be selected based on the value of --param 
wstrict-aliasing-accuracy.  I would go for version 1 included by 
default, because 2 gives false positives.

Thank you,
Silvius