public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Patch: don't cap TYPE_PRECISION of bitsizetype at MAX_FIXED_MODE_SIZE
@ 2019-05-28 15:47 Hans-Peter Nilsson
  2019-05-29 13:16 ` Richard Biener
  0 siblings, 1 reply; 10+ messages in thread
From: Hans-Peter Nilsson @ 2019-05-28 15:47 UTC (permalink / raw)
  To: gcc-patches

TL;DR: instead of capping TYPE_PRECISION of bitsizetype at
MAX_FIXED_MODE_SIZE, search for the largest fitting size from
scalar_int_mode modes supported by the target using
targetm.scalar_mode_supported_p.

---------
In initialize_sizetypes, MAX_FIXED_MODE_SIZE is used as an upper
limit to the *precision* of the bit size of the size-type
(typically address length) of the target, which is wrong.

The effect is that if a 32-bit target says "please don't cook up
pieces larger than a register size", then we don't get more
precision in address-related calculations than that, while the
bit-precision needs to be at least (precision +
LOG2_BITS_PER_UNIT + 1) with precision being the size of the
address, to diagnose overflows.  There are gcc_asserts that
guard this, causing ICE when broken.

This MAX_FIXED_MODE_SIZE usage comes from r118977 (referencing
PR27885 and PR28176) and was introduced as if
MAX_FIXED_MODE_SIZE is the size of the largest supported type
for the target (where "supported" is in the most trivial sense
as in can move and add).  But it's not.

MAX_FIXED_MODE_SIZE is arguably a bit vague, but documented as
"the size in bits of the largest integer machine mode that
should actually be used.  All integer machine modes of this size
or smaller can be used for structures and unions with the
appropriate sizes."  While in general the documentation
sometimes differs from reality, that's mostly right, with
"should actually be" meaning "is preferably": it's the largest
size that the target indicates as beneficial of use besides that
directly mapped from types used in the source code; sort-of a
performance knob.  (I did a static reality code check looking
for direct and indirect uses before imposing this my own
interpretation and recollection.)  Typical use is when gcc finds
that some operations can be combined and synthesized to
optionally use a wider mode than seen in the source (but mostly
copying).  Then this macro sets an upper limit to the those
operations, whether to be done at all or the chunk-size.
Unfortunately some of the effects are unintuitive and I wouldn't
be surprised if this de-facto affects ABI corners.  It's not
something you tweak more than once for a target.

Two tests pass with this fixed for cris-elf (MAX_FIXED_MODE_SIZE
32): gcc.dg/attr-vector_size.c and gcc.dg/pr69973.c, where the
lack of precision (32 bits instead of 64 bits for bitsizetype)
caused an consistency check to ICE, from where I tracked this.

Regarding the change, MAX_FIXED_MODE_SIZE is still mentioned but
just to initialize the fall-back largest-supported precision.
Sometimes the target supports no larger mode than that of the
address, like for a 64-bit target lacking support for larger
sizes (e.g. TImode), as in the motivating PR27885.  I guess they
can still get ICE for overflowing address-calculation checks,
but that's separate problem, fixable by the target.

I considered making a separate convenience function as well as
amending smallest_int_mode_for_size (e.g., an optional argument
to have smallest_mode_for_size neither abort or cap at
MAX_FIXED_MODE_SIZE) but I think the utility is rather specific
to this use.  We rarely want to both settle for a smaller type
than the one requested, and that possibly being larger than
MAX_FIXED_MODE_SIZE.
---------

Regtested cris-elf and x86_64-linux-gnu, and a separate
cross-build for hppa64-hp-hpux11.11 to spot-check that I didn't
re-introduce PR27885.  (Not a full cross-build, just building
f951 and following initialize_sizetypes in gdb to see TRT
happening.)

Ok to commit?

gcc:
	* stor-layout.c (initialize_sizetypes): Set the precision
	of bitsizetype to the size of largest integer mode
	supported by target, not necessarily MAX_FIXED_MODE_SIZE.

--- gcc/stor-layout.c.orig	Sat May 25 07:12:49 2019
+++ gcc/stor-layout.c	Tue May 28 04:29:10 2019
@@ -2728,9 +2728,36 @@ initialize_sizetypes (void)
 	gcc_unreachable ();
     }
 
-  bprecision
-    = MIN (precision + LOG2_BITS_PER_UNIT + 1, MAX_FIXED_MODE_SIZE);
-  bprecision = GET_MODE_PRECISION (smallest_int_mode_for_size (bprecision));
+  bprecision = precision + LOG2_BITS_PER_UNIT + 1;
+
+  /* Find the precision of the largest supported mode equal to or larger
+     than needed for the bitsize precision.  This may be larger than
+     MAX_FIXED_MODE_SIZE, which is just the largest preferred size. */
+  machine_mode bpmode;
+  unsigned int largest_target_precision = MAX_FIXED_MODE_SIZE;
+
+  FOR_EACH_MODE_IN_CLASS (bpmode, MODE_INT)
+    {
+      scalar_int_mode smode = scalar_int_mode::from_int (bpmode);
+      unsigned int mode_prec = GET_MODE_PRECISION (smode);
+
+      if (!targetm.scalar_mode_supported_p (smode))
+	continue;
+
+      if (mode_prec > largest_target_precision)
+	largest_target_precision = mode_prec;
+
+      if (mode_prec >= (unsigned int) bprecision)
+	{
+	  bprecision = mode_prec;
+	  break;
+	}
+    }
+
+  /* Fall back to the largest known supported size. */
+  if (bpmode == VOIDmode)
+    bprecision = largest_target_precision;
+
   if (bprecision > HOST_BITS_PER_DOUBLE_INT)
     bprecision = HOST_BITS_PER_DOUBLE_INT;
 
brgds, H-P

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-06-07  7:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-28 15:47 Patch: don't cap TYPE_PRECISION of bitsizetype at MAX_FIXED_MODE_SIZE Hans-Peter Nilsson
2019-05-29 13:16 ` Richard Biener
2019-06-05 19:38   ` Hans-Peter Nilsson
2019-06-05 20:03     ` Eric Botcazou
2019-06-06  7:59       ` Richard Biener
2019-06-06 14:40         ` Hans-Peter Nilsson
2019-06-06 14:04       ` Hans-Peter Nilsson
2019-06-06 14:16         ` Hans-Peter Nilsson
2019-06-07  7:00           ` Richard Biener
2019-06-07  7:02         ` Richard Biener

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