On Wed, Apr 24, 2013 at 2:44 PM, Richard Sandiford
<rdsandif...@googlemail.com> wrote:
> Richard Biener <richard.guent...@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. Current code simply trusts that it is, given
the context from ...
>> What does wide-int do with VOIDmode mode inputs?
>> It seems to ICE on them for from_rtx and use garbage (0) for from_shwi. Ugh.
>
> ICEing is right. As mentioned before, every rtx constant has a mode,
> whether it's stored in the rtx or not. Callers must keep track of
> what that mode is.
... here. So I see that both CONST_INT and CONST_DOUBLE get their
mode (or in wide-int speak precision) from the context.
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.
>> 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.
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"?
Given that we do not verify the input values and do not care for mode for
producing the output value the current behavior of plus_constant is to
compute in arbitrary precision.
wide-int changes the above to produce a different result (CONST_INT (-128)).
I'd rather not have this patch-set introduce such subtle differences.
I'd like to see the following:
1) strip away 'precision' from wide-int, make the sign/zero-extend operations
that are now implicitely done explicit in the same way as done currently,
thus, ...
2) do a more-or-less 1:1 conversion of existing double-int code. double-int
code already has all sign/zero-extensions that are required for correctness.
and after merging in wide-int
3) see what common code can be factored out (wide_int::insert and friends)
4) if seems fit, introduce a wide_int_with_precision class that provides
a wrapper around wide_int and carries out operations in a fixed precision,
doing sign-/zero-extends after each operation (I suppose not much code
will be simplified by that)
before merging converting all targets is necessary - this isn't a part of
the infrastructure that can stand a partial conversion. I suspect that
conversion of all targets is much easier after 2), especially if most
of the double-int interfaces are not removed but their implementation
changed to work on wide-ints (just as I mentioned for the immed_double_int_const
case, but likely not restricted to that - CONST_DOUBLE_LOW/HIGH
can be converted to code that asserts the encoding is sufficiently small
for example). Thus,
5) piecewise remove legacy code dealing with CONST_DOUBLE
Btw, on 64bit hosts the rtx_def struct has 32bits padding before
the rtunion. I think 32bit hosts are legacy now, so using that 32bits
padding by storing 'len' there will make space-efficient conversion
of CONST_INT possible. Well, and avoids wasting another 32bits
of padding for CONST_WIDE on 64bit hosts.
> So if the rtx does specify a mode (everything except CONST_INT and
> CONST_DOUBLE), the assert is making sure that the caller has correctly
> tracked the rtx's mode and provided the right mode parameter. The caller
> must do that for all rtxes, it's just that we can't assert for it in the
> CONST_INT and CONST_DOUBLE case, because the rtx has no mode to check
> against. If CONST_INT and CONST_DOUBLE did have a mode to check against,
> there would be no need for the mode parameter at all. Likewise there
> would be no need for wide_int::from_rtx to have a mode parameter.
constants do not have an intrinsic mode or precision. They either do or
do not fit into a specific mode or precision. If an operation is to be carried
out in a specific mode or precision I expect the result of the operation
fits into that specific mode or precision (which oddly isn't what it does now,
appearantly).
Richard.
> Richard
