On Thu, 17 Aug 2017, Marek Polacek wrote:
> On Thu, Aug 17, 2017 at 04:17:54PM +0200, Richard Biener wrote:
> > On Thu, 17 Aug 2017, Marek Polacek wrote:
> >
> > > This PR is about wrong-code and has gone undetected for over 10 years (!).
> > > The issue is that e.g. the following
> > >
> > > (signed char) x == 0 ? (unsigned long long) x : 0
> > >
> > > was wrongly folded to 0, because fold_cond_expr_with_comparison will fold
> > > A != 0 ? A : 0 to 0. But for x = 0x01000000 this is wrong: (signed char)
> > > is 0,
> > > but (unsigned long long) x is not. The culprit is
> > > operand_equal_for_comparison_p
> > > which contains shorten_compare-like code which says that the above is
> > > safe to
> > > fold. The code harks back to 1992 so I thought it worth to just get rid
> > > of it.
> > >
> > > But I did some measurements and it turns out that substituting
> > > operand_equal_p
> > > for operand_equal_for_comparison_p prevents folding ~60000 times in
> > > bootstrap.
> > > So I feel uneasy about removing the function completely. Instead, I
> > > propose to
> > > remove just the part that is causing trouble. (Maybe I should also
> > > delete the
> > > first call to operand_equal_p in operand_equal_for_comparison_p.)
> > >
> > > Bootstrapped/regtested on x86_64-linux, ok for trunk? What about 7?
> >
> > Ok for trunk. Do you have numbers for this patch variant as well?
>
> Thanks. Yeah, I've gathered some, too. This patch prevents calling
> fold_cond_expr_with_comparison that would end up with non-NULL_TREE result
> 8322 times (all Ada files), this is the
> 11325 if (COMPARISON_CLASS_P (arg0)
> 11326 && operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
> arg1)
> 11327 && !HONOR_SIGNED_ZEROS (element_mode (arg1)))
> case; plus 648 times in the
> 11334 if (COMPARISON_CLASS_P (arg0)
> 11335 && operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
> op2)
> 11336 && !HONOR_SIGNED_ZEROS (element_mode (op2)))
> case (and a lot of that is coming from libgfortran/generated/*.c and
> reload.c).
So you should be able to extract a C testcase? I suspect sth like
long foo (long x, int y)
{
return y > x ? y : x;
}
to no longer be folded to return MAX_EXPR (x, (long) y).
That would be a shame btw.
Richard.
>
> > It seems that with some refactoring the remaining transforms should
> > be easily expressible as match.pd patterns now.
>
> That'd be great.
>
> Marek
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
