public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p
@ 2022-09-05  8:01 linkw at gcc dot gnu.org
  2022-09-05  8:09 ` [Bug middle-end/106833] " linkw at gcc dot gnu.org
                   ` (15 more replies)
  0 siblings, 16 replies; 17+ messages in thread
From: linkw at gcc dot gnu.org @ 2022-09-05  8:01 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

            Bug ID: 106833
           Summary: Handle OPAQUE_TYPE in
                    gimple_canonical_types_compatible_p
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: linkw at gcc dot gnu.org
  Target Milestone: ---

This is from one encountered ICE when using const type:
  https://gcc.gnu.org/pipermail/gcc-patches/2022-August/600751.html

As the above link, the ICE reproduction requires to use type
ptr_vector_quad_type_node for the pointer type build and under lto, function
gimple_canonical_types_compatible_p is intended to check if const __vector_quad
is canonical types compatible to __vector_quad, the arguments are: const
__vector_quad, __vector_quad and false.

For now, in function gimple_canonical_types_compatible_p we don't handle
OPAQUE_TYPE explicitly, so it will go to the code:

      /* Consider all types with language specific trees in them mutually
         compatible.  This is executed only from verify_type and false
         positives can be tolerated.  */
      gcc_assert (!in_lto_p);
      return true;

Since it's in wpa, the assertion failed.

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

* [Bug middle-end/106833] Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
@ 2022-09-05  8:09 ` linkw at gcc dot gnu.org
  2022-09-05  8:25 ` rguenth at gcc dot gnu.org
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: linkw at gcc dot gnu.org @ 2022-09-05  8:09 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

Kewen Lin <linkw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ice-checking
                 CC|                            |bergner at gcc dot gnu.org,
                   |                            |hubicka at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org,
                   |                            |segher at gcc dot gnu.org
             Target|                            |powerpc*-linux-gnu

--- Comment #1 from Kewen Lin <linkw at gcc dot gnu.org> ---
IMHO this is an omission when we were adding supports for opaque type, const
__vector_quad and __vector_quad should be taken as canonical_types_compatible.

I wonder if we can simply take it just like what it handles for "Non-aggregate
types", for example:

