On Wed, 19 Jun 2024, Martin Uecker wrote:
> Am Mittwoch, dem 19.06.2024 um 08:04 +0200 schrieb Richard Biener:
> >
> > > Am 18.06.2024 um 20:18 schrieb Martin Uecker <uec...@tugraz.at>:
> > >
> > > Am Dienstag, dem 18.06.2024 um 17:27 +0200 schrieb Richard Biener:
> > > >
> > > > > > Am 18.06.2024 um 17:20 schrieb Martin Uecker <uec...@tugraz.at>:
> > > > >
> > > > >
> > > > > As discussed this replaces the use of check_qualified_type with
> > > > > a simple check for qualifiers as suggested by Jakub in
> > > > > c_update_type_canonical.
> > > >
> > > > Note a canonical type should always be unqualified (for
> > > > classical qualifiers, not address space or atomic qualification)
> > >
> > > The logic in build_qualified_type is the same as in this patch,
> > > it constructs TYPE_CANONICAL with qualifiers. Or what am I
> > > missing?
> >
> > Nothing if you chose to do TYPE_QUALS (canonical) by copy-pasting.
>
> I would rather like to undertand how this work. Is the following
> correct?
>
> For a type T with TYPE_CANONICAL (T) == S, if we construct a qualified
> type "const" T it would get a const qualified version Q of S as
> canonical type. S has TYPE_CANONICAL (S) == S and Q has
> TYPE_CANONICAL (Q) == Q.
I think this is a misunderstanding - TYPE_CANONICAL of both T and Q
should be the same (S).
> But the middle end would then ignore this for regular qualifiers
> and use the TYPE_CANONICAL of the main variant for computing
> the aliasing set?
The middle end uses the canonical type for alias set purposes.
The indirection via TYPE_MAIN_VARIANT shouldn't be necessary.
> Setting it differently for qualified types would still be important
> so that derived types get their own different TYPE_CANONICAL
> during construction.
>
>
> The other thing I would like to confirm? The alias set is
> initialized to -1 so set later in the middle end based on
> TYPE_CANONICAL (if not set to zero before). This is never
> done before the FE is finished or do we need to worry that
> this may become inconsistent?
It used to be that -Wstrict-aliasing calls get_alias_set so
TYPE_ALIAS_SET can become set, I don't remember this being
avoided so yes, you'd have to worry about this. Note
TYPE_ALIAS_SET is only ever considered on the canonical type
(but IIRC we have checking code looking at other type alias set).
Richard.
> Martin
>
> > Richard
> >
> > > Martin
> > >
> > > >
> > > > Richard
> > > >
> > > > > Martin
> > > > >
> > > > >
> > > > > Bootstrapped and regression tested on x86_64.
> > > > >
> > > > >
> > > > > C23: Fix ICE related to incomplete structures [PR114930,PR115502].
> > > > >
> > > > > The fix for PR114574 needs to be further revised because
> > > > > check_qualified_type
> > > > > makes decision based on TYPE_NAME which can be incorrect for C when
> > > > > there
> > > > > are TYPE_DECLS involved.
> > > > >
> > > > > gcc/c/:
> > > > > * c-decl.c (c_update_type_canonical): Do not use
> > > > > check_qualified_type.
> > > > >
> > > > > gcc/testsuite/:
> > > > > * gcc.dg/pr114930.c: New test.
> > > > > * gcc.dg/pr115502.c: New test.
> > > > >
> > > > > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> > > > > index 01326570e2b..610061a07f8 100644
> > > > > --- a/gcc/c/c-decl.cc
> > > > > +++ b/gcc/c/c-decl.cc
> > > > > @@ -9374,7 +9374,7 @@ c_update_type_canonical (tree t)
> > > > > if (TYPE_QUALS (x) == TYPE_QUALS (t))
> > > > > TYPE_CANONICAL (x) = TYPE_CANONICAL (t);
> > > > > else if (TYPE_CANONICAL (t) != t
> > > > > - || check_qualified_type (x, t, TYPE_QUALS (x)))
> > > > > + || TYPE_QUALS (x) != TYPE_QUALS (TYPE_CANONICAL (t)))
> > > > > TYPE_CANONICAL (x)
> > > > > = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
> > > > > else
> > > > > diff --git a/gcc/testsuite/gcc.dg/pr114930.c
> > > > > b/gcc/testsuite/gcc.dg/pr114930.c
> > > > > new file mode 100644
> > > > > index 00000000000..5e982fb8929
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/gcc.dg/pr114930.c
> > > > > @@ -0,0 +1,9 @@
> > > > > +/* { dg-do compile }
> > > > > + * { dg-options "-std=c23 -flto" } */
> > > > > +
> > > > > +typedef struct WebPPicture WebPPicture;
> > > > > +typedef int (*WebPProgressHook)(const WebPPicture *);
> > > > > +WebPProgressHook progress_hook;
> > > > > +struct WebPPicture {
> > > > > +} WebPGetColorPalette(const struct WebPPicture *);
> > > > > +
> > > > > diff --git a/gcc/testsuite/gcc.dg/pr115502.c
> > > > > b/gcc/testsuite/gcc.dg/pr115502.c
> > > > > new file mode 100644
> > > > > index 00000000000..02b52622c5a
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/gcc.dg/pr115502.c
> > > > > @@ -0,0 +1,9 @@
> > > > > +/* { dg-do compile }
> > > > > + * { dg-options "-std=c23 -flto" } */
> > > > > +
> > > > > +typedef struct _OSet OSet;
> > > > > +typedef OSet AvlTree;
> > > > > +void vgPlain_OSetGen_Lookup(const OSet *);
> > > > > +struct _OSet {};
> > > > > +void vgPlain_OSetGen_Lookup(const AvlTree *);
> > > > > +
> > > > >
> > >
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
