From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) by sourceware.org (Postfix) with ESMTPS id A8A6738582B7 for ; Mon, 27 Nov 2023 07:46:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A8A6738582B7 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A8A6738582B7 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=195.135.223.131 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701071212; cv=none; b=tEASrOBAvt3WJAnfz08QoXDQRz/o51AnbCn99pjGyzajP5Tn4f5h7o7sZuxem69iHeuzWbDrqJa+uHUTooLul/h6XdMVIypitiKrngwMAGkjjxEH5wYpaIZA2EC3ohlL3V94i0A2y+wRhTPVQQai8QmbS6TOPLT68Kstx9FDsiU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1701071212; c=relaxed/simple; bh=cFVqX1VESZZMV/gECw8tJlq8l+bfVZhzt1Vy7jdWHPA=; h=Date:From:To:Subject:Message-ID:MIME-Version; b=hMAG1uqJlOMmmNOJWpoaMamygqH9A9i0pRH27oir3rOzkiGeUtubYUK+2NOmFMIkBywl3cQt254r6gPrmbLkvDQvYNDpuVVma4WdY7i71ozgdl1Q/COzHJKE37To762WDTUlgbOxTzyFJvY7XCm5rJvb0cO9BkJsF/alon1fr8A= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from relay2.suse.de (unknown [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id B62461FB7D; Mon, 27 Nov 2023 07:46:49 +0000 (UTC) Received: from wotan.suse.de (wotan.suse.de [10.160.0.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id F083C2C152; Mon, 27 Nov 2023 07:46:47 +0000 (UTC) Date: Mon, 27 Nov 2023 07:46:47 +0000 (UTC) From: Richard Biener To: Martin Uecker cc: Joseph Myers , gcc-patches@gcc.gnu.org Subject: Re: [PATCH 3/4] c23: aliasing of compatible tagged types In-Reply-To: <4337947e3d85a6796567eceb7e3470db6327f7a8.camel@tugraz.at> Message-ID: References: <02a9b94e4d653b6f1b9f89a1b62187f46e871738.camel@tugraz.at> <223aa096afbdbb177d4ad5245696d439ad4cf87f.camel@tugraz.at> <4337947e3d85a6796567eceb7e3470db6327f7a8.camel@tugraz.at> User-Agent: Alpine 2.22 (LSU 394 2020-01-19) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="-1609957120-691440405-1701071208=:21409" X-Spam-Level: Authentication-Results: smtp-out2.suse.de; none X-Rspamd-Server: rspamd2 X-Spamd-Result: default: False [-4.00 / 50.00]; REPLY(-4.00)[] X-Spam-Score: -4.00 X-Rspamd-Queue-Id: B62461FB7D X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. ---1609957120-691440405-1701071208=:21409 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT On Sun, 26 Nov 2023, Martin Uecker wrote: > > Thanks Joseph, I will sent an updated series tomorrow. > > Richard, maybe you could look at what I wrote below > about my use of TYPE_CANONICAL ? Does this make sense? > > > Am Donnerstag, dem 23.11.2023 um 23:47 +0000 schrieb Joseph Myers: > > On Thu, 16 Nov 2023, Martin Uecker wrote: > > > > > Tell the backend which types are equivalent by setting > > > TYPE_CANONICAL to one struct in the set of equivalent > > > structs. Structs are considered equivalent by ignoring > > > all sizes of arrays nested in types below field level. > > > > Is TYPE_CANONICAL *only* used for alias analysis? It's not obvious to me > > that setting TYPE_CANONICAL to a type that's definitely not equivalent for > > other purposes is necessarily safe. > > My understand is that it is used for aliasing analysis and also > checking of conversions. TYPE_CANONICAL must be consistent with > the idea the middle-end has about type conversions. But as long > as we do not give the same TYPE_CANONICAL to types the middle-end > thinks must be incompatible using its own type checking machinery, > it should be safe even for types the C standard thinks must be > incompatible for some reason. TYPE_CANONICAL is used to record structurally equivalent types that should be in a common class for TBAA. GIMPLE allows direct assignment of structurally equivalent types in aggregate copies and with LTO we are forming conservatively larger such groups to make cross-language TBAA possible. That means when the language standards says two types are compatible (and for example valid to assign to each other) then they have to have the same canonical type (or the frontend has to insert conversions). It's also the only means besides the type variant chain to assign the same alias set to a class of types when LTO is in effect (the exception is when the langhook assigns alias-set zero which is honored). > > I also think more rationale is needed for ignoring sizes like this. Is it > > intended for e.g. making structs with flexible array members > > alias-compatible with similar structs with a fixed-size array? > > The main reason are pointers to arrays: > > struct foo { int (*x)[]; } > struct foo { int (*x)[2]; }; > struct foo { int (*x)[1]; }; > > So at least when putting it in terms of equivalence classes, > one has no choice than making those types equivalent. So > all those would get the same TYPE_CANONICAL. The middle-end  > does not care about the different pointer types (in > useless_type_conversion_p or > gimple_canonical_types_compatible_p). Indeed LTO would put those into the same equivalence class. Richard. > > Martin > > > > > > > > > @@ -1250,6 +1266,9 @@ comptypes_internal (const_tree type1, const_tree type2, > > > > > > if ((d1 == NULL_TREE) != (d2 == NULL_TREE)) > > > data->different_types_p = true; > > > + /* ignore size mismatches */ > > > + if (data->equiv) > > > + return 1; > > > > Should start comment with capital letter, end with '.'. > > > > > diff --git a/gcc/testsuite/gcc.dg/c23-tag-2.c b/gcc/testsuite/gcc.dg/c23-tag-2.c > > > index 5dd4a21e9df..e28c2b5eea2 100644 > > > --- a/gcc/testsuite/gcc.dg/c23-tag-2.c > > > +++ b/gcc/testsuite/gcc.dg/c23-tag-2.c > > > @@ -1,5 +1,5 @@ > > > -/* { dg-do compile { target { ! "*-*-*" } } } > > > - * { dg-options "-std=c23" } > > > +/* { dg-do compile } > > > + * { dg-options "-std=c2x" } > > > */ > > > > > > // compatibility of structs in assignment > > > diff --git a/gcc/testsuite/gcc.dg/c23-tag-5.c b/gcc/testsuite/gcc.dg/c23-tag-5.c > > > index ff40d07aef1..95a04bf9b0e 100644 > > > --- a/gcc/testsuite/gcc.dg/c23-tag-5.c > > > +++ b/gcc/testsuite/gcc.dg/c23-tag-5.c > > > @@ -1,5 +1,6 @@ > > > -/* { dg-do run { target { ! "*-*-*" } } } > > > - * { dg-options "-std=c23" } > > > +/* > > > + * { dg-do run } > > > + * { dg-options "-std=c2x" } > > > > These tests should not be changed to use -std=c2x. > > > > > diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-2.c b/gcc/testsuite/gcc.dg/c23-tag-alias-2.c > > > new file mode 100644 > > > index 00000000000..555c30a8501 > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-2.c > > > @@ -0,0 +1,73 @@ > > > +/* > > > + * { dg-do run } > > > + * { dg-options "-std=c23 -O2" } > > > + */ > > > + > > > + > > > +struct foo { int x; }; > > > + > > > +int test_foo1(struct foo* a, void* b) > > > +{ > > > + a->x = 1; > > > + > > > + struct foo { int x; int y; }* p = b; > > > + p->x = 2; > > > + > > > + return a->x; > > > +} > > > > > +int main() > > > +{ > > > + struct foo y; > > > + > > > + if (1 != test_foo1(&y, &y)) > > > + __builtin_abort(); > > > > This test appears to be testing various invalid cases - testing that the > > compiler does not consider aliasing to occur in those cases (even though > > in fact there is aliasing). > > > > If that's the intent of this test, it definitely needs commenting. The > > test would also need to (be a gnu23-* test and) use appropriate attributes > > to disable interprocedural analysis, since it would be entirely valid for > > the compiler in this test to inline test_foo1, see that p->x in fact > > points to the same location as a->x despite the incompatible types, and > > have the function return 2. > > > > The same applies to c23-tag-alias-4.c and c23-tag-alias-5.c. > > > > > diff --git a/gcc/testsuite/gcc.dg/c23-tag-alias-5.c b/gcc/testsuite/gcc.dg/c23-tag-alias-5.c > > > new file mode 100644 > > > index 00000000000..4e956720143 > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.dg/c23-tag-alias-5.c > > > @@ -0,0 +1,30 @@ > > > +/* { dg-do run } > > > + * { dg-options "-std=c23 -O2" } > > > + */ > > > + > > > +// not sure this is wise, but this was already like thi sbefore > > > > "this before" > > > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg) ---1609957120-691440405-1701071208=:21409--