On Fri, Sep 29, 2023 at 11:30:06AM +0100, Richard Sandiford wrote:
> Yeah, think I agree with this. widest_int really combined two things:
>
> (a) a way of storing any integer IL value without loss of precision
>
> (b) a way of attaching sign information
>
> Arithmetic on widest_int is dubious, because it wraps at an essentially
> arbitrary point.
Yeah, but some more sensible than others. Logical ops make sense at any
precision, addition/subtraction might better have for widest_int a few bits
of extra maximum precision over wide_int maximum supported precision such
that overflows are caught, but say for multiplication that would be much
more. Of course, we already document that bswap/rotates don't make any
sense on widest_int, and as I wrote, e.g. lrshift or unsigned division
of values with MSB set are very questionable too.
> The approach in the patch looks good to me from a quick scan FWIW.
> Will try to review over the weekend.
For the actual patch I have another worry (but without the GTY widest_int
uses and slsr etc. addressed first it can't be easily verified).
wide_int_ref_storage has VAR_PRECISION like wide_int, while I've hacked up
get_binary_precision not to allocate uselessly for it a lot of memory,
I'm afraid any time we perform some operation on wide_int_refs created from
widest_int (so, they get in most cases reasonably small get_len () but
huge get_precision ()) we'd uselessly allocate 255 HOST_WIDE_INTs of memory
from heap. So maybe wide_int should also like widest_int in the patch
have u.val vs. u.valp decided based on estimated or later real get_len ()
rather than get_precision (). In the end, I think we should make sure that
unless _BitInt is seen in the sources, we don't really ever allocate any
heap memory in wide_int/widest_int. At least unless we change the number
of inline elements of the arrays for wide_int/widest_int, if we lower that
say to some hardcoded number of limbs on all arches (say 4 or 6 or 8, 9
x86-64 is kind of weird) that allocations happen only very rarely. Normal
128-bit precision math shouldn't trigger them certainly.
Jakub
