wide-int branch now up for public comment and review

[Kenneth Zadeck]

On 08/29/2013 04:42 AM, Richard Biener wrote:
> On Wed, 28 Aug 2013, Kenneth Zadeck wrote:
>
>>>>>     Note that the bits above the precision are not defined and the
>>>>>     algorithms used here are careful not to depend on their value.  In
>>>>>     particular, values that come in from rtx constants may have random
>>>>>     bits.
>>> Which is a red herring.  It should be fixed.  I cannot even believe
>>> that sentence given the uses of CONST_DOUBLE_LOW/CONST_DOUBLE_HIGH
>>> or INTVAL/UINTVAL.  I don't see accesses masking out 'undefined' bits
>>> anywhere.
>>>
>> I can agree with you that this could be fixed.   But it is not necessary to
>> fix it.   The rtl level and most of the tree level has existed for a long time
>> by doing math within the precision.
>>
>> you do not see the masking at the rtl level because the masking is not
>> necessary.    if you never look beyond the precision you just do not care.
>> There is the issue with divide and mod and quite frankly the code on the trunk
>> scares me to death.   my code at the rtl level makes sure that everything is
>> clean since i can see if it is a udiv or a div that is enough info to clean
>> the value up.
>>
>>>> I have a feeling I'm rehashing a past debate, sorry, but rtx constants
>>>> can't
>>>> have random bits.  The upper bits must be a sign extension of the value.
>>>> There's exactly one valid rtx for each (value, mode) pair.  If you saw
>>>> something different then that sounds like a bug.  The rules should already
>>>> be fairly well enforced though, since something like (const_int 128) --
>>>> or (const_int 256) -- will not match a QImode operand.
>>> See.  We're saved ;)
>> this is richard's theory.   There is a block of code at wide-int.cc:175 that
>> is ifdefed out that checks to see if the rtl consts are canonical.   if you
>> bring it in, building the libraries fails very quickly.   The million dollar
>> question is this the only bug or is this the first bug of a 1000 bugs.   Your
>> comment about not seeing any masking cuts both ways.   There is very little
>> code on the way in if you create ints with GEN_INT that makes sure someone has
>> done the right thing on that side.   So my guess is that there are a lot of
>> failures and a large number of them will be in the ports.
> Well, clearly RTL code _expects_ constants to be properly extended.  I
> can reproduce the bug and I simply guess that's a matter of mismatching
> modes here (or performing an implicit truncation without properly
> extending the constant).
>
>      at /space/rguenther/src/svn/wide-int/gcc/combine.c:10086
> 10086                                                      GEN_INT
> (count));
> (gdb) l
> 10081
> 10082                 mask_rtx = GEN_INT (nonzero_bits (varop, GET_MODE
> (varop)));
> 10083
> 10084                 mask_rtx
> 10085                   = simplify_const_binary_operation (code,
> result_mode, mask_rtx,
> 10086                                                      GEN_INT
> (count));
>
> uses of GEN_INT are frowned upon ... for exactly that reason - the
> mask_rtx is not a proper RTL constant for SImode.
over time, the GEN_INTs will go away at the portable rtl level  as more 
of the code is transitioned to use wide-int.    The port story is not so 
good.   Any port that uses TI or beyond will likely evolve to using 
wide-int for the math and unifying the cases between CONST_INT and 
CONST_WIDE_INT.  (the wide-int constructor from rtl takes either and the 
constructor to rtl looks at the constant.) But a port like the mips that 
has no TI will likely never change.
>
> Btw, all this isn't a reason to not have a well-defined wide-int
> representation.  You just have (as you have for trees) to properly
> canonize the representation at the time you convert from RTL
> constants to wide-int constants.
The wide-int representation is completely well defined.    This is what 
i cannot see why you do not understand.   You do not need to look above 
the precision so there is nothing undefined!!!!!!   I know this bothers 
you very badly and i will agree that if it is easy to clean up rtl to 
match this, then it would simplify the code somewhat.    But it is not 
undefined or random.   Furthermore, cleaning it will not change the abi 
so if rtl gets cleaner, we can change this.

> In the long run we want a uniform representation of constants
> so we can do zero-copying - but it looks like we now have
> three different representations - the tree one (sign or zero
> extended dependent on sign), RTL (garbage as you show) and
> wide-int (always sign-extended).
You wanted this to look more like double-int, now that it does, you now 
to see the flaws in it.    Do i ever get to win?   I do not think anyone 
would have been served well by trying to make trees have only signed 
constants in the way that double-int did.  I will admit that the 
implementation would be cleaner if we could just change everything else 
in the compiler to match a super clean wide-int design.   I took the 
other approach and bottled as much of the ugliness behind the a api and 
tried to keep the rest of the compiler looking mostly the way that it does.

To a first approximation, ugliness is a conserved force in the universe.

>
> That's why I was looking at at least matching what tree does
> (because tree constants _are_ properly canonized).
well, they are very close.    i am likely working on the "last" bug now 
for mips.
>
> Richard.