This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: patch to fix constant math -5th patch, rtl


On Wed, Apr 24, 2013 at 4:03 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Wed, Apr 24, 2013 at 2:44 PM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>> Can we in such cases please to a preparatory patch and change the
>>>> CONST_INT/CONST_DOUBLE paths to do an explicit [sz]ext to
>>>> mode precision first?
>>>
>>> I'm not sure what you mean here.  CONST_INT HWIs are already sign-extended
>>> from mode precision to HWI precision.  The 8-bit value 0xb10000000 must be
>>> represented as (const_int -128); nothing else is allowed.
>>> E.g. (const_int 128)
>>> is not a valid QImode value on BITS_PER_UNIT==8 targets.
>>
>> Yes, that's what I understand.  But consider you get a CONST_INT that is
>> _not_ a valid QImode value.
>
> But that's invalid :-)  It is not valid to call:
>
>       plus_constant (QImode, GEN_INT (128), 1)
>
> The point is that, even though it's invalid, we can't assert for it.

Why can't we assert for it?

> plus_constant is not for arbitrary precision arithmetic.  It's for
> arithmetic in a given non-VOIDmode mode.
>
>> Effectively a CONST_INT and CONST_DOUBLE is valid in multiple
>> modes and thus "arbitrary precision" with a limit set by the limit
>> of the encoding.
>
> The same CONST_INT and CONST_DOUBLE can be shared for several constants
> in different modes, yes, which is presumably what motivated making them
> VOIDmode in the first place.  E.g. zero is const0_rtx for every integer
> mode.  But in any given context, including plus_constant, the CONST_INT
> or CONST_DOUBLE has a specific mode.
>
>>>> Btw, plus_constant asserts that mode is either VOIDmode (I suppose
>>>> semantically do "arbitrary precision")
>>>
>>> No, not arbitrary precision.  It's always the precision specified
>>> by the "mode" parameter.  The assert is:
>>>
>>>   gcc_assert (GET_MODE (x) == VOIDmode || GET_MODE (x) == mode);
>>>
>>> This is because GET_MODE always returns VOIDmode for CONST_INT and
>>> CONST_DOUBLE integers.  The mode parameter is needed to tell us what
>>> precision those CONST_INTs and CONST_DOUBLEs actually have, because
>>> the rtx itself doesn't tell us.  The mode parameter serves no purpose
>>> beyond that.
>>
>> That doesn't make sense.  The only thing we could then do with the mode
>> is assert that the CONST_INT/CONST_DOUBLE is valid for mode.
>
> No, we have to generate a correct CONST_INT or CONST_DOUBLE result.
> If we are adding 1 to a QImode (const_int 127), we must return
> (const_int -128).  If we are adding 1 to HImode (const_int 127),
> we must return (const_int 128).  However...
>
>> mode does not constrain the result in any way, thus it happily produces
>> a CONST_INT (128) from QImode CONST_INT (127) + 1.  So, does the
>> caller of plus_constant have to verify the result is actually valid in the
>> mode it expects?  And what should it do if the result is not "valid"?
>
> ...good spot.  That's a bug.  It should be:
>
>       return gen_int_mode (INTVAL (x) + c, mode);
>
> rather than:
>
>       return GEN_INT (INTVAL (x) + c);
>
> It's a long-standing bug, because in the old days we didn't have
> the mode to hand.  It was missed when the mode was added.
>
> But the mode is also used in:
>
>       if (GET_MODE_BITSIZE (mode) > HOST_BITS_PER_WIDE_INT)
>         {
>           double_int di_x = double_int::from_shwi (INTVAL (x));
>           double_int di_c = double_int::from_shwi (c);
>
>           bool overflow;
>           double_int v = di_x.add_with_sign (di_c, false, &overflow);
>           if (overflow)
>             gcc_unreachable ();
>
>           return immed_double_int_const (v, VOIDmode);
>         }
>
> which is deciding whether the result should be kept as a HWI even
> in cases where the addition overflows.  It isn't arbitrary precision.

The above is wrong for SImode HOST_WIDE_INT and 0x7fffffff + 1
in the same way as the QImode case above.  It will produce
0x80000000.  The ICEing on "overflow" is odd as well as I'd have
expected twos-complement behavior which double-int, when
overflowing its 2 * HWI precision, provides.

I suppose the above should use immed_double_int_const (v, mode), too,
which oddly only ever truncates to mode for modes <= HOST_BITS_PER_WIDE_INT
via gen_int_mode.

Same of course for the code for CONST_DOUBLE.

Richard.

> Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]