patch to fix constant math -5th patch, rtl

[Richard Biener]

On Fri, May 3, 2013 at 2:31 PM, Kenneth Zadeck <zadeck@naturalbridge.com> wrote:
> On 05/03/2013 08:12 AM, Richard Biener wrote:
>>
>> On Fri, May 3, 2013 at 1:49 PM, Kenneth Zadeck <zadeck@naturalbridge.com>
>> wrote:
>>>
>>> On 05/03/2013 07:34 AM, Richard Biener wrote:
>>>>
>>>> On Thu, Apr 25, 2013 at 1:18 AM, Kenneth Zadeck
>>>> <zadeck@naturalbridge.com> wrote:
>>>>>
>>>>> On 04/24/2013 11:13 AM, Richard Biener wrote:
>>>>>>
>>>>>> On Wed, Apr 24, 2013 at 5:00 PM, Richard Sandiford
>>>>>> <rdsandiford@googlemail.com>  wrote:
>>>>>>>
>>>>>>> Richard Biener<richard.guenther@gmail.com>  writes:
>>>>>>>>
>>>>>>>> On Wed, Apr 24, 2013 at 4:29 PM, Richard Sandiford
>>>>>>>> <rdsandiford@googlemail.com>  wrote:
>>>>>>>>>
>>>>>>>>> In other words, one of the reasons wide_int can't be exactly 1:1
>>>>>>>>> in practice is because it is clearing out these mistakes (GEN_INT
>>>>>>>>> rather than gen_int_mode) and missing features (non-power-of-2
>>>>>>>>> widths).
>>>>>>>>
>>>>>>>> Note that the argument should be about CONST_WIDE_INT here,
>>>>>>>> not wide-int.  Indeed CONST_WIDE_INT has the desired feature
>>>>>>>> and can be properly truncated/extended according to mode at the time
>>>>>>>> we
>>>>>>>> build it
>>>>>>>> via immed_wide_int_cst (w, mode).  I don't see the requirement that
>>>>>>>> wide-int itself is automagically providing that truncation/extension
>>>>>>>> (though it is a possibility, one that does not match existing
>>>>>>>> behavior
>>>>>>>> of
>>>>>>>> HWI for CONST_INT or double-int for CONST_DOUBLE).
>>>>>>>
>>>>>>> I agree it doesn't match the existing behaviour of HWI for CONST_INT
>>>>>>> or
>>>>>>> double-int for CONST_DOUBLE, but I think that's very much a good
>>>>>>> thing.
>>>>>>> The model for HWIs at the moment is that you have to truncate results
>>>>>>> to the canonical form after every operation where it matters.  As you
>>>>>>> proved in your earlier message about the plus_constant bug, that's
>>>>>>> easily
>>>>>>> forgotten.  I don't think the rtl code is doing all CONST_INT
>>>>>>> arithmetic
>>>>>>> on full HWIs because it wants to: it's doing it because that's the
>>>>>>> way
>>>>>>> C/C++ arithmetic on primitive types works.  In other words, the
>>>>>>> current
>>>>>>> CONST_INT code is trying to emulate N-bit arithmetic (for gcc runtime
>>>>>>> N)
>>>>>>> using a single primitive integer type.  wide_int gives us N-bit
>>>>>>> arithmetic
>>>>>>> directly; no emulation is needed.
>>>>>>
>>>>>> Ok, so what wide-int provides is integer values encoded in 'len' HWI
>>>>>> words that fit in 'precision' or more bits (and often in less).
>>>>>> wide-int
>>>>>> also provides N-bit arithmetic operations.  IMHO both are tied
>>>>>> too closely together.  A give constant doesn't really have a
>>>>>> precision.
>>>>>> Associating one with it to give a precision to an arithmetic operation
>>>>>> looks wrong to me and are a source of mismatches.
>>>>>>
>>>>>> What RTL currently has looks better to me - operations have
>>>>>> explicitely specified precisions.
>>>>>
>>>>> I have tried very hard to make wide-int work very efficiently with both
>>>>> tree
>>>>> and rtl without biasing the rep towards either representation.  Both
>>>>> rtl
>>>>> and
>>>>> trees constants have a precision.   In tree, constants are done better
>>>>> than
>>>>> in rtl because the tree really does have a field that is filled in that
>>>>> points to a type. However, that does not mean that rtl constants do not
>>>>> have
>>>>> a precision: currently you have to look around at the context to find
>>>>> the
>>>>> mode of a constant that is in your hand, but it is in fact always
>>>>> there.
>>>>> At the rtl level, you can see the entire patch - we always find an
>>>>> appropriate mode.
>>>>
>>>> Appearantly you cannot.  See Richard S. examples.
>>>>
>>>> As of "better", the tree has the issue that we have so many unshared
>>>> constants because they only differ in type but not in their
>>>> representation.
>>>> That's the nice part of RTL constants all having VOIDmode ...
>>>>
>>>> Richard.
>>>
>>> I said we could always find a mode, i did not say that in order to find
>>> the
>>> mode we did not have to stand on our head, juggle chainsaws and say
>>> "mother
>>> may i".   The decision to leave the mode as void in rtl integer constants
>>> was made to save space, but comes with an otherwise very high cost and in
>>> today's world of cheap memory seems fairly dated.   It is a decision that
>>> i
>>> and others would love to change and the truth is wide int is one step in
>>> that direction (in that it gets rid of the pun of using double-int for
>>> both
>>> integers and floats where the discriminator is voidmode for ints.) But
>>> for
>>> now we have to live with that poor decision.
>>
>> As far as I have read your wide-int patches the CONST_WIDE_INT RTX
>> object does not include a mode.  So I don't see it as a step forward in
>> any way (other than that it makes it explicit that you _do_ need a mode
>> to do any operation on a constant).
>>
>> Richard.
>
> There are several problems with just dropping a mode into the already
> existing mode field of an rtx constant.
> 1) There may be places where the a back end is testing equality to see if
> constants of different modes are in fact the same value.

That supposedly only happens in places where both RTX objects are
know to be constants.  Which makes me guess that it's in 99% of the
cases a comparison against one of the static RTX objects like
const0_rtx - thus easily greppable for (and easily converted similar
to the tree case where we have predicates for such tests like integer_zerop ()).
The remaining cases would be missed optimizations at most.

> 2) Most of the places what build int constants use GEN_INT which does not
> take a mode, even though about 95% of those places have a mode right there
> and the rest just take a little work.    There are constructor that do take
> a mode, but in the end they just throw the mode on the floor.

The fix is easy - make GEN_INT take a mandatory mode argument.
(and fix the fallout ...)

> 3) The canonical test to see if a CONST_DOUBLE contains an int or float is
> to test if the mode is VOIDmode.

I think you addressed this already by introducing CONST_DOUBLE_AS_INT_P ().

> Any port that is converted to have TARGET_SUPPORTS_WIDE_INT has no more of
> problem (3).   I admit that rooting out (1) is likely to be the worst of the
> problems.   But we were careful to at least make this work move us in the
> correct direction.

Well, you were careful to not walk in the wrong direction.  But I cannot see
were you get closer to fix any of 1-3 (apart from considering the new predicates
being that, or not overloading CONST_DOUBLE with floats and ints).

Richard.