[PATCH] Fix ancient wrong-code with ?: (PR middle-end/81814)

[Marek Polacek]

On Thu, Aug 17, 2017 at 04:36:02PM +0200, Richard Biener wrote:
> 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).

This is still folded to return MAX_EXPR <(long int) y, x>; so that's fine.
It's got to be something more complex that will not be handled now.  I'll
look tomorrow for a testcase.

	Marek