This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] Fix PR middle-end/56474
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Eric Botcazou <ebotcazou at adacore dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 17 Apr 2013 10:11:22 +0200
- Subject: Re: [patch] Fix PR middle-end/56474
- References: <1630485 dot 0zg5rbKfGZ at polaris> <4747807 dot yAnva0Zvtb at polaris> <CAFiYyc07zkSiH44Q62Rgi4YhYfN+VOs-oYbSPo75kAY4T=Lz1g at mail dot gmail dot com> <22774122 dot QJzaNXD29q at polaris>
On Wed, Apr 17, 2013 at 1:12 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> For the C family I found exactly one - the layout_type case, and fixed
>> it in the FEs by making empty arrays use [1, 0] domains or signed domains
>> (I don't remember exactly). I believe the layout_type change was to make
>> Ada happy.
>
> I'm skeptical, I had to narrow down the initial kludge because it hurted Ada.
>
>> It may be that enabling overflow detection for even unsigned sizetype was
>> because of Ada as well. After all only Ada changed its sizetype sign
>> recently.
>
> Not true, overflow detection has _always_ been enabled for sizetypes.
> But sizetypes, including unsigned ones, were sign-extended so 0 -1 didn't
> overflow and we need that behavior back for Ada to work properly,
Yeah, well - they were effectively signed.
>> I don't like special casing 0 - 1 in a general compute function. Maybe
>> you want to use size_diffop for the computation? That would result in
>> a signed result and thus no overflow for 0 - 1.
>
> But it's not a general compute function, it's size_binop which is meant to be
> used for sizetypes only and which forces overflow on unsigned types. We need
> overflow detection for sizetypes but we can also tailor it to fit our needs.
I'm not against tailoring it to fit our needs - I'm just against special casing
behavior for specific values. That just sounds wrong.
Maybe we should detect overflow as if the input and output were signed
while computing an unsigned result. As far as I can see int_const_binop_1
does detect overflow as if operations were signed (it passes 'false' as
uns to all double-int operations rather than TYPE_UNSIGNED).
For example sub_with_overflow simply does
neg_double (b.low, b.high, &ret.low, &ret.high);
add_double (low, high, ret.low, ret.high, &ret.low, &ret.high);
*overflow = OVERFLOW_SUM_SIGN (ret.high, b.high, high);
which I believe is wrong. Shouldn't it be
neg_double (b.low, b.high, &ret.low, &ret.high);
HOST_WIDE_INT tem = ret.high;
add_double (low, high, ret.low, ret.high, &ret.low, &ret.high);
*overflow = OVERFLOW_SUM_SIGN (ret.high, tem, high);
? Because we are computing a + (-b) and thus OVERFLOW_SUM_SIGN
expects the sign of a and -b, not a and b to verify against the
sign of ret.
>> The other option is to for example disable overflow handling for _all_
>> constants and MINUS_EXPR (and then please PLUS_EXPR as well)
>> in size_binop. Maybe it's only the MULT_EXPR overflow we want to
>> know (byte-to-bit conversion / element size scaling IIRC).
>
> Well, we just need 0 - 1 because of the way we compute size expressions for
> variable-sized arrays.
I'm sceptical. Where do you compute the size expression for variable-sized
arrays? I suppose with the testcase in the initial patch I can then inspect
myself what actually happens?
Thanks,
Richard.
> --
> Eric Botcazou