Re: wide-int branch now up for public comment and review

[Kenneth Zadeck <zadeck at naturalbridge dot com>]

> > >    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.

If richard is right then there will be changes.  The internals of 
wide-int will be changed so that everything is maintained in canonical 
form rather than just doing the canonization on the outside.   This will 
clean up things like fits_uhwi_p and a lot more of the wide-int internals.

But i think that if you want to change the compiler to use infinite 
precision arithmetic, you really ought to have a better reason that you 
think that it is cleaner.   Because, it buys you nothing for most of the 
compiler and it is slower AND it is a much bigger change to the compiler 
than the one we want to make.


> > This is probably the part of the representation that I disagree most with.
> > There seem to be two main ways we could hande the extension to whole HWIs:
> >
> > (1) leave the stored upper bits undefined and extend them on read
> > (2) keep the stored upper bits in extended form
> >
> > The patch goes for (1) but (2) seems better to me, for a few reasons:
> I agree whole-heartedly.
my statement is above.   if we can do 2 then we should.   But I do not 
think that it is worth several people-years to do this.
> > * As above, constants coming from rtl are already in the right form,
> >    so if you create a wide_int from an rtx and only query it, no explicit
> >    extension is needed.
> >
> > * Things like logical operations and right shifts naturally preserve
> >    the sign-extended form, so only a subset of write operations need
> >    to take special measures.
> >
> > * You have a public interface that exposes the underlying HWIs
> >    (which is fine with me FWIW), so it seems better to expose a fully-defined
> >    HWI rather than only a partially-defined HWI.
> >
> > E.g. zero_p is:
> >
> >    HOST_WIDE_INT x;
> >
> >    if (precision && precision < HOST_BITS_PER_WIDE_INT)
> >      x = sext_hwi (val[0], precision);
> >    else if (len == 0)
> >      {
> >        gcc_assert (precision == 0);
> >        return true;
> >      }
> >    else
> >      x = val[0];
> >
> >    return len == 1 && x == 0;
> >
> > but I think it really ought to be just:
> >
> >    return len == 1 && val[0] == 0;
> Yes!
>
> But then - what value does keeping track of a 'precision' have
> in this case?  It seems to me it's only a "convenient carrier"
> for
>
>    wide_int x = wide-int-from-RTX (y);
>    machine_mode saved_mode = mode-available? GET_MODE (y) : magic-mode;
>    ... process x ...
>    RTX = RTX-from-wide_int (x, saved_mode);
>
> that is, wide-int doesn't do anything with 'precision' but you
> can extract it later to not need to remember a mode you were
> interested in?
>
> Oh, and of course some operations require a 'precision', like rotate.
As i have said, when the operands and result are all precision correct, 
as they are generally in both rtl and tree, then the default fixed 
precision interface is really very clean.    I know you object to some 
implementation issues, but this is why we put the branch up, so you can 
see what you get from the other side.


> > >    When the precision is 0, all the bits in the LEN elements of
> > >    VEC are significant with no undefined bits.  Precisionless
> > >    constants are limited to being one or two HOST_WIDE_INTs.  When two
> > >    are used the upper value is 0, and the high order bit of the first
> > >    value is set.  (Note that this may need to be generalized if it is
> > >    ever necessary to support 32bit HWIs again).
> > I didn't understand this.  When are two HOST_WIDE_INTs needed for
> > "precision 0"?
> For the wide_int containing unsigned HOST_WIDE_INT ~0.  As we
> sign-extend the representation (heh, yes, we do or should!) we
> require an extra HWI to store the fact that ~0 is unsigned.
>
> > The main thing that's changed since the early patches is that we now
> > have a mixture of wide-int types.  This seems to have led to a lot of
> > boiler-plate forwarding functions (or at least it felt like that while
> > moving them all out the class).  And that in turn seems to be because
> > you're trying to keep everything as member functions.  E.g. a lot of the
> > forwarders are from a member function to a static function.
> >
> > Wouldn't it be better to have the actual classes be light-weight,
> > with little more than accessors, and do the actual work with non-member
> > template functions?  There seems to be 3 grades of wide-int:
> >
> >    (1) read-only, constant precision  (from int, etc.)
> >    (2) read-write, constant precision  (fixed_wide_int)
> >    (3) read-write, variable precision  (wide_int proper)
> >
> > but we should be able to hide that behind templates, with compiler errors
> > if you try to write to (1), etc.
> Yeah, I'm probably trying to clean up the implementation once I
> got past recovering from two month without GCC ...

> Richard.