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

* Re: Patch: don't cap TYPE_PRECISION of bitsizetype at MAX_FIXED_MODE_SIZE
  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
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Biener @ 2019-05-29 13:16 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: GCC Patches

On Tue, May 28, 2019 at 5:43 PM Hans-Peter Nilsson
<hans-peter.nilsson@axis.com> wrote:
>
> 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."

I read it as the machine may not have ways to do basic
things like add two numbers in modes bigger than this
but you can use larger modes as simple bit "containers".

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

So why does cris-elf have 32 as MAX_FIXED_MODE_SIZE when it
can appearantly do DImode arithmetic just fine?  On x86_64
we end up with TImode which is MAX_FIXED_MODE_SIZE, on
32bit x86 it is DImode.

So - fix cris instead?

> 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

* Re: Patch: don't cap TYPE_PRECISION of bitsizetype at MAX_FIXED_MODE_SIZE
  2019-05-29 13:16 ` Richard Biener
@ 2019-06-05 19:38   ` Hans-Peter Nilsson
  2019-06-05 20:03     ` Eric Botcazou
  0 siblings, 1 reply; 10+ messages in thread
From: Hans-Peter Nilsson @ 2019-06-05 19:38 UTC (permalink / raw)
  To: richard.guenther; +Cc: gcc-patches

> From: Richard Biener <richard.guenther@gmail.com>
> Date: Wed, 29 May 2019 15:04:42 +0200

> On Tue, May 28, 2019 at 5:43 PM Hans-Peter Nilsson
> <hans-peter.nilsson@axis.com> wrote:
> >
> > 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."
> 
> I read it as the machine may not have ways to do basic
> things like add two numbers in modes bigger than this
> but you can use larger modes as simple bit "containers".

Quick dismissal of the "should actually" in the documentation and
of my code-digging findings noted.

Either way, for expediency, it sounds like you accept that
MAX_FIXED_MODE_SIZE can validly be set to just the bitsize of
Pmode for a target (for example, targets where Pmode=SImode and
DImode is just a 'bit-container'), so let's skip forward to...

> >  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.
> 
> So why does cris-elf have 32 as MAX_FIXED_MODE_SIZE when it
> can appearantly do DImode arithmetic just fine?

(For performance reasons, by choice rather than necessity.  Long
time ago; this was in the initial commit.  That's all incidental.)

>  On x86_64
> we end up with TImode which is MAX_FIXED_MODE_SIZE, on
> 32bit x86 it is DImode.
> 
> So - fix cris instead?

...here:

Umm no, this is not a CRIS-specific issue, that's just one
target where the issue was spotted.  See last for more targets.
Please don't suggest sweeping this bug under the carpet, for the
CRIS port or other targets, by tweaking their
MAX_FIXED_MODE_SIZE.  Also, as I mentioned, this can have other
unwanted effects; the macro is used elsewhere.  IMHO it's usage
should be replaced by more specific target settings.

This issue exists, not just for targets that can have their
MAX_FIXED_MODE_SIZE more-or-less easily tweaked higher, but also
for the 'bit-container' targets where it *can't* be set higher.

Let's please DTRT and correct the code here in the middle-end,
so we don't ICE for those targets and this line (gcc.dg/pr69973.c):
 typedef int v4si __attribute__ ((vector_size (1 << 29)));
(all listed targets happen to have Pmode == SImode)

So, considering that: ok to commit?

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

Targets with MAX_FIXED_MODE_SIZE == "bitsizeof (Pmode)"
By choice:
cris
mcore

By being "2*Pmode is a bit-container" -targets (condition):
ft32
h8300 ((TARGET_H8300H || TARGET_H8300S) && !TARGET_NORMAL_MODE)
moxie

brgds, H-P

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

* Re: Patch: don't cap TYPE_PRECISION of bitsizetype at MAX_FIXED_MODE_SIZE
  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:04       ` Hans-Peter Nilsson
  0 siblings, 2 replies; 10+ messages in thread
From: Eric Botcazou @ 2019-06-05 20:03 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: gcc-patches, richard.guenther

> This issue exists, not just for targets that can have their
> MAX_FIXED_MODE_SIZE more-or-less easily tweaked higher, but also
> for the 'bit-container' targets where it *can't* be set higher.
> 
> Let's please DTRT and correct the code here in the middle-end,
> so we don't ICE for those targets and this line (gcc.dg/pr69973.c):
>  typedef int v4si __attribute__ ((vector_size (1 << 29)));
> (all listed targets happen to have Pmode == SImode)
> 
> So, considering that: ok to commit?

