From: Richard Biener <rguenther@suse.de>
To: Martin Uecker <uecker@tugraz.at>
Cc: Jakub Jelinek <jakub@redhat.com>,
gcc-patches@gcc.gnu.org, Jan Hubicka <hubicka@ucw.cz>
Subject: Re: [PATCH] middle-end/114931 - type_hash_canon and structual equality types
Date: Tue, 7 May 2024 13:05:52 +0200 (CEST) [thread overview]
Message-ID: <ns67974p-3s35-69o7-nnsr-p0o1815q44r7@fhfr.qr> (raw)
In-Reply-To: <3ea18459a1a1cb968919cf270d71dcd441b08311.camel@tugraz.at>
[-- Attachment #1: Type: text/plain, Size: 5546 bytes --]
On Mon, 6 May 2024, Martin Uecker wrote:
> Am Montag, dem 06.05.2024 um 11:07 +0200 schrieb Richard Biener:
> > On Mon, 6 May 2024, Martin Uecker wrote:
> >
> > > Am Montag, dem 06.05.2024 um 09:00 +0200 schrieb Richard Biener:
> > > > On Sat, 4 May 2024, Martin Uecker wrote:
> > > >
> > > > > Am Freitag, dem 03.05.2024 um 21:16 +0200 schrieb Jakub Jelinek:
> > > > > > > On Fri, May 03, 2024 at 09:11:20PM +0200, Martin Uecker wrote:
> > > > > > > > > > > TYPE_CANONICAL as used by the middle-end cannot express this but
> > > > > > > > >
> > > > > > > > > Hm. so how does it work now for arrays?
> > > > > > >
> > > > > > > Do you have a testcase which doesn't work correctly with the arrays?
> > > > >
> > > > > I am mostly trying to understand better how this works. But
> > > > > if I am not mistaken, the following example would indeed
> > > > > indicate that we do incorrect aliasing decisions for types
> > > > > derived from arrays:
> > > > >
> > > > > https://godbolt.org/z/rTsE3PhKc
> > > >
> > > > This example is about pointer-to-array types, int (*)[2] and
> > > > int (*)[1] are supposed to be compatible as in receive the same alias
> > > > set.
> > >
> > > In C, char (*)[2] and char (*)[1] are not compatible. But with
> > > COMPAT set, the example operates^1 with char (*)[] and char (*)[1]
> > > which are compatible. If we form equivalence classes, then
> > > all three types would need to be treated as equivalent.
> > >
> > > ^1 Actually, pointer to functions returning pointers
> > > to arrays. Probably this example can still be simplified...
> > >
> > > > This is ensured by get_alias_set POINTER_TYPE_P handling,
> > > > the alias set is supposed to be the same as that of int *. It seems
> > > > we do restrict the handling a bit, the code does
> > > >
> > > > /* Unnest all pointers and references.
> > > > We also want to make pointer to array/vector equivalent to
> > > > pointer to
> > > > its element (see the reasoning above). Skip all those types, too.
> > > > */
> > > > for (p = t; POINTER_TYPE_P (p)
> > > > || (TREE_CODE (p) == ARRAY_TYPE
> > > > && (!TYPE_NONALIASED_COMPONENT (p)
> > > > || !COMPLETE_TYPE_P (p)
> > > > || TYPE_STRUCTURAL_EQUALITY_P (p)))
> > > > || TREE_CODE (p) == VECTOR_TYPE;
> > > > p = TREE_TYPE (p))
> > > >
> > > > where the comment doesn't exactly match the code - but C should
> > > > never have TYPE_NONALIASED_COMPONENT (p).
> > > >
> > > > But maybe I misread the example or it goes wrong elsewhere.
> > >
> > > If I am not confusing myself too much, the example shows that
> > > aliasing analysis treats the the types as incompatible in
> > > both cases, because it does not reload *a with -O2.
> > >
> > > For char (*)[1] and char (*)[2] this would be correct (but an
> > > implementation exploiting this would need to do structural
> > > comparisons and not equivalence classes) but for
> > > char (*)[2] and char (*)[] it is not.
> >
> > Oh, these are function pointers, so it's about the alias set of
> > a pointer to FUNCTION_TYPE. I don't see any particular code
> > trying to make char[] * (*)() and char[1] *(*)() inter-operate
> > for TBAA iff the FUNCTION_TYPEs themselves are not having the
> > same TYPE_CANONICAL.
> >
> > Can you open a bugreport and please point to the relevant parts
> > of the C standard that tells how pointer-to FUNCTION_TYPE TBAA
> > is supposed to work?
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114959
I have now pushed the patch after consulting with Jakub.
Richard.
> Martin
> >
>
> > Thanks,
> > Richard.
> >
> > > Martin
> > >
> > >
> > > >
> > > > Richard.
> > > >
> > > > > Martin
> > > > >
> > > > > > >
> > > > > > > E.g. same_type_for_tbaa has
> > > > > > > type1 = TYPE_MAIN_VARIANT (type1);
> > > > > > > type2 = TYPE_MAIN_VARIANT (type2);
> > > > > > >
> > > > > > > /* Handle the most common case first. */
> > > > > > > if (type1 == type2)
> > > > > > > return 1;
> > > > > > >
> > > > > > > /* If we would have to do structural comparison bail out. */
> > > > > > > if (TYPE_STRUCTURAL_EQUALITY_P (type1)
> > > > > > > || TYPE_STRUCTURAL_EQUALITY_P (type2))
> > > > > > > return -1;
> > > > > > >
> > > > > > > /* Compare the canonical types. */
> > > > > > > if (TYPE_CANONICAL (type1) == TYPE_CANONICAL (type2))
> > > > > > > return 1;
> > > > > > >
> > > > > > > /* ??? Array types are not properly unified in all cases as we have
> > > > > > > spurious changes in the index types for example. Removing this
> > > > > > > causes all sorts of problems with the Fortran frontend. */
> > > > > > > if (TREE_CODE (type1) == ARRAY_TYPE
> > > > > > > && TREE_CODE (type2) == ARRAY_TYPE)
> > > > > > > return -1;
> > > > > > > ...
> > > > > > > and later compares alias sets and the like.
> > > > > > > So, even if int[] and int[0] have different TYPE_CANONICAL, they
> > > > > > > will be considered maybe the same. Also, guess get_alias_set
> > > > > > > has some ARRAY_TYPE handling...
> > > > > > >
> > > > > > > Anyway, I think we should just go with Richi's patch.
> > > > > > >
> > > > > > > Jakub
> > > > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > >
> > >
> >
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
prev parent reply other threads:[~2024-05-07 11:05 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-03 12:13 Richard Biener
2024-05-03 15:32 ` Martin Uecker
2024-05-03 16:23 ` Richard Biener
2024-05-03 17:44 ` Martin Uecker
2024-05-03 17:30 ` Jakub Jelinek
2024-05-03 18:04 ` Martin Uecker
2024-05-03 18:18 ` Jakub Jelinek
2024-05-03 18:37 ` Martin Uecker
2024-05-03 18:48 ` Richard Biener
2024-05-03 19:11 ` Martin Uecker
2024-05-03 19:16 ` Jakub Jelinek
2024-05-04 6:38 ` Martin Uecker
2024-05-06 7:00 ` Richard Biener
2024-05-06 7:27 ` Martin Uecker
2024-05-06 9:07 ` Richard Biener
2024-05-06 11:20 ` Martin Uecker
2024-05-07 11:05 ` Richard Biener [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ns67974p-3s35-69o7-nnsr-p0o1815q44r7@fhfr.qr \
--to=rguenther@suse.de \
--cc=gcc-patches@gcc.gnu.org \
--cc=hubicka@ucw.cz \
--cc=jakub@redhat.com \
--cc=uecker@tugraz.at \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).