This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Fix ancient wrong-code with ?: (PR middle-end/81814)
- From: Marek Polacek <polacek at redhat dot com>
- To: Richard Biener <rguenther at suse dot de>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 17 Aug 2017 16:31:26 +0200
- Subject: Re: [PATCH] Fix ancient wrong-code with ?: (PR middle-end/81814)
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=polacek at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 5D8794A706
- References: <20170817124611.GH17069@redhat.com> <alpine.LSU.email@example.com>
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).
> It seems that with some refactoring the remaining transforms should
> be easily expressible as match.pd patterns now.
That'd be great.