public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Richard Biener <rguenther@suse.de>,
	gcc-patches@gcc.gnu.org, richard.sandiford@arm.com
Subject: Re: [PATCH] wide-int: Allow up to 16320 bits wide_int and change widest_int precision to 32640 bits [PR102989]
Date: Tue, 10 Oct 2023 20:13:37 +0200	[thread overview]
Message-ID: <ZSWUUSw9NiUE34hd@tucnak> (raw)
In-Reply-To: <mpt7cnue5nl.fsf@arm.com>

On Tue, Oct 10, 2023 at 06:41:50PM +0100, Richard Sandiford wrote:
> > On the wide_int side, I see
> >  155291 576
> > (supposedly because of bound_wide_int, where we create wide_int_ref from
> > the 576-bit precision bound_wide_int and then create 576-bit wide_int when
> > using unary or binary operation on that).
> 
> 576 bits seems quite a lot for a loop bound.  We're enterning near-infinite
> territory with 128 bits. :)  But I don't want to rehash old discussions.
> If we take this size for wide_int as given, then...

We could go for 128 bits bounds_wide_int and redo the stats given that.
Though maybe such tweaks can be done incrementally.

> > So, perhaps we could get away with say WIDEST_INT_MAX_INL_ELTS of 5 or 6
> > instead of 9 but keep WIDE_INT_MAX_INL_ELTS at 9 (or whatever is computed
> > from MAX_BITSIZE_MODE_ANY_INT?).  Or keep it at 9 for both (i.e. without
> > the third patch).
> 
> ...I agree we might as well keep the widest_int size the same for
> simplicity.  It'd only be worth distinguishing them if we have positive
> proof that it's worthwhile.
> 
> So going with patches 1 + 2 sounds good to me, but I don't have a strong
> preference.

Ok.

> > +  if (prec > WIDE_INT_MAX_INL_PRECISION && !high)
> > +    prec = (op1len + op2len + 1) * HOST_BITS_PER_WIDE_INT;
> 
> Changing the precision looked a bit dangerous at first, but I agree it
> looks correct in context, in that nothing later on seems to require prec
> to be the real precision of the number.  But I wonder whether we should
> instead do:
> 
>   if (!high)
>     prec = MIN ((op1len + op2len + 1) * HOST_BITS_PER_WIDE_INT, prec);

Ok, will try that (and in other spots too).

> so that the assumption gets a bit more testing.  Same idea for the others.
> I.e. in any case where we think it's safe to reduce a precision or
> length for out-of-line buffers, I think we should try to do the same
> for inline ones.
> 
> I'm not sure off-hand why + 1 is safe here but + 2 is needed for the
> write_val estimate.  Might be worth a comment in one place or the other.

As I wrote earlier, I'm not sure about the need for the + 1 above in
prec computation, perhaps just
  if (!high)
    prec = MIN ((op1len + op2len + 1) * HOST_BITS_PER_WIDE_INT, prec);
could work too (especially if the multiplication is always signed vs. signed
or unsigned vs. unsigned).  Even say
  HOST_WIDE_INT_MIN * HOST_WIDE_INT_MIN (i.e. op1len == op2len == 1) result
of 0x40000000'000000000'00000000'00000000 (for smallest signed^2) or
~(unsigned HOST_WIDE_INT) 0 * ~(unsigned HOST_WIDE_INT) 0 (largest
unsigned^2) of
0xffffffff'fffffffe'00000000'00000001
fits into prec 2; and for widest_int-like representation,
0xffffffff'ffffffffff wouldn't have op1len 1, but 2; but the + 1 on top of
that is needed because of the way wi_pack then works.  But guess will try to
play with it some more tomorrow.

> > @@ -2399,9 +2453,12 @@ from_int (int i)
> >  static void
> >  assert_deceq (const char *expected, const wide_int_ref &wi, signop sgn)
> >  {
> > -  char buf[WIDE_INT_PRINT_BUFFER_SIZE];
> > -  print_dec (wi, buf, sgn);
> > -  ASSERT_STREQ (expected, buf);
> > +  char buf[WIDE_INT_PRINT_BUFFER_SIZE], *p = buf;
> > +  unsigned len = wi.get_len ();
> > +  if (UNLIKELY (len > WIDE_INT_MAX_INL_ELTS))
> > +    p = XALLOCAVEC (char, len * HOST_BITS_PER_WIDE_INT / 4 + 4);
> 
> Is this size enough for decimal?  E.g. to go back to the 576-bit example,
> 2^575 is O(10^173), but 576/4+4 == 148.

generic_wide_ints with precision larger than HOST_BITS_PER_WIDE_INT and
length larger than 1 are always printed hexadecimally, we don't have code to
print it decimally (it wouldn't be terribly hard, we have divmod, but could
be expensive).  Though, I've always wondered why we at least for print_decs
don't print negative values as -0xwhatever, rather than 0xsomethinglarge.
Any change in this area could affect a lot of dumps though.

	Jakub


  reply	other threads:[~2023-10-10 18:13 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-09 10:55 Jakub Jelinek
2023-10-09 12:54 ` Richard Sandiford
2023-10-09 13:44   ` Jakub Jelinek
2023-10-09 18:28     ` Jakub Jelinek
2023-10-10 17:41       ` Richard Sandiford
2023-10-10 18:13         ` Jakub Jelinek [this message]
2023-10-09 14:59 ` [PATCH] wide-int: Remove rwide_int, introduce dw_wide_int Jakub Jelinek
2023-10-10  9:30   ` Richard Biener
2023-10-10  9:49     ` Jakub Jelinek
2023-10-10 13:42     ` [PATCH] dwarf2out: Stop using wide_int in GC structures Jakub Jelinek
2023-10-10 13:43       ` Richard Biener
2023-10-11 16:47 [PATCH] wide-int: Allow up to 16320 bits wide_int and change widest_int precision to 32640 bits [PR102989] Jakub Jelinek
2023-10-12 10:54 ` Richard Sandiford
2023-10-12 11:10   ` Jakub Jelinek
2023-10-12 11:34     ` Richard Sandiford
2023-10-12 11:10 ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZSWUUSw9NiUE34hd@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rguenther@suse.de \
    --cc=richard.sandiford@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).