On Thu, Oct 4, 2012 at 9:27 PM, Richard Sandiford
<rdsandif...@googlemail.com> wrote:
> Kenneth Zadeck <zad...@naturalbridge.com> writes:
>> On 10/04/2012 12:58 PM, Richard Guenther wrote:
>>> On Thu, Oct 4, 2012 at 3:55 PM, Kenneth Zadeck <zad...@naturalbridge.com>
>>> wrote:
>>>> Let me talk about the mode here first.
>>>>
>>>> What this interface/patch provides is a facility where the constant math
>>>> that is done in optimizations is done exactly the way that it would be done
>>>> on the target machine. What we have now is a compiler that only does this
>>>> if it convenient to do on the host. I admit that i care about this more
>>>> than others right now, but if intel adds a couple of more instructions to
>>>> their vector units, other people will start to really care about this
>>>> issue.
>>>> If you take an OImode value with the current compiler and left shift it by
>>>> 250 the middle end will say that the result is 0. This is just wrong!!!
>>>>
>>>> What this means is that the bitsize and precision of the operations need to
>>>> be carried along when doing math. when wide-int checks for overflow on the
>>>> multiply or add, it is not checking the if the value overflowed on two
>>>> HWIs,
>>>> it is checking if the add overflowed in the mode of the types that are
>>>> represented on the target. When we do shift, we are not doing a shift
>>>> within two HWIs, we are truncating the shift value (if this is appropriate)
>>>> according to the bitsize and shifting according the precision.
>>>>
>>>> I think that an argument could be made that storing the mode should be
>>>> changed to an explicit precision and bitsize. (A possible other option
>>>> would be to store a tree type, but this would make the usage at the rtl
>>>> level very cumbersome since types are rare.) Aside from the work, you would
>>>> not get much push back.
>>>>
>>>> But the signess is a different argument. At the rtl level, the signess is
>>>> a matter of context. (you could argue that this is a mistake and i would
>>>> agree, but that is an even bigger change.) But more to the point, at the
>>>> tree level, there are a surprising number of places where the operation
>>>> desired does not follow the sign of the types that were used to construct
>>>> the constants. Furthermore, not carrying the sign is more consistent with
>>>> the double int code, which as you point out carries nothing.
>>> Well, on RTL the signedness is on the operation (you have sdiv and udiv,
>>> etc.).
>> yes, there is a complete enough set of operations that allow you to
>> specify the signess where this matters.
>>
>>> double-int tries to present a sign-less twos-complement entity of size
>>> 2 * HOST_BITS_PER_WIDE_INT. I think that is sensible and for
>>> obvious reasons should not change. Both tree and RTL rely on this.
>
> I disagree, at least from an RTL perspective. HOST_BITS_PER_WIDE_INT is
> a host property, and as Kenny says, it's not really sensible to tie our
> main target-specific IR to something host-specific. We've only done
> that for historical reasons.
Oh, I agree - that HOST_WIDE_INT provides the limit for the largest
integer constants we can encode on a target is a bug!
> On a target that's been converted to wide_int, I don't think a pair
> of HWIs (whether or not it's expressed as a double_int) has any
> significance at all at the RTL level.
>
> As far as the wide_ints recording a mode or precision goes: we're in
> the "lucky" position of having tried both options. Trees record the
> type (and thus precision) of all compile-time integer constants.
> RTL doesn't. And the RTL way is a right pain. It leads to interfaces
> in which rtx arguments often have to be accompanied by a mode. And it
> leads to clunky differences like those between simplify_unary_operation
> (which takes two mode arguments) and simplify_binary_operation
> (which with our current set of binary operations requires only one).
But the issue here is not that double_int doesn't record a mode
or precision (or a sign). The issue is that CONST_DOUBLE and CONST_INT
don't! The _tree_ INTEGER_CST contains of a type and a double-int.
I see double-int (and the proposed wide-int) as a common building-block
used for kind of "arbitrary precision" (as far as the target needs) integer
constants on both tree and RTL. And having a common worker implementation
requires it to operate on something different than tree types or RTL mode
plus signedness.
> To put it another way: every wide_int operation needs to know
> the precision of its arguments and the precision of its result.
That's not true. Every tree or RTL operation does, not every
wide_int operation. double_int's are just twos-complement numbers
of precision 2 * HOST_BITS_PER_WIDE_INT. wide_int's should
be just twos-complement numbers of precision len *
WHATEVER_HOST_COMPONENT_TYPE_IS_SUITABLE_FOR_A_FAST_IMPLEMENTATION.
Operations on double_int and wide_int are bare-metal,
in nearly all of the times you should use routines that do
proper sign-/zero-extending to a possibly smaller precision. When
using bare-metal you are supposed to do that yourself.
Which is why I was suggesting to add double_sint, double_uint,
double_sintp (with precision), etc., wrappers and operations.
> Not storing the precision in the wide_int is putting the onus
> on the caller to keep track of the precision separately.
But that's a matter of providing something high-level ontop of
the bare-metal double-int/wide-int (something shared between
RTL and trees). Duplicating information in the bare-metal
doesn't make sense (and no, INTEGER_CSTs on the tree level
are _not_ short-lived, and how can a double-int make sense on
the tree level when you say it doesn't make sense on the RTL level?)
> And that just leads to the possibility of mismatches (e.g. from
> accidentally passing the precision of a different wide_int instead;
> the corresponding rtx mistake is easily made when you have several
> variables with "mode" in their name).
Well, the mistake was obviously that CONST_INT/DOUBLE_INT
do not have a mode. You are passing those RTXen, you are not
passing double-ints! Don't blame double-ints for something that is
not their fault.
To me it looks like you want to create a CONST_WIDE, not a wide-int
(as you are not touching the tree side at all). I'm all for that if that
is progress for you. But make sure to provide an underlying
object type that is suitable for sharing between RTL and trees
(thus do not put in redundant information like a precision).
In the other mail Kenny mentioned that wide-ints are all of the same
compile-time, target specific size. That might be suitable for
CONST_WIDE (on RTL you still have CONST_INT for integers that
can be encoded smaller, and we only have one function live in RTL
at a time) - but for trees certainly INTEGER_CSTs can consume
a significant portion of GC memory, so it's not an option to enlarge
it from containing two HWIs to what is required for 2 * OImode.
But again there is enough redundant information in the INTEGER_CST
(it's type which contains its mode) that allows it to be of variable size.
> Storing the precision in the wide_int means that we can assert for
> such mismatches. And those kinds of assert are much easier to debug,
> because they trigger regardless of the integer values involved.
> In contrast, RTL-level mismatches between CONST_INTs/CONST_DOUBLEs
> and their accompanying mode have tended to trigger ICEs or wrong-code
> bugs only for certain relatively uncommon inputs. I.e. they're much
> more likely to be found in the field rather than before release.
>
> But let's say for the sake of argument that we went that way and didn't
> record the precision in the wide_int. We're unlikely to want interfaces
> like:
>
> div (precision_of_result, precision_of_op0, op0,
> precsion_of_op1, op1)
>
> because the three precisions are always going to be the same.
> We'd have:
>
> div (precision_of_result, op0, op1)
>
> instead. So in cases where we "know" we're only going to be calling
> wide_int interfaces like these, the callers will probably be lazy and
> not keep track of op0 and op1's precision separately. But the functions
> like sign and zero extension, popcount, etc., will definitely need to
> know the precision of their operand, and once we start using functions
> like those, we will have to make the caller aware of the precision of
> existing wide_ints. It's not always easy to retrofit precision-tracking
> code to the caller, and combine.c's handling of ZERO_EXTEND is one
> example where we ended up deciding it was easier not to bother and just
> avoid optimising ZERO_EXTENDs of constants instead. See also cselib,
> where we have to wrap certain types of rtl in a special CONST wrapper
> (with added mode) so that we don't lose track.
>
> And I think all that "need to know the precision" stuff applies to
> double_int, except that the precision of a double_int is always
> implicitly 2 * HOST_BITS_PER_WIDE_INT. We don't want to hardcode
> a similar precision for wide_int because it's always the precision
> of the current operation, not the precision of the target's widest mode
> (or whatever) that matters.
>
> We only get away with using double_int (or more generally a pair of HWIs)
> for RTL because (a) single HWI arithmetic tends to use a different path
> and (b) no-one is making active use of modes with precisions in the range
> (HOST_BITS_PER_WIDE_INT, 2 * HOST_BITS_PER_WIDE_INT). If they were,
> a lot of our current RTL code would be wrong, because we often don't
> truncate intermediate or final 2-HWI values to the precision of the
> result mode.
>
> One of the advantages of wide_int is that it removes this (IMO) artifical
> restriction "for free" (or in fact with less RTL code than we have now).
> And that's not just academic: I think Kenny said that his target had
> modes that are wider than 64 bits but not a power of 2 in width.
>
> Sorry for the rant. I just don't see any advantage to keeping the
> precision and integer separate.
Avoiding redundant information (which can get out of sync) for something
that is supposed to be shared between tree and RTL (is it?!).
So, I think you want a CONST_WIDE RTX that has
struct const_wide_rtx {
wide_int w;
enum machine_mode mode;
};
which, if I look close enough, we already have (in struct rtx_def we _do_
have a mode field).
So I'm not really sure what your issue is?
Look at RTL users of the double-int routines and provide wrappers
that take RTXen as inputs. Enforce that all CONSTs have a mode.
Richard.
> Richard
