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

[Richard Biener <rguenther at suse dot de>]

On Tue, 5 Aug 2014, Jeff Law wrote:

> On 08/05/14 08:36, Marek Polacek wrote:
> > On Mon, Aug 04, 2014 at 02:04:36PM +0200, Richard Biener wrote:
> > > > 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).
> > 
> > Ok, so the following adds the (T)P - (T)(P + A) -> (T)-A
> > transformation.  It is based on code hunk that does the
> > (T)(P + A) - (T)P -> (T)A transformation.  The difference it makes is
> > in the .optimized dump something like:
> > 
> >   int fn2(int, int) (int p, int i)
> >   {
> > -  unsigned int p.2_2;
> > +  unsigned int _3;
> >     int _4;
> > -  unsigned int _5;
> >     unsigned int _6;
> > -  int _7;
> > 
> >     <bb 2>:
> > -  p.2_2 = (unsigned int) p_1(D);
> > -  _4 = p_1(D) + i_3(D);
> > -  _5 = (unsigned int) _4;
> > -  _6 = p.2_2 - _5;
> > -  _7 = (int) _6;
> > -  return _7;
> > +  _6 = (unsigned int) i_2(D);
> > +  _3 = -_6;
> > +  _4 = (int) _3;
> > +  return _4;
> > 
> > i.e., the PLUS_EXPR and MINUS_EXPR are gone, and NEGATE_EXPR is used
> > instead.
> > During bootstrap with --enable-languages=c,c++ this optimization triggered
> > 238 times.
> > 
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > 
> > 2014-08-05  Marek Polacek  <polacek@redhat.com>
> > 
> > 	PR c/61240
> > 	* tree-ssa-forwprop.c (associate_plusminus): Add (T)P - (T)(P + A)
> > 	-> (T)-A transformation.
> > c/
> > 	* c-typeck.c (pointer_diff): Remove P - (P + CST) optimization.
> > testsuite/
> > 	* gcc.dg/pr61240.c: New test.
> > 	* gcc.dg/tree-ssa/forwprop-29.c: New test.
> So I'm all for delaying folding when possible, so I'm comfortable with the
> general direction this is going.
> 
> My concern is the code we're removing discusses the need to simplify when
> these expressions are in static initializers.  What's going to ensure that
> we're still simplifying instances which appear in static initializers?  I
> don't see anything which tests that.   And does it still work for targets
> which utilize PSImode?

You mean stuff like

int i[4];
const int p = i - (i + 2);

?  I don't think that i - (i + 2) fulfills the requirements of an
integral constant expression, so we are free to reject it.

OTOH, ISTR that we tried hard in the past to support initializers
that implement offsetof in various ways.

Btw, before the idea of moving foldings to GIMPLE the consensus was
that moving foldings from frontends and convert to fold-const.c was
the appropriate thing to do.

As this is offsetof-like moving it to fold-const.c would work for
me as well (also given that forwprop is hardly a generic folding
framework).

Bah, I should accelerate that match-and-simplify stuff ...

Richard.