You'd need to audit the effects on other targets though.  Are we sure that we 
want to do bitsizetype calculations in a larger mode on very embedded targets?

-- 
Eric Botcazou

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

* Re: Patch: don't cap TYPE_PRECISION of bitsizetype at MAX_FIXED_MODE_SIZE
  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
  1 sibling, 1 reply; 10+ messages in thread
From: Richard Biener @ 2019-06-06  7:59 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: Hans-Peter Nilsson, GCC Patches

On Wed, Jun 5, 2019 at 10:03 PM Eric Botcazou <ebotcazou@adacore.com> wrote:
>
> > This issue exists, not just for targets that can have their
> > MAX_FIXED_MODE_SIZE more-or-less easily tweaked higher, but also
> > for the 'bit-container' targets where it *can't* be set higher.
> >
> > Let's please DTRT and correct the code here in the middle-end,
> > so we don't ICE for those targets and this line (gcc.dg/pr69973.c):
> >  typedef int v4si __attribute__ ((vector_size (1 << 29)));
> > (all listed targets happen to have Pmode == SImode)
> >
> > So, considering that: ok to commit?
>
> You'd need to audit the effects on other targets though.  Are we sure that we
> want to do bitsizetype calculations in a larger mode on very embedded targets?

I didn't actually write it down but originally wanted - what about adding
a way for the target to specify what type to use for bitsize_type?
We do have SIZETYPE so say that if BITSIZETYPE is defined then
use that (otherwise fall back to the existing mechanism)?  There may
not be a C type that maps to DImode for cris and it's not that
I like those C type names very much (probably a way to make the
macros independent of the chosen multilib?), so eventually a
BITSIZEMODE or BITSIZE_LARGER_THAN_MAX_FIXED_MODE_SIZE
macro?  That said, if BITSIZETYPE would work I'd prefer that just
for consistency.

Richard.

> --
> Eric Botcazou

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

* Re: Patch: don't cap TYPE_PRECISION of bitsizetype at MAX_FIXED_MODE_SIZE
  2019-06-05 20:03     ` Eric Botcazou
  2019-06-06  7:59       ` Richard Biener
@ 2019-06-06 14:04       ` Hans-Peter Nilsson
  2019-06-06 14:16         ` Hans-Peter Nilsson
  2019-06-07  7:02         ` Richard Biener
  1 sibling, 2 replies; 10+ messages in thread
From: Hans-Peter Nilsson @ 2019-06-06 14:04 UTC (permalink / raw)
  To: ebotcazou; +Cc: gcc-patches, richard.guenther

> From: Eric Botcazou <ebotcazou@adacore.com>
> Date: Wed, 05 Jun 2019 22:03:04 +0200

> > This issue exists, not just for targets that can have their
> > MAX_FIXED_MODE_SIZE more-or-less easily tweaked higher, but also
> > for the 'bit-container' targets where it *can't* be set higher.
> > 
> > Let's please DTRT and correct the code here in the middle-end,
> > so we don't ICE for those targets and this line (gcc.dg/pr69973.c):
> >  typedef int v4si __attribute__ ((vector_size (1 << 29)));
> > (all listed targets happen to have Pmode == SImode)
> > 
> > So, considering that: ok to commit?
> 
> You'd need to audit the effects on other targets though.  Are we sure that we 
> want to do bitsizetype calculations in a larger mode on very embedded targets?

IIUC, the precision of the bitsizetype is used on the host.
(Which is bad for native builds.)  When bitsizetype objects end
up on the target, they use the actual Pmode and not the larger
precision mode.

The only "other targets" affected are the one I listed, where
it's needed in order to be able to detect near-address-range
overflow, as shown.

So, it's a question of correctness, not want.

Are you suggesting I need to follow where the precision of
bitsizetype ends up in target code?  If so, can you please do
that for Ada?  You may already have the answer for that.

brgds, H-P

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

* Re: Patch: don't cap TYPE_PRECISION of bitsizetype at MAX_FIXED_MODE_SIZE
  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
  1 sibling, 1 reply; 10+ messages in thread
From: Hans-Peter Nilsson @ 2019-06-06 14:16 UTC (permalink / raw)
  To: gcc-patches; +Cc: ebotcazou, richard.guenther

> Date: Thu, 6 Jun 2019 16:04:47 +0200
> From: Hans-Peter Nilsson <hp@axis.com>

> When bitsizetype objects end
> up on the target, they use the actual Pmode and not the larger
> precision mode.

Oops, a half-way-done email slipped away, this part still needs
to be investigated.

I don't really know where the precision of bitsizetype ends up;
if it's a host-thing-only or a leaks into target code.  Anyone?

brgds, H-P

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

* Re: Patch: don't cap TYPE_PRECISION of bitsizetype at MAX_FIXED_MODE_SIZE
  2019-06-06  7:59       ` Richard Biener
