This is the mail archive of the
mailing list for the GCC project.
Re: patch to fix constant math -5th patch, rtl
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>, Kenneth Zadeck <zadeck at naturalbridge dot com>, Mike Stump <mikestump at comcast dot net>, gcc-patches <gcc-patches at gcc dot gnu dot org>, Lawrence Crowl <crowl at google dot com>, Ian Lance Taylor <iant at google dot com>, rdsandiford at googlemail dot com
- Date: Wed, 24 Apr 2013 15:36:09 +0200
- Subject: Re: patch to fix constant math -5th patch, rtl
- References: <506C72C7 dot 7090207 at naturalbridge dot com> <CAFiYyc1=8=LffiZ=EDBOy_uzn_ARdXk1OWxT=abYd8ot+iFp5Q at mail dot gmail dot com> <50891AAC dot 8090301 at naturalbridge dot com> <CAFiYyc15kmhRWhN3tsZqJDbZ5Uh6tVa45ssiYdsytLEfqaZ4zw at mail dot gmail dot com> <87y5im3orb dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <CAFiYyc2buJtu8RMKnLnvvb-A2=aYwopO+RBLPO6iJ3gKnq-hvg at mail dot gmail dot com> <87pq3y3kyk dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <CAFiYyc3NjOxpQ-Y9GDrQOET+dc3LXWuiuM=DxqmyASE8urRoWw at mail dot gmail dot com> <50912D85 dot 1070002 at naturalbridge dot com> <CAFiYyc2Q2UQPmkhExi2c8f-BSGLv+Rq1rOy4NdPQmTqSRE1A0A at mail dot gmail dot com> <5091331C dot 3030504 at naturalbridge dot com> <CAFiYyc1L6zuehE75dEfd_fB1-81F1fDHpL3kS=tbk6qAK3Texg at mail dot gmail dot com> <512D686B dot 90000 at naturalbridge dot com> <515EC4E7 dot 7040907 at naturalbridge dot com> <CAFiYyc3x6xb16kbYB9LzcC+XX17hM=R-h9KEWCZy6o0k-Csjfw at mail dot gmail dot com> <516DAF9B dot 3050008 at naturalbridge dot com> <516DB1F3 dot 8090908 at naturalbridge dot com> <CAFiYyc3P8sFfFQT95yr_UZSOs-JOUefvCm0bxQe1ZSUDgcMg_A at mail dot gmail dot com> <871ua0qer8 dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com>
On Wed, Apr 24, 2013 at 2:44 PM, Richard Sandiford
> Richard Biener <email@example.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,
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,