On Wed, Apr 24, 2013 at 4:03 PM, Richard Sandiford
<[email protected]> wrote:
> Richard Biener <[email protected]> writes:
>> On Wed, Apr 24, 2013 at 2:44 PM, Richard Sandiford
>> <[email protected]> wrote:
>>> Richard Biener <[email protected]> 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