@ 2019-06-06 14:40         ` Hans-Peter Nilsson
  0 siblings, 0 replies; 10+ messages in thread
From: Hans-Peter Nilsson @ 2019-06-06 14:40 UTC (permalink / raw)
  To: richard.guenther; +Cc: ebotcazou, gcc-patches

> MIME-Version: 1.0
> X-Axis-User: NO
> X-Axis-NonUser: YES
> X-Virus-Scanned: Debian amavisd-new at bastet.se.axis.com
> X-Spam-Score: 1.102
> X-Spam-Level: *
> X-Spam-Status: No, score=1.102 required=5 tests=[DKIM_ADSP_CUSTOM_MED=0.001,
> 	DKIM_INVALID=0.1, DKIM_SIGNED=0.1, FREEMAIL_FROM=0.001,
> 	NML_ADSP_CUSTOM_MED=0.9, RCVD_IN_DNSWL_NONE=-0.0001]
> 	autolearn=no autolearn_force=no
> Authentication-Results: bastet.se.axis.com (amavisd-new);
> 	dkim=fail (2048-bit key) reason="fail (message has been altered)"
> 	header.d=gmail.com
> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
>         d=gmail.com; s=20161025;
>         h=mime-version:references:in-reply-to:from:date:message-id:subject:to
>          :cc;
>         bh=AAYGM2axRy+4hoUcsyx/QB2xNqS0+rKeO265TdzbaYs=;
>         b=EvVgla8PF8FUMiRf82HqSQYHX78PzrU+GiXPiqw2uC24Fqu/gGFiLDj4IBbKaszKDi
>          s8GoNR6NbyflH1aoj2GunbYJNvUI4VibPEVbviVNYdyTCHV0q6TGGYaE5cZoo2UWtcK/
>          0d/ZKMOM4Qjje9+r0rSTMIJZWTJ0/sVd0NS1euJuPthYVNmvVpb7AB/PhJh54CDHQDSR
>          9PEhf7dFxqV92mf8+GI0tOCQ+nm9Y71dVZCwh1k/Tu0Y1TTwhuq5IepHmVE77z/yNuHA
>          A12vhQcQfjhAP8V+W/BMJHiJAHUDjZPzEH49e01LiYsbVAKr+KOdr5cNTz5Bv4ItW79W
>          D9dQ==
> X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
>         d=1e100.net; s=20161025;
>         h=x-gm-message-state:mime-version:references:in-reply-to:from:date
>          :message-id:subject:to:cc;
>         bh=AAYGM2axRy+4hoUcsyx/QB2xNqS0+rKeO265TdzbaYs=;
>         b=orrCZrV+/CuKSTflfzU8FAJQ7oMcW5ZSEWJODWJy4CHX8ZdR0RzkcWS/SullfhG1cJ
>          KEt3DVxCK9szmKsUkBXIXxmGvJEqma1RmybFDitV0kcN2VDh60YPdp75AcLkCfgD6Fez
>          epoB4IbjbyQMf20/pYpiyFRUEDVBa/UVkKvB5bzD5NxdNxQfL3x5J6fOvRhaGmk+1QzO
>          AfVcQ+gvpKxHLwyuDJzs4OG8YnGxbmGAae3xh0PWWj2sBx7Tb+8h49Eh4VRmMOi9JprK
>          5mgGos8VM3rpyo6W3adAqrCgPy1MC/yITWetaawNdNP+l4aGsNSxqZBdQwZAfczwEMGg
>          rJGA==
> X-Gm-Message-State: APjAAAU+9Bb82pA2/UKZGhKnzsPN9t+7zZadQNJrSS7iIplrqHOmuzm8
> 	bPGmBk4MWcN13b4Pjd316YowZiEFUVjiy8t8CTTqCg==
> X-Google-Smtp-Source: APXvYqxOm2u4IqYKwhXP3MEEu2jy+NePCHnhKcBVCF7HIHkdksucHqDhQPlCymvdaoRcj41yORfaHxfBZo5iaRZ8cv4=
> X-Received: by 2002:a19:2981:: with SMTP id p123mr22301587lfp.190.1559807980081;
>  Thu, 06 Jun 2019 00:59:40 -0700 (PDT)
> References: <201906051938.x55JcSsW016232@ignucius.se.axis.com>
>  <18571728.MIQ1nkMWVm@polaris>
> From: Richard Biener <richard.guenther@gmail.com>
> Date: Thu, 6 Jun 2019 09:59:29 +0200
> Cc: Hans-Peter Nilsson <hp@axis.com>,
> 	GCC Patches <gcc-patches@gcc.gnu.org>
> Old-Content-Type: text/plain; charset="UTF-8"
> X-TM-AS-GCONF: 00
> X-TM-AS-Product-Ver: IMSVA-9.1.0.1817-8.5.0.1020-24660.005
> X-TMASE-Version: IMSVA-9.1.0.1817-8.5.1020-24660.005
> X-TMASE-Result: 10--13.361300-10.000000
> X-TMASE-MatchedRID: OCgf9vSZjhKtHGjI+4ePLkKGB4JJ2ELXO4rmf5nWGLY2lSfrRmNlMlOi
> 	wGvrPOJI/ye/3Hc9K1qfhUT+CqHntkvOGeNuCS0Sx5sgyUhLCNtrLj3DxYBIN1eIuu+Gkot87vf
> 	jHqfMw2LuANzOai4fLwLhmAWwrROCua8xKml5Zs2ar6Iu0UJj0ndYbPDVqm8dNgrwTjLio7iKXt
> 	SrhR1F6LfTxRyZysPB3S9otnhJMWXMgfjGGlvykOXSonB/2H+nF9s8UTYYetXNWDA/tkxh/2BSh
> 	v8puDpLUAsHThNBbWPj2iOyGc7mHghiPDSyUlz9L9Kx8SxUYHtBmlBF/IJ0fFdZsJXctXRjVVrM
> 	hkGIRMOxNUU9LT2EMIwve9GnFZLPHDOmeQqRrUyeAiCmPx4NwFkMvWAuahr8JnwEOk8wQnYqtq5
> 	d3cxkNd6Gc5JbLppQbfv6+tC0kEv7tmrl9YnUIaKw1O9DfwWCpRyUja5VPhXrpcchznD6Bw==
> X-TMASE-SNAP-Result: 1.821001.0001-0-1-12:0,22:0,33:0,34:0-0
> X-RBL-Checked: 10.0.5.15 10.0.5.26 10.0.5.60 10.20.1.11 127.0.0.1 209.85.167.48
> Content-Transfer-Encoding: 8bit
> Content-Type: TEXT/plain; charset=iso-8859-1
> 
> On Wed, Jun 5, 2019 at 10:03 PM Eric Botcazou <ebotcazou@adacore.com> wrote:
> >
> > > This issue exists, not just for targets that can have their
> > > MAX_FIXED_MODE_SIZE more-or-less easily tweaked higher, but also
> > > for the 'bit-container' targets where it *can't* be set higher.
> > >
> > > Let's please DTRT and correct the code here in the middle-end,
> > > so we don't ICE for those targets and this line (gcc.dg/pr69973.c):
> > >  typedef int v4si __attribute__ ((vector_size (1 << 29)));
> > > (all listed targets happen to have Pmode == SImode)
> > >
> > > So, considering that: ok to commit?
> >
> > You'd need to audit the effects on other targets though.  Are we sure that we
> > want to do bitsizetype calculations in a larger mode on very embedded targets?
> 
> I didn't actually write it down but originally wanted - what about adding
> a way for the target to specify what type to use for bitsize_type?
> We do have SIZETYPE so say that if BITSIZETYPE is defined then
> use that (otherwise fall back to the existing mechanism)?  There may
> not be a C type that maps to DImode for cris

(Again: 64-bit types work fine for CRIS, it's just the cooked-up
middle-end expressions that shouldn't use 64-bit-entities.
Like, extracting bytes out of a 8-byte vector type.  Not sure if
that actually happens, but MAX_FIXED_MODE_SIZE is used in
situations like that, where the data wasn't originally a 64-bit
scalar.)

> and it's not that
> I like those C type names very much (probably a way to make the
> macros independent of the chosen multilib?), so eventually a
> BITSIZEMODE or BITSIZE_LARGER_THAN_MAX_FIXED_MODE_SIZE
> macro?  That said, if BITSIZETYPE would work I'd prefer that just
> for consistency.

I kind of *like* this suggestion (and agree about BITSIZETYPE)
only because it'd be a more specific mechanism.

I don't know if it's the right way though, maybe it's totally
unnecessary; I guess I should do some grepping and stalking of
bitsizetype now.

brgds, H-P
PS: To celebrate the day: "grep" (noun) is Swedish for pitchfork. ;)
(as a verb, "seized")

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

* Re: Patch: don't cap TYPE_PRECISION of bitsizetype at MAX_FIXED_MODE_SIZE
  2019-06-06 14:16         ` Hans-Peter Nilsson