diff --git a/gcc/tree.cc b/gcc/tree.cc
index 2f488e4467c..555e96c59d5 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -13510,6 +13510,7 @@ gimple_canonical_types_compatible_p (const_tree t1,
const_tree t2,
       || TREE_CODE (t1) == VECTOR_TYPE
       || TREE_CODE (t1) == COMPLEX_TYPE
       || TREE_CODE (t1) == OFFSET_TYPE
+      || TREE_CODE (t1) == OPAQUE_TYPE
       || POINTER_TYPE_P (t1))
     {
       /* Can't be the same type if they have different recision.  */

Or adding one default hook which does the similar thing, and then if one target
needs some target specific checks on its opaque type, one specific hook can be
provided.

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

* [Bug middle-end/106833] Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
  2022-09-05  8:09 ` [Bug middle-end/106833] " linkw at gcc dot gnu.org
@ 2022-09-05  8:25 ` rguenth at gcc dot gnu.org
  2022-09-05  8:27 ` rguenth at gcc dot gnu.org
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-09-05  8:25 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Kewen Lin from comment #1)
> IMHO this is an omission when we were adding supports for opaque type, const
> __vector_quad and __vector_quad should be taken as
> canonical_types_compatible.
> 
> I wonder if we can simply take it just like what it handles for
> "Non-aggregate types", for example:
> 
> diff --git a/gcc/tree.cc b/gcc/tree.cc
> index 2f488e4467c..555e96c59d5 100644
> --- a/gcc/tree.cc
> +++ b/gcc/tree.cc
> @@ -13510,6 +13510,7 @@ gimple_canonical_types_compatible_p (const_tree t1,
> const_tree t2,
>        || TREE_CODE (t1) == VECTOR_TYPE
>        || TREE_CODE (t1) == COMPLEX_TYPE
>        || TREE_CODE (t1) == OFFSET_TYPE
> +      || TREE_CODE (t1) == OPAQUE_TYPE
>        || POINTER_TYPE_P (t1))
>      {
>        /* Can't be the same type if they have different recision.  */
> 
> Or adding one default hook which does the similar thing, and then if one
> target needs some target specific checks on its opaque type, one specific
> hook can be provided.

I'm quoting tree.def, emphasis mine:

/* This is for types that will use MODE_OPAQUE in the back end.  They are meant
   to be able to go in a register of some sort but are _EXPLICITLY NOT TO BE
   CONVERTED_ or operated on like INTEGER_TYPE.  They will have size and
   alignment information only.  */
DEFTREECODE (OPAQUE_TYPE, "opaque_type", tcc_type, 0)

so why should we care about special-casing them?  The target should have set
TYPE_CANONICAL appropriately if necessary, why didn't it?  Btw, 'const'
qualification should go into the type variant chain (well, for "normal"
types), where TYPE_MAIN_VARIANT is the unqualified type variant. 
TYPE_CANONICAL
shouldn't come into play here.

Btw, the whole idea of "opaque" is a hack and it seems to backfire now?

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

* [Bug middle-end/106833] Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
  2022-09-05  8:09 ` [Bug middle-end/106833] " linkw at gcc dot gnu.org
  2022-09-05  8:25 ` rguenth at gcc dot gnu.org
@ 2022-09-05  8:27 ` rguenth at gcc dot gnu.org
  2022-09-05  9:26 ` linkw at gcc dot gnu.org
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-09-05  8:27 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #2)
> (In reply to Kewen Lin from comment #1)
> > IMHO this is an omission when we were adding supports for opaque type, const
> > __vector_quad and __vector_quad should be taken as
> > canonical_types_compatible.
> > 
> > I wonder if we can simply take it just like what it handles for
> > "Non-aggregate types", for example:
> > 
> > diff --git a/gcc/tree.cc b/gcc/tree.cc
> > index 2f488e4467c..555e96c59d5 100644
> > --- a/gcc/tree.cc
> > +++ b/gcc/tree.cc
> > @@ -13510,6 +13510,7 @@ gimple_canonical_types_compatible_p (const_tree t1,
> > const_tree t2,
> >        || TREE_CODE (t1) == VECTOR_TYPE
> >        || TREE_CODE (t1) == COMPLEX_TYPE
> >        || TREE_CODE (t1) == OFFSET_TYPE
> > +      || TREE_CODE (t1) == OPAQUE_TYPE
> >        || POINTER_TYPE_P (t1))
> >      {
> >        /* Can't be the same type if they have different recision.  */
> > 
> > Or adding one default hook which does the similar thing, and then if one
> > target needs some target specific checks on its opaque type, one specific
> > hook can be provided.
> 
> I'm quoting tree.def, emphasis mine:
> 
> /* This is for types that will use MODE_OPAQUE in the back end.  They are
> meant
>    to be able to go in a register of some sort but are _EXPLICITLY NOT TO BE
>    CONVERTED_ or operated on like INTEGER_TYPE.  They will have size and
>    alignment information only.  */
> DEFTREECODE (OPAQUE_TYPE, "opaque_type", tcc_type, 0)

Also the above says "have size and alignment information only" but the
path you patch will look at TYPE_PRECISION and signedness.  Why, if that
was necessary, did the TYPE_MODE check not trigger?

> so why should we care about special-casing them?  The target should have set
> TYPE_CANONICAL appropriately if necessary, why didn't it?  Btw, 'const'
> qualification should go into the type variant chain (well, for "normal"
> types), where TYPE_MAIN_VARIANT is the unqualified type variant. 
> TYPE_CANONICAL
> shouldn't come into play here.
> 
> Btw, the whole idea of "opaque" is a hack and it seems to backfire now?

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

* [Bug middle-end/106833] Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2022-09-05  8:27 ` rguenth at gcc dot gnu.org
@ 2022-09-05  9:26 ` linkw at gcc dot gnu.org
  2022-09-05  9:29 ` linkw at gcc dot gnu.org
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: linkw at gcc dot gnu.org @ 2022-09-05  9:26 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #4 from Kewen Lin <linkw at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #2)
> (In reply to Kewen Lin from comment #1)
> > IMHO this is an omission when we were adding supports for opaque type, const
> > __vector_quad and __vector_quad should be taken as
> > canonical_types_compatible.
> > 
> > I wonder if we can simply take it just like what it handles for
> > "Non-aggregate types", for example:
> > 
> > diff --git a/gcc/tree.cc b/gcc/tree.cc
> > index 2f488e4467c..555e96c59d5 100644
> > --- a/gcc/tree.cc
> > +++ b/gcc/tree.cc
> > @@ -13510,6 +13510,7 @@ gimple_canonical_types_compatible_p (const_tree t1,
> > const_tree t2,
> >        || TREE_CODE (t1) == VECTOR_TYPE
> >        || TREE_CODE (t1) == COMPLEX_TYPE
> >        || TREE_CODE (t1) == OFFSET_TYPE
> > +      || TREE_CODE (t1) == OPAQUE_TYPE
> >        || POINTER_TYPE_P (t1))
> >      {
> >        /* Can't be the same type if they have different recision.  */
> > 
> > Or adding one default hook which does the similar thing, and then if one
> > target needs some target specific checks on its opaque type, one specific
> > hook can be provided.
> 
> I'm quoting tree.def, emphasis mine:
> 
> /* This is for types that will use MODE_OPAQUE in the back end.  They are
> meant
>    to be able to go in a register of some sort but are _EXPLICITLY NOT TO BE
>    CONVERTED_ or operated on like INTEGER_TYPE.  They will have size and
>    alignment information only.  */
> DEFTREECODE (OPAQUE_TYPE, "opaque_type", tcc_type, 0)
> 

Good point! My fault, I didn't read through this documentation. It seems to say
no conversions are allowed on it? (either explicit or implicit?)

> so why should we care about special-casing them?  The target should have set
> TYPE_CANONICAL appropriately if necessary, why didn't it?  Btw, 'const'
> qualification should go into the type variant chain (well, for "normal"
> types), where TYPE_MAIN_VARIANT is the unqualified type variant. 
> TYPE_CANONICAL
> shouldn't come into play here.
> 

With flag_checking on, while doing lto_fixup_state, verify_type will check
every tree type, it further checks with verify_type_variant, then fails with
gimple_canonical_types_compatible_p (t, tv, false).  (here trust_type_canonical
is false).

I think this is a common issue for any cv-qualified opaque type when lto
checking is on.

In this case, 
t1:
  const __vector_quad                                                           

t2:
  __vector_quad

Both TYPE_MAIN_VARIANT and both TYPE_CANONICAL is exactly the same here (all
equivalent to t2).

> Btw, the whole idea of "opaque" is a hack and it seems to backfire now?

Not sure, it had some adjustments with r11-5222 before, I thought we need some
similar for this issue.

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

* [Bug middle-end/106833] Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
                   ` (3 preceding siblings ...)
  2022-09-05  9:26 ` linkw at gcc dot gnu.org
@ 2022-09-05  9:29 ` linkw at gcc dot gnu.org
  2022-09-05  9:58 ` rguenther at suse dot de
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: linkw at gcc dot gnu.org @ 2022-09-05  9:29 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #5 from Kewen Lin <linkw at gcc dot gnu.org> ---
> > I'm quoting tree.def, emphasis mine:
> > 
> > /* This is for types that will use MODE_OPAQUE in the back end.  They are
> > meant
> >    to be able to go in a register of some sort but are _EXPLICITLY NOT TO BE
> >    CONVERTED_ or operated on like INTEGER_TYPE.  They will have size and
> >    alignment information only.  */
> > DEFTREECODE (OPAQUE_TYPE, "opaque_type", tcc_type, 0)
> 
> Also the above says "have size and alignment information only" but the
> path you patch will look at TYPE_PRECISION and signedness.  Why, if that
> was necessary, did the TYPE_MODE check not trigger?
> 

After reading the documentation you posted, I think the patching place is
wrong. The previous TYPE_MODE check passed, as both TYPE_MODEs are E_XOmode.

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

* [Bug middle-end/106833] Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
                   ` (4 preceding siblings ...)
  2022-09-05  9:29 ` linkw at gcc dot gnu.org
@ 2022-09-05  9:58 ` rguenther at suse dot de
  2022-09-05 16:25 ` segher at gcc dot gnu.org
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenther at suse dot de @ 2022-09-05  9:58 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #6 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 5 Sep 2022, linkw at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833
> 
> --- Comment #4 from Kewen Lin <linkw at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #2)
> > (In reply to Kewen Lin from comment #1)
> > > IMHO this is an omission when we were adding supports for opaque type, const
> > > __vector_quad and __vector_quad should be taken as
> > > canonical_types_compatible.
> > > 
> > > I wonder if we can simply take it just like what it handles for
> > > "Non-aggregate types", for example:
> > > 
> > > diff --git a/gcc/tree.cc b/gcc/tree.cc
> > > index 2f488e4467c..555e96c59d5 100644
> > > --- a/gcc/tree.cc
> > > +++ b/gcc/tree.cc
> > > @@ -13510,6 +13510,7 @@ gimple_canonical_types_compatible_p (const_tree t1,
> > > const_tree t2,
> > >        || TREE_CODE (t1) == VECTOR_TYPE
> > >        || TREE_CODE (t1) == COMPLEX_TYPE
> > >        || TREE_CODE (t1) == OFFSET_TYPE
> > > +      || TREE_CODE (t1) == OPAQUE_TYPE
> > >        || POINTER_TYPE_P (t1))
> > >      {
> > >        /* Can't be the same type if they have different recision.  */
> > > 
> > > Or adding one default hook which does the similar thing, and then if one
> > > target needs some target specific checks on its opaque type, one specific
> > > hook can be provided.
> > 
> > I'm quoting tree.def, emphasis mine:
> > 
> > /* This is for types that will use MODE_OPAQUE in the back end.  They are
> > meant
> >    to be able to go in a register of some sort but are _EXPLICITLY NOT TO BE
> >    CONVERTED_ or operated on like INTEGER_TYPE.  They will have size and
> >    alignment information only.  */
> > DEFTREECODE (OPAQUE_TYPE, "opaque_type", tcc_type, 0)
> > 
> 
> Good point! My fault, I didn't read through this documentation. It seems to say
> no conversions are allowed on it? (either explicit or implicit?)
> 
> > so why should we care about special-casing them?  The target should have set
> > TYPE_CANONICAL appropriately if necessary, why didn't it?  Btw, 'const'
> > qualification should go into the type variant chain (well, for "normal"
> > types), where TYPE_MAIN_VARIANT is the unqualified type variant. 
> > TYPE_CANONICAL
> > shouldn't come into play here.
> > 
> 
> With flag_checking on, while doing lto_fixup_state, verify_type will check
> every tree type, it further checks with verify_type_variant, then fails with
> gimple_canonical_types_compatible_p (t, tv, false).  (here trust_type_canonical
> is false).

Ah, that special "mode".  I think verify_types shouldn't do anything
for OPAQUE_TYPES or alternatively trust the targets setup of
TYPE_MAIN_VARIANT/TYPE_CANONICAL.  Maybe verify TYPE_CANONICAL
and TYPE_MAIN_VARIANT are also OPAQUE_TYPE.

So the solution should be fully inside verify_type.

> I think this is a common issue for any cv-qualified opaque type when lto
> checking is on.
> 
> In this case, 
> t1:
>   const __vector_quad                                                           
> 
> t2:
>   __vector_quad
> 
> Both TYPE_MAIN_VARIANT and both TYPE_CANONICAL is exactly the same here (all
> equivalent to t2).
> 
> > Btw, the whole idea of "opaque" is a hack and it seems to backfire now?
> 
> Not sure, it had some adjustments with r11-5222 before, I thought we need some
> similar for this issue.

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

* [Bug middle-end/106833] Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
                   ` (5 preceding siblings ...)
  2022-09-05  9:58 ` rguenther at suse dot de
@ 2022-09-05 16:25 ` segher at gcc dot gnu.org
  2022-09-06  3:40 ` linkw at gcc dot gnu.org
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: segher at gcc dot gnu.org @ 2022-09-05 16:25 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #7 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #6)
> Ah, that special "mode".  I think verify_types shouldn't do anything
> for OPAQUE_TYPES or alternatively trust the targets setup of
> TYPE_MAIN_VARIANT/TYPE_CANONICAL.  Maybe verify TYPE_CANONICAL
> and TYPE_MAIN_VARIANT are also OPAQUE_TYPE.

It's probably easiest to just test if the TYPE_MODEs match, for OPAQUE_TYPEs?

> So the solution should be fully inside verify_type.

Yeah.  OPAQUE might be a "hack" (or call it an "invention" :-) ), but the nice
thing about is it is very self-contained, no interactions anywhere.  That is
the
point even :-)

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

* [Bug middle-end/106833] Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
                   ` (6 preceding siblings ...)
  2022-09-05 16:25 ` segher at gcc dot gnu.org
@ 2022-09-06  3:40 ` linkw at gcc dot gnu.org
  2022-09-06 21:21 ` segher at gcc dot gnu.org
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: linkw at gcc dot gnu.org @ 2022-09-06  3:40 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #8 from Kewen Lin <linkw at gcc dot gnu.org> ---
Created attachment 53542
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53542&action=edit
Specially handle opaque type in verify_type

(In reply to Segher Boessenkool from comment #7)
> (In reply to rguenther@suse.de from comment #6)
> > Ah, that special "mode".  I think verify_types shouldn't do anything
> > for OPAQUE_TYPES or alternatively trust the targets setup of
> > TYPE_MAIN_VARIANT/TYPE_CANONICAL.  Maybe verify TYPE_CANONICAL
> > and TYPE_MAIN_VARIANT are also OPAQUE_TYPE.
> 
> It's probably easiest to just test if the TYPE_MODEs match, for OPAQUE_TYPEs?

Thanks for suggestions from both of you! Following your suggestions, I made a
patch which also considers size and alignment information which is mentioned in
the documentation, does it make sense to you?

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

* [Bug middle-end/106833] Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
                   ` (7 preceding siblings ...)
  2022-09-06  3:40 ` linkw at gcc dot gnu.org
@ 2022-09-06 21:21 ` segher at gcc dot gnu.org
  2022-09-07  4:28 ` linkw at gcc dot gnu.org
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: segher at gcc dot gnu.org @ 2022-09-06 21:21 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #9 from Segher Boessenkool <segher at gcc dot gnu.org> ---
Although, preferably we should not allow assigning one opaque type to another
opaque type just because they will eventually use the same mode, not without
warning anyway?  Or is that unavoidable?  Compare assigning a V4SI to a V4SF.

I don't know if your patch does this, btw, and it isn't so easy to test, we
currently have only one type for each of our opaque modes.  Maybe test by
adding an extra builtin type :-)

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

* [Bug middle-end/106833] Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
                   ` (8 preceding siblings ...)
  2022-09-06 21:21 ` segher at gcc dot gnu.org
@ 2022-09-07  4:28 ` linkw at gcc dot gnu.org
  2022-09-07  4:31 ` [Bug middle-end/106833] Miss to handle OPAQUE_TYPE specially in verify_type linkw at gcc dot gnu.org
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: linkw at gcc dot gnu.org @ 2022-09-07  4:28 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #10 from Kewen Lin <linkw at gcc dot gnu.org> ---
(In reply to Segher Boessenkool from comment #9)
> Although, preferably we should not allow assigning one opaque type to another
> opaque type just because they will eventually use the same mode, not without
> warning anyway?  Or is that unavoidable?  Compare assigning a V4SI to a V4SF.
> 

IIUC, you meant the assignment happening for two different opaque types, then
it's a conversion? If so, I think we can check it in rs6000_invalid_conversion,
currently it just simply checks the modes. If we have two different opaque
types mapping to one same mode, we can further check if the things like
TYPE_CANONICAL match.

> I don't know if your patch does this, btw, and it isn't so easy to test, we
> currently have only one type for each of our opaque modes.  Maybe test by
> adding an extra builtin type :-)

This patch doesn't handle that, the main issue here is that some cv-qualified
opaque type can cause ICE in type verification during LTO. IMHO, opaque types
conversion issue looks like a separated issue and it can be handled in target
hook invalid_conversion. But I guess you want a more generic check?  And as you
pointed out, there is no such scenario that two opaque types have the same
mode, not sure if we really want to handle it for now. :-)

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

* [Bug middle-end/106833] Miss to handle OPAQUE_TYPE specially in verify_type
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
                   ` (9 preceding siblings ...)
  2022-09-07  4:28 ` linkw at gcc dot gnu.org
@ 2022-09-07  4:31 ` linkw at gcc dot gnu.org
  2022-09-07 13:08 ` segher at gcc dot gnu.org
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: linkw at gcc dot gnu.org @ 2022-09-07  4:31 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #11 from Kewen Lin <linkw at gcc dot gnu.org> ---
One modified case from pr102347.c (same option set is used), which can
reproduce the ICE without any gcc source adjustment. 

#pragma GCC target "cpu=power10"
int main ()
{
  float *b;
  const __vector_quad c;
  __builtin_mma_disassemble_acc (b, &c);
  return 0;
}

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

* [Bug middle-end/106833] Miss to handle OPAQUE_TYPE specially in verify_type
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
                   ` (10 preceding siblings ...)
  2022-09-07  4:31 ` [Bug middle-end/106833] Miss to handle OPAQUE_TYPE specially in verify_type linkw at gcc dot gnu.org
@ 2022-09-07 13:08 ` segher at gcc dot gnu.org
  2022-09-09 13:18 ` cvs-commit at gcc dot gnu.org
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: segher at gcc dot gnu.org @ 2022-09-07 13:08 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #12 from Segher Boessenkool <segher at gcc dot gnu.org> ---
(In reply to Kewen Lin from comment #10)
> (In reply to Segher Boessenkool from comment #9)
> > Although, preferably we should not allow assigning one opaque type to another
> > opaque type just because they will eventually use the same mode, not without
> > warning anyway?  Or is that unavoidable?  Compare assigning a V4SI to a V4SF.
> 
> IIUC, you meant the assignment happening for two different opaque types,
> then it's a conversion?

Yes exactly.

> If so, I think we can check it in
> rs6000_invalid_conversion, currently it just simply checks the modes.

Yup.

> If we
> have two different opaque types mapping to one same mode, we can further
> check if the things like TYPE_CANONICAL match.

Like that.  It isn't urgent -- we currently have only one type for each of
our two opaque modes -- but if we allow too much here, we would need a separate
mode for each opaque thing we want to distinguish, which is contrary to the
point of having it :-)

> > I don't know if your patch does this, btw, and it isn't so easy to test, we
> > currently have only one type for each of our opaque modes.  Maybe test by
> > adding an extra builtin type :-)
> 
> This patch doesn't handle that, the main issue here is that some
> cv-qualified opaque type can cause ICE in type verification during LTO.
> IMHO, opaque types conversion issue looks like a separated issue and it can
> be handled in target hook invalid_conversion. But I guess you want a more
> generic check?  And as you pointed out, there is no such scenario that two
> opaque types have the same mode, not sure if we really want to handle it for
> now. :-)

I think it can be handled generically, no target code is needed for it.

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

* [Bug middle-end/106833] Miss to handle OPAQUE_TYPE specially in verify_type
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
                   ` (11 preceding siblings ...)
  2022-09-07 13:08 ` segher at gcc dot gnu.org
@ 2022-09-09 13:18 ` cvs-commit at gcc dot gnu.org
  2022-09-14 12:43 ` linkw at gcc dot gnu.org
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-09-09 13:18 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Kewen Lin <linkw@gcc.gnu.org>:

https://gcc.gnu.org/g:e230f11e9784eefed316df7dbc5df6ac999841b2

commit r13-2562-ge230f11e9784eefed316df7dbc5df6ac999841b2
Author: Kewen Lin <linkw@linux.ibm.com>
Date:   Fri Sep 9 08:17:21 2022 -0500

    Handle OPAQUE_TYPE specially in verify_type [PR106833]

    As PR106833 shows, cv-qualified opaque type can cause ICE
    during LTO.  It exposes that we missd to handle OPAQUE_TYPE
    well in type verification.  As Richi pointed out, also
    assuming that target will always define TYPE_MAIN_VARIANT
    TYPE_CANONICAL for opaque type, this patch is to check
    both are OPAQUE_TYPE_P and their modes are of MODE_OPAQUE
    class.  Besides, it also checks the only available size
    and alignment information.

            PR middle-end/106833

    gcc/ChangeLog:

            * tree.cc (verify_opaque_type): New function.
            (verify_type): Call verify_opaque_type for OPAQUE_TYPE.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/pr106833.c: New test.

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

* [Bug middle-end/106833] Miss to handle OPAQUE_TYPE specially in verify_type
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
                   ` (12 preceding siblings ...)
  2022-09-09 13:18 ` cvs-commit at gcc dot gnu.org
@ 2022-09-14 12:43 ` linkw at gcc dot gnu.org
  2022-09-14 12:53 ` bergner at gcc dot gnu.org
  2022-09-14 13:50 ` linkw at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: linkw at gcc dot gnu.org @ 2022-09-14 12:43 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

Kewen Lin <linkw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
           Assignee|unassigned at gcc dot gnu.org      |linkw at gcc dot gnu.org
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #14 from Kewen Lin <linkw at gcc dot gnu.org> ---
Should be fixed on trunk, opened PR106941 for conversion as Segher's comments.

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

* [Bug middle-end/106833] Miss to handle OPAQUE_TYPE specially in verify_type
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
                   ` (13 preceding siblings ...)
  2022-09-14 12:43 ` linkw at gcc dot gnu.org
@ 2022-09-14 12:53 ` bergner at gcc dot gnu.org
  2022-09-14 13:50 ` linkw at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: bergner at gcc dot gnu.org @ 2022-09-14 12:53 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #15 from Peter Bergner <bergner at gcc dot gnu.org> ---
(In reply to Kewen Lin from comment #14)
> Should be fixed on trunk

I assume this is broken on the release branches too and we'll need backports?

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

* [Bug middle-end/106833] Miss to handle OPAQUE_TYPE specially in verify_type
  2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
                   ` (14 preceding siblings ...)
  2022-09-14 12:53 ` bergner at gcc dot gnu.org
@ 2022-09-14 13:50 ` linkw at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: linkw at gcc dot gnu.org @ 2022-09-14 13:50 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106833

--- Comment #16 from Kewen Lin <linkw at gcc dot gnu.org> ---
(In reply to Peter Bergner from comment #15)
> (In reply to Kewen Lin from comment #14)
> > Should be fixed on trunk
> 
> I assume this is broken on the release branches too and we'll need backports?

Good question, currently function verify_type uses are all guarded by
flag_checking, I just had a testing, the ICE didn't get exposed with release
branches, I think it is because that they are configured with
--enable-checking=release by default, but it's still reproducible with one
extra explicit option -fchecking=1 for release branches.

Hi Richi, what do you think of the backporting for this case?

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

end of thread, other threads:[~2022-09-14 13:50 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-05  8:01 [Bug middle-end/106833] New: Handle OPAQUE_TYPE in gimple_canonical_types_compatible_p linkw at gcc dot gnu.org
2022-09-05  8:09 ` [Bug middle-end/106833] " linkw at gcc dot gnu.org
2022-09-05  8:25 ` rguenth at gcc dot gnu.org
2022-09-05  8:27 ` rguenth at gcc dot gnu.org
2022-09-05  9:26 ` linkw at gcc dot gnu.org
2022-09-05  9:29 ` linkw at gcc dot gnu.org
2022-09-05  9:58 ` rguenther at suse dot de
2022-09-05 16:25 ` segher at gcc dot gnu.org
2022-09-06  3:40 ` linkw at gcc dot gnu.org
2022-09-06 21:21 ` segher at gcc dot gnu.org
2022-09-07  4:28 ` linkw at gcc dot gnu.org
2022-09-07  4:31 ` [Bug middle-end/106833] Miss to handle OPAQUE_TYPE specially in verify_type linkw at gcc dot gnu.org
2022-09-07 13:08 ` segher at gcc dot gnu.org
2022-09-09 13:18 ` cvs-commit at gcc dot gnu.org
2022-09-14 12:43 ` linkw at gcc dot gnu.org
2022-09-14 12:53 ` bergner at gcc dot gnu.org
2022-09-14 13:50 ` linkw at gcc dot gnu.org

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