[C PATCH] Discard P - (P + CST) optimization in pointer_diff (PR c/61240)

[Richard Biener]

On Mon, 4 Aug 2014, Marek Polacek wrote:

> On Mon, Aug 04, 2014 at 12:26:01PM +0200, Richard Biener wrote:
> > On Mon, 4 Aug 2014, Marek Polacek wrote:
> > 
> > > This PR is about bogus overflow warning that we issue for e.g.
> > >   int *q = &i + 1;
> > >   q - (q - 1);
> > > because pointer_diff receives p - (p + -1U) which gets simplified to
> > > 1U - with overflow.  We could drop the overflow flag to suppress the
> > > warning, but I think we should just remove the optimization
> > > altogether.  First, FE shouldn't perform such transformations at
> > > all.  Second, C++ FE has its own pointer_diff function that doesn't
> > > do such optimization.  With this patch, the C FE will generate the
> > > same expression as the C++ FE.  It's true that we should try to
> > > optimize this, but not in the front end.  It ought to be easy to
> > > write a pattern for match-and-simplify that would handle this.
> > 
> > I think that tree-ssa-forwprop.c already simplifies this in
> > associate_plusminus with (T)(P + A) - (T)P -> (T)A.  Well,
> > maybe not - but then the code should be massages to handle it.
> > 
> > Can you double-check and do that?
> 
> Looks like .fre can optimize "q - (q - 1)" into 1:
>    <bb 2>:
>    q.0_3 = (long int) &MEM[(void *)&i + 4B];
>    _5 = (long int) &i;
> -  _6 = q.0_3 - _5;
> -  t.1_7 = _6 /[ex] 4;
> -  t ={v} t.1_7;
> +  t ={v} 1;
>    i ={v} {CLOBBER};
>    return;
>  
> But associate_plusminus doesn't optimize it:
>           else if (code == MINUS_EXPR
>                    && CONVERT_EXPR_CODE_P (def_code)
>                    && TREE_CODE (gimple_assign_rhs1 (def_stmt)) == SSA_NAME
>                    && TREE_CODE (rhs2) == SSA_NAME)
>             {
>               /* (T)(P + A) - (T)P -> (T)A.  */
> becase gimple_assign_rhs1 (def_stmt) is not an SSA_NAME, but ADDR_EXPR (it's
> &MEM[(void *)&i + 4B]).  Then there's transformation A - (A +- B) -> -+ B
> below, but that doesn't handle casts.
> 
> So - should I try to handle it in associate_plusminus?

Yes please, with a (few) testcase(s).

Thanks,
Richard.