Am Mittwoch, dem 19.06.2024 um 08:57 +0200 schrieb Richard Biener:
> 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).
Ok. Then should it, instead of
TYPE_CANONICAL (x)
= build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
be
tree c = build_qualified_type (TYPE_CANONICAL (t), TYPE_QUALS (x));
TYPE_CANONICAL (x) = TREE_CANONICAL (c);
in the patch below?
Martin
>
> > 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 *);
> > > > > > +
> > > > > >
> > > >
> >
> >
>
--
Univ.-Prof. Dr. rer. nat. Martin Uecker
Graz University of Technology
Institute of Biomedical Imaging