@ 2019-06-07  7:00           ` Richard Biener
  0 siblings, 0 replies; 10+ messages in thread
From: Richard Biener @ 2019-06-07  7:00 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: GCC Patches, Eric Botcazou

On Thu, Jun 6, 2019 at 4:15 PM Hans-Peter Nilsson
<hans-peter.nilsson@axis.com> wrote:
>
> > Date: Thu, 6 Jun 2019 16:04:47 +0200
> > From: Hans-Peter Nilsson <hp@axis.com>
>
> > When bitsizetype objects end
> > up on the target, they use the actual Pmode and not the larger
> > precision mode.
>
> Oops, a half-way-done email slipped away, this part still needs
> to be investigated.
>
> I don't really know where the precision of bitsizetype ends up;
> if it's a host-thing-only or a leaks into target code.  Anyone?

It definitely ends up in target code.  I don't remember if it only
happens for Ada but that may very well be the case (I think
you need bit-precision layout of variable sized stuff).

Richard.

> brgds, H-P

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

* Re: Patch: don't cap TYPE_PRECISION of bitsizetype at MAX_FIXED_MODE_SIZE
  2019-06-06 14:04       ` Hans-Peter Nilsson
  2019-06-06 14:16         ` Hans-Peter Nilsson
@ 2019-06-07  7:02         ` Richard Biener
  1 sibling, 0 replies; 10+ messages in thread
