This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix forwporp pattern (T)(P + A) - (T)P -> (T)A
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Bernd Edlinger <bernd dot edlinger at hotmail dot de>
- Cc: Eric Botcazou <ebotcazou at adacore dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 23 Jun 2014 10:40:53 +0200
- Subject: Re: [PATCH] Fix forwporp pattern (T)(P + A) - (T)P -> (T)A
- Authentication-results: sourceware.org; auth=none
- References: <DUB118-W5160114A82AC375D95A7A4E41C0 at phx dot gbl>
On Sun, Jun 22, 2014 at 9:14 AM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> Hi,
>
> I noticed that several testcases in the GMP-4.3.2 test suite are failing now which
> did not happen with GCC 4.9.0. I debugged the first one, mpz/convert, and found
> the file mpn/generic/get_str.c was miscompiled.
>
> mpn/get_str.c.132t.dse2:
> pretmp_183 = (sizetype) chars_per_limb_80;
> pretmp_184 = -pretmp_183;
> _23 = chars_per_limb_80 + 4294967295;
> _68 = (sizetype) _23;
> _28 = _68 + pretmp_184;
>
> mpn/get_str.c.133t.forwprop4:
> _28 = 4294967295;
>
>
> That is wrong, because chars_per_limb is unsigned, and it is not zero.
> So the right result should be -1. This makes the loop termination in that
> function fail.
>
> The reason for this is in this check-in:
>
> r210807 | ebotcazou | 2014-05-22 16:32:56 +0200 (Thu, 22 May 2014) | 3 lines
>
> * tree-ssa-forwprop.c (associate_plusminus): Extend (T)(P + A) - (T)P
> -> (T)A transformation to integer types.
>
>
> Because it implicitly assumes that integer overflow is not allowed with all types,
> including unsigned int.
Hmm? But the transform is correct if overflow wraps. And it's correct if
overflow is undefined as well, as (T)A is always well-defined (implementation
defined) if it is a truncation.
So we match the above an try to transform it to (T)P + (T)A - (T)P. That's
wrong if the conversion is extending I think.
Richard.
>
> The attached patch fixes these regressions, and because the reasoning depends
> on the TYPE_OVERFLOW_UNDEFINED attribute, a strict overflow warning has to be
> emitted here, at least for widening conversions.
>
>
> Boot-strapped and regression-tested on x86_64-linux-gnu with all languages, including Ada.
> OK for trunk?
+ if (!TYPE_SATURATING (TREE_TYPE (a))
this is already tested at the very beginning of the function.
+ && !FLOAT_TYPE_P (TREE_TYPE (a))
+ && !FIXED_POINT_TYPE_P (TREE_TYPE (a))
likewise.
+ || (!POINTER_TYPE_P (TREE_TYPE (p))
+ && INTEGRAL_TYPE_P (TREE_TYPE (a))
+ && TYPE_OVERFLOW_UNDEFINED (TREE_TYPE (a)))
INTEGRAL_TYPE_P are always !POINTER_TYPE_P.
>
> Thanks
> Bernd.
>