From: Richard Biener @ 2019-06-07  7:02 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: Eric Botcazou, GCC Patches

On Thu, Jun 6, 2019 at 4:04 PM Hans-Peter Nilsson
<hans-peter.nilsson@axis.com> wrote:
>
> > From: Eric Botcazou <ebotcazou@adacore.com>
> > Date: Wed, 05 Jun 2019 22:03:04 +0200
>
> > > This issue exists, not just for targets that can have their
> > > MAX_FIXED_MODE_SIZE more-or-less easily tweaked higher, but also
> > > for the 'bit-container' targets where it *can't* be set higher.
> > >
> > > Let's please DTRT and correct the code here in the middle-end,
> > > so we don't ICE for those targets and this line (gcc.dg/pr69973.c):
> > >  typedef int v4si __attribute__ ((vector_size (1 << 29)));
> > > (all listed targets happen to have Pmode == SImode)
> > >
> > > So, considering that: ok to commit?
> >
> > You'd need to audit the effects on other targets though.  Are we sure that we
> > want to do bitsizetype calculations in a larger mode on very embedded targets?
>
> IIUC, the precision of the bitsizetype is used on the host.
> (Which is bad for native builds.)  When bitsizetype objects end
> up on the target, they use the actual Pmode and not the larger
> precision mode.

The lager precision mode ends up being used to compute a
bit offset, that divided by BITS_PER_UNIT will be used
in Pmode indeed.  But offset computations in TImode definitely
happen on x86_64.

> The only "other targets" affected are the one I listed, where
> it's needed in order to be able to detect near-address-range
> overflow, as shown.
>
> So, it's a question of correctness, not want.
>
> Are you suggesting I need to follow where the precision of
> bitsizetype ends up in target code?  If so, can you please do
> that for Ada?  You may already have the answer for that.
>
> 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).