Re: Do less generous pointer globbing in alias.c
On 05/27/2015 07:28 AM, Jan Hubicka wrote:
> Hi,
> this patch makes it possible for non-LTO alias oracle to TBAA disambiguate
> pointer types. It makes void * conflicting with all of them and does not put
> it
> to alias set 0. It also preserves the property that qualifiers of pointer-to
> type should not matter to determine the alias set and that pointer to array is
> same as pointer to array element. Finally it makes pointer void * to be
> equivalent to void ** (and more *) and to types with structural equality only.
>
> I think those are all globbing rules we discussed for the non-LTO patch.
>
> It does two things. First is kind of "canonicalization" where for a given
> pointer
> it looks for non-pointer pointed-to type and then rebuilds is without
> qualifiers.
> This is fast, because build_pointer_type will reuse existing types.
>
> It makes void * to conflict with everyting by making its alias set to be
> subset
> of alias set of any other pointer. This means that writes to void * conflict
> with writes to any other pointer without really need to glob all the pointers
> to one equivalence class.
>
> This patch makes quite some difference on C++. For example on deal II the
> TBAA
> stats reports 4344358 disambiguations and 7008576 queries, while with the
> patch
> we get 5368737 and 5687399 queries (I did not chose deal II for reason, it is
> just random C++ file)
>
> The patch bootstrap and regtests ppc64le-linux with the following testsuite
> differences:
> @@ -30,7 +30,9 @@
> FAIL: c-c++-common/asan/null-deref-1.c -O3 -g output pattern test, is
> ASAN:SIGSEGV
> FAIL: c-c++-common/asan/null-deref-1.c -Os output pattern test, is
> ASAN:SIGSEGV
> FAIL: gcc.dg/cpp/_Pragma3.c (test for excess errors)
> +XPASS: gcc.dg/alias-8.c (test for warnings, line 11)
> FAIL: gcc.dg/loop-8.c scan-rtl-dump-times loop2_invariant "Decided" 1
> +FAIL: gcc.dg/pr62167.c scan-tree-dump-not pre "Removing basic block"
> FAIL: gcc.dg/sms-4.c scan-rtl-dump-times sms "SMS succeeded" 1
> XPASS: gcc.dg/guality/example.c -O0 execution test
> XPASS: gcc.dg/guality/example.c -O1 execution test
> @@ -304,6 +306,9 @@
> FAIL: c-c++-common/asan/null-deref-1.c -O3 -g output pattern test, is
> ASAN:SIGSEGV
> FAIL: g++.dg/cpp1y/vla-initlist1.C -std=gnu++11 execution test
> FAIL: g++.dg/cpp1y/vla-initlist1.C -std=gnu++14 execution test
> +FAIL: g++.dg/ipa/ipa-icf-4.C -std=gnu++11 scan-ipa-dump icf "Equal
> symbols: [67]"
> +FAIL: g++.dg/ipa/ipa-icf-4.C -std=gnu++14 scan-ipa-dump icf "Equal
> symbols: [67]"
> +FAIL: g++.dg/ipa/ipa-icf-4.C -std=gnu++98 scan-ipa-dump icf "Equal
> symbols: [67]"
>
> ipa-icf-4 is about alias info now being more perceptive to block the merging.
> pr62167 seems just confused. The template checks that memory stores are not
> unified. It looks for BB removal message, but with the patch we get:
> :
> node.next = 0B;
> head.0_4 = head;
> node.prev = head.0_4;
> head.0_4->first = &node;
> k.1_7 = k;
> h_8 = &heads[k.1_7];
> heads[2].first = 0B;
> if (head.0_4 == h_8)
> goto ;
> else
> goto ;
>
> :
> goto ;
>
> :
> p_10 = MEM[(struct head *)&heads][k.1_7].first;
>
> :
> # p_1 = PHI
> _11 = p_1 != 0B;
> _12 = (int) _11;
> return _12;
>
> before PR, the message is about the bb 5 sitting at critical edge removed.
> The TBAA incompatible load it looks for is optimized away by FRE:
> head->first = &node;
>
> struct node *n = head->first;
>
> struct head *h = &heads[k];
>
> heads[2].first = n->next;
>
> if ((void*)n->prev == (void *)h)
> p = h->first;
> else
> /* Dead tbaa-unsafe load from ((struct node *)&heads[2])->next. */
> p = n->prev->next;
>
> here n is known to be head->first that is known to be &node.
> The testcase runtime checks that result is Ok and passes.
>
> Bootstrapped/regtested ppc64le-linux.
>
> * alias.c (get_alias_set): Do not glob all pointer types into one;
> just produce euqivalence classes based on canonical type of pointed
> type type; make void * equivalent to void **.
> (record_component_aliases): Make void * to conflict with all other
> pointer types.
> Index: alias.c
> ===
> --- alias.c (revision 223633)
> +++ alias.c (working copy)
> @@ -903,35 +906,79 @@ get_alias_set (tree t)
> the pointed-to types. This issue has been reported to the
> C++ committee.
>
> - In addition to the above canonicalization issue, with LTO
> - we should also canonicalize `T (*)[]' to `T *' avoiding
> - alias issues with pointer-to element types and pointer-to
> - array types.
> -
> - Likewise we need to deal with the situation of incomplete
> - pointed-to types and make `*(struct X **)&a' and
> - `*(struct X {} **)&a' alias. Otherwise we will have to
> - guarantee that all pointer-to incomplete type variants
> - will be replaced by p
Re: Do less generous pointer globbing in alias.c
> The problem is that ipa_icf::sem_function::parse is lacking the f->init > call and the function epilog, falling through to the next function that > happens to follow. Thank you, that is indeed the devirtualization issue - I suppose the function descriptors confuses the code even more. I will finish the fix for that tonight. Honza
Re: Do less generous pointer globbing in alias.c
The problem is that ipa_icf::sem_function::parse is lacking the f->init call and the function epilog, falling through to the next function that happens to follow. Revision r223897: Dump of assembler code for function ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*): 0x4194c480 <+0>: [MMI] alloc r36=ar.pfs,12,6,0 0x4194c481 <+1>: adds r14=16,r32 0x4194c482 <+2>: mov r35=b0 0x4194c490 <+16>:[MMI] mov r37=r1;; 0x4194c491 <+17>:ld8 r38=[r14] 0x4194c492 <+18>:nop.i 0x0;; 0x4194c4a0 <+32>:[MMI] ld2 r14=[r38];; 0x4194c4a1 <+33>:cmp4.eq p6,p7=32,r14 0x4194c4a2 <+34>:nop.i 0x0;; 0x4194c4b0 <+48>:[MMI] (p07) addl r39=-833288,r1 0x4194c4b1 <+49>: (p07) mov r43=r0 0x4194c4b2 <+50>:nop.i 0x0 0x4194c4c0 <+64>:[MMI] (p07) mov r42=32 0x4194c4c1 <+65>: (p07) mov r40=1693 0x4194c4c2 <+66>: (p07) addl r41=-628512,r1;; 0x4194c4d0 <+80>:[MIB] (p07) ld8 r39=[r39] 0x4194c4d1 <+81>:nop.i 0x0 0x4194c4d2 <+82>: (p07) br.call.spnt.many b0=0x41530200 ;; 0x4194c4e0 <+96>:[MMI] adds r14=152,r38;; 0x4194c4e1 <+97>:ld8 r14=[r14] 0x4194c4e2 <+98>:nop.i 0x0;; 0x4194c4f0 <+112>: [MIB] cmp.eq p6,p7=0,r14 0x4194c4f1 <+113>: nop.i 0x0 0x4194c4f2 <+114>: (p06) br.cond.dpnt.few 0x4194c5f0 ;; 0x4194c500 <+128>: [MMI] nop.m 0x0 0x4194c501 <+129>: ld8 r14=[r32] 0x4194c502 <+130>: nop.i 0x0;; 0x4194c510 <+144>: [MIB] nop.m 0x0 0x4194c511 <+145>: tbit.z p6,p7=r14,16 0x4194c512 <+146>: (p06) br.cond.dptk.few 0x4194c610 0x4194c520 <+160>: [MMI] adds r15=387,r32;; 0x4194c521 <+161>: ld1 r15=[r15] 0x4194c522 <+162>: nop.i 0x0;; 0x4194c530 <+176>: [MIB] nop.m 0x0 0x4194c531 <+177>: cmp4.eq p7,p6=0,r15 0x4194c532 <+178>: (p06) br.cond.dptk.few 0x4194c550 ;; 0x4194c540 <+192>: [MIB] nop.m 0x0 0x4194c541 <+193>: tbit.z p7,p6=r14,17 0x4194c542 <+194>: (p06) br.cond.dptk.few 0x4194c5f0 0x4194c550 <+208>: [MMI] addl r14=-911832,r1;; 0x4194c551 <+209>: ld8 r14=[r14] 0x4194c552 <+210>: nop.i 0x0;; 0x4194c560 <+224>: [MMI] adds r14=2059,r14;; 0x4194c561 <+225>: ld1 r14=[r14] 0x4194c562 <+226>: nop.i 0x0;; 0x4194c570 <+240>: [MII] cmp4.eq p6,p7=1,r14 0x4194c571 <+241>: nop.i 0x0;; 0x4194c572 <+242>: (p07) addl r40=-833288,r1 0x4194c580 <+256>: [MMI] (p07) addl r42=-628512,r1 0x4194c581 <+257>: (p07) mov r41=1698 0x4194c582 <+258>: (p07) mov r39=11;; 0x4194c590 <+272>: [MIB] (p07) ld8 r40=[r40] 0x4194c591 <+273>: nop.i 0x0 0x4194c592 <+274>: (p07) br.call.spnt.many b0=0x41532200 ;; 0x4194c5a0 <+288>: [MMI] adds r38=88,r38 0x4194c5a1 <+289>: nop.m 0x0 0x4194c5a2 <+290>: mov r39=4;; 0x4194c5b0 <+304>: [MMI] ld8 r40=[r38] 0x4194c5b1 <+305>: nop.m 0x0 0x4194c5b2 <+306>: addl r38=-832208,r1;; 0x4194c5c0 <+320>: [MIB] cmp.eq p7,p6=0,r40 0x4194c5c1 <+321>: nop.i 0x0 0x4194c5c2 <+322>: (p07) br.cond.dpnt.few 0x4194c650 ;; 0x4194c5d0 <+336>: [MIB] ld8 r38=[r38] 0x4194c5d1 <+337>: nop.i 0x0 0x4194c5d2 <+338>: br.call.sptk.many b0=0x41534f40 ;; 0x4194c5e0 <+352>: [MIB] mov r1=r37 0x4194c5e1 <+353>: cmp.eq p6,p7=0,r8 0x4194c5e2 <+354>: (p06) br.cond.spnt.few 0x4194c650 0x4194c5f0 <+368>: [MMI] mov r8=r0 0x4194c5f1 <+369>: nop.m 0x0 0x4194c5f2 <+370>: mov.i ar.pfs=r36;; 0x4194c600 <+384>: [MIB] nop.m 0x0 0x4194c601 <+385>: mov b0=r35 0x4194c602 <+386>: br.ret.sptk.many b0 0x4194c610 <+400>: [MMI] adds r14=387,r32;; 0x4194c611 <+401>:
Re: Do less generous pointer globbing in alias.c
Jan Hubicka writes: > I am not sure I have access to working ia64 box. Can you possibly help me > to debug this? Is it devirtualization related? Here's a backtrace: 0x40422311 in bitmap_obstack_alloc_stat ( bit_obstack=0x4194cf20 ) at ../../gcc/bitmap.c:288 288 bit_obstack->heads = (struct bitmap_head *) map->first; (gdb) bt full #0 0x40422311 in bitmap_obstack_alloc_stat ( bit_obstack=0x4194cf20 ) at ../../gcc/bitmap.c:288 map = 0x8401880020420020 #1 0x4192bf10 in ipa_icf::sem_item::setup (this=0x20ab0350, stack=0x4194cf20 ) at ../../gcc/ipa-icf.c:224 No locals. #2 0x4194c730 in ipa_icf::sem_variable::sem_variable ( this=0x20ab0350, node=0x60309430, _hash=3188864, stack=0x4194cf20 ) at ../../gcc/ipa-icf.c:1847 No locals. #3 0x4194c690 in sem_function (stack=0x6008c4d8 , hash=0, node=0x7fff, this=0x20966920) at ../../gcc/ipa-icf.c:286 No locals. #4 ipa_icf::sem_function::parse (node=0x7fff, stack=0x6008c4d8 ) at ../../gcc/ipa-icf.c:1701 fndecl = func = __FUNCTION__ = "parse" #5 0x40d24110 in execute_ipa_summary_passes ( ipa_pass=0x20ab0350) at ../../gcc/passes.c:2143 pass = 0x20ab0350 #6 0x4194eb00 in ipa_icf::ipa_icf_generate_summary () at ../../gcc/ipa-icf.c:3502 No locals. #7 0x601230b0 in default_target_ira_int () No symbol table info available. #8 0x4192bf10 in ipa_icf::sem_item::setup (this=0xffc0, stack=0x0) at ../../gcc/ipa-icf.c:224 No locals. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: Do less generous pointer globbing in alias.c
> Jan Hubicka writes: > > > * alias.c (alias_set_entry_d): Add is_pointer and has_pointer. > > (alias_stats): Add num_universal. > > (alias_set_subset_of): Special case pointers; be ready for NULL > > children. > > (alias_sets_conflict_p): Special case pointers; be ready for NULL > > children. > > (init_alias_set_entry): Break out from ... > > (record_alias_subset): ... here; propagate new fields; > > allocate children only when really needed. > > (get_alias_set): Do less generous pointer globbing. > > (dump_alias_stats_in_alias_c): Update statistics. > > * gcc.dg/alias-8.c: Do not xfail. > > * gcc.dg/pr62167.c: Prevent FRE. > > * gcc.dg/alias-14.c: New testcase. > > This is causing a miscompilation of the stage2 compiler on ia64. Hmm, lovely :( According to GCC compile farm wiki, GCC 60 and 66 are ia64 but they are not. I am not sure I have access to working ia64 box. Can you possibly help me to debug this? Is it devirtualization related? With earlier versions of the patch I run into issue of ipa-icf ICEing in sem_function::parse because ipa_polymorphic_call_context::get_dynamic_type missed initialization of VPTR. I pushed out just partial fix to the issue as I want to test it on Firefox first and I am running into intresting issues with Firefox LTO right now (unrelated to the aliasing). With some luck it is the same problem because IA-64 has different representation of function pointers. Honza > > Andreas. > > -- > Andreas Schwab, sch...@linux-m68k.org > GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 > "And now for something completely different."
Re: Do less generous pointer globbing in alias.c
Jan Hubicka writes: > * alias.c (alias_set_entry_d): Add is_pointer and has_pointer. > (alias_stats): Add num_universal. > (alias_set_subset_of): Special case pointers; be ready for NULL > children. > (alias_sets_conflict_p): Special case pointers; be ready for NULL > children. > (init_alias_set_entry): Break out from ... > (record_alias_subset): ... here; propagate new fields; > allocate children only when really needed. > (get_alias_set): Do less generous pointer globbing. > (dump_alias_stats_in_alias_c): Update statistics. > * gcc.dg/alias-8.c: Do not xfail. > * gcc.dg/pr62167.c: Prevent FRE. > * gcc.dg/alias-14.c: New testcase. This is causing a miscompilation of the stage2 compiler on ia64. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: Do less generous pointer globbing in alias.c
On Thu, May 28, 2015 at 1:12 PM, Jan Hubicka wrote:
> hello,
> only providing you the testcase why I need transitive closure of "contains
> pointer" via the extra child I noticed that there is extra symmetry to handle:
>
> struct a {void *ptr;}
> char **ptr = (char **)&a.ptr;
> ptr = ...
>
> This one doesn't really fly with my extra subset code, because ptr is not
> universal pointer, but struct a contains one and thus should conflict with
> every pointer. Adding every pointer as subset of every structure with
> universal pointer is impractical (childs of those structures would be
> appearing
> as new pointer types get alias sets) and thus indeed it is better to handle it
> same way as alias set 0 - by a special case in alias_set_subset_of
> and alias_sets_conflict_p.
>
> So I added the second flag - has_pointer that is transitive closure of
> is_pointer and added the special case to alias_sets_conflict_p instead of
> adding the extra subset relation into the DAG.
>
> I also added statistics and made changes you suggested (making child
> hash to be possibly NULL and clenaing up alias set conflict construction)
>
> I also constructed a testcase that covers all the new code paths.
>
> The patch bootstrapped/regtested ppc64-linux. I am not bound to teaching
> next week, so if I hear no negative comments, I will schedule commiting the
> patch for weekend to deal with possible fallout.
>
> There are few cleanups possible incrementally - i.e. the hash set seems
> irrationaly large for average type, we could avoid some pointer travelling
> overhead and we could also do better at alias_sets_must_conflict_p.
>
> Honza
>
> * alias.c (alias_set_entry_d): Add is_pointer and has_pointer.
> (alias_stats): Add num_universal.
> (alias_set_subset_of): Special case pointers; be ready for NULL
> children.
> (alias_sets_conflict_p): Special case pointers; be ready for NULL
> children.
> (init_alias_set_entry): Break out from ...
> (record_alias_subset): ... here; propagate new fields;
> allocate children only when really needed.
> (get_alias_set): Do less generous pointer globbing.
> (dump_alias_stats_in_alias_c): Update statistics.
> * gcc.dg/alias-8.c: Do not xfail.
> * gcc.dg/pr62167.c: Prevent FRE.
> * gcc.dg/alias-14.c: New testcase.
==
> --- testsuite/gcc.dg/alias-8.c (revision 223772)
> +++ testsuite/gcc.dg/alias-8.c (working copy)
> @@ -8,5 +8,5 @@ struct s {
> void
> func(struct s *ptr)
> {
> - *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { xfail
> *-*-* } } */
> + *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { } } */
> }
This caused:
ERROR: gcc.dg/alias-8.c: syntax error in target selector "" for "
dg-warning 11 "type-punned pointer" "" { } "
I checked in this fix.
H.J.
---
Index: ChangeLog
===
--- ChangeLog (revision 223886)
+++ ChangeLog (working copy)
@@ -1,3 +1,7 @@
+2015-05-30 H.J. Lu
+
+ * gcc.dg/alias-8.c: Fix dg-warning.
+
2015-05-30 Jan Hubicka
* gcc.dg/alias-8.c: Do not xfail.
Index: gcc.dg/alias-8.c
===
--- gcc.dg/alias-8.c (revision 223886)
+++ gcc.dg/alias-8.c (working copy)
@@ -8,5 +8,5 @@
void
func(struct s *ptr)
{
- *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { } } */
+ *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" } */
}
Re: Do less generous pointer globbing in alias.c
hello,
only providing you the testcase why I need transitive closure of "contains
pointer" via the extra child I noticed that there is extra symmetry to handle:
struct a {void *ptr;}
char **ptr = (char **)&a.ptr;
ptr = ...
This one doesn't really fly with my extra subset code, because ptr is not
universal pointer, but struct a contains one and thus should conflict with
every pointer. Adding every pointer as subset of every structure with
universal pointer is impractical (childs of those structures would be appearing
as new pointer types get alias sets) and thus indeed it is better to handle it
same way as alias set 0 - by a special case in alias_set_subset_of
and alias_sets_conflict_p.
So I added the second flag - has_pointer that is transitive closure of
is_pointer and added the special case to alias_sets_conflict_p instead of
adding the extra subset relation into the DAG.
I also added statistics and made changes you suggested (making child
hash to be possibly NULL and clenaing up alias set conflict construction)
I also constructed a testcase that covers all the new code paths.
The patch bootstrapped/regtested ppc64-linux. I am not bound to teaching
next week, so if I hear no negative comments, I will schedule commiting the
patch for weekend to deal with possible fallout.
There are few cleanups possible incrementally - i.e. the hash set seems
irrationaly large for average type, we could avoid some pointer travelling
overhead and we could also do better at alias_sets_must_conflict_p.
Honza
* alias.c (alias_set_entry_d): Add is_pointer and has_pointer.
(alias_stats): Add num_universal.
(alias_set_subset_of): Special case pointers; be ready for NULL
children.
(alias_sets_conflict_p): Special case pointers; be ready for NULL
children.
(init_alias_set_entry): Break out from ...
(record_alias_subset): ... here; propagate new fields;
allocate children only when really needed.
(get_alias_set): Do less generous pointer globbing.
(dump_alias_stats_in_alias_c): Update statistics.
* gcc.dg/alias-8.c: Do not xfail.
* gcc.dg/pr62167.c: Prevent FRE.
* gcc.dg/alias-14.c: New testcase.
Index: alias.c
===
--- alias.c (revision 223772)
+++ alias.c (working copy)
@@ -183,10 +184,6 @@ struct GTY(()) alias_set_entry_d {
/* The alias set number, as stored in MEM_ALIAS_SET. */
alias_set_type alias_set;
- /* Nonzero if would have a child of zero: this effectively makes this
- alias set the same as alias set zero. */
- int has_zero_child;
-
/* The children of the alias set. These are not just the immediate
children, but, in fact, all descendants. So, if we have:
@@ -195,6 +192,17 @@ struct GTY(()) alias_set_entry_d {
continuing our example above, the children here will be all of
`int', `double', `float', and `struct S'. */
hash_map *children;
+
+ /* Nonzero if would have a child of zero: this effectively makes this
+ alias set the same as alias set zero. */
+ bool has_zero_child;
+ /* Nonzero if alias set corresponds to pointer type itself (i.e. not to
+ aggregate contaiing pointer.
+ This is used for a special case where we need an universal pointer type
+ compatible with all other pointer types. */
+ bool is_pointer;
+ /* Nonzero if is_pointer or if one of childs have has_pointer set. */
+ bool has_pointer;
};
typedef struct alias_set_entry_d *alias_set_entry;
@@ -222,6 +230,7 @@ static struct {
unsigned long long num_same_objects;
unsigned long long num_volatile;
unsigned long long num_dag;
+ unsigned long long num_universal;
unsigned long long num_disambiguated;
} alias_stats;
@@ -454,18 +463,58 @@ mems_in_disjoint_alias_sets_p (const_rtx
bool
alias_set_subset_of (alias_set_type set1, alias_set_type set2)
{
- alias_set_entry ase;
+ alias_set_entry ase2;
/* Everything is a subset of the "aliases everything" set. */
if (set2 == 0)
return true;
- /* Otherwise, check if set1 is a subset of set2. */
- ase = get_alias_set_entry (set2);
- if (ase != 0
- && (ase->has_zero_child
- || ase->children->get (set1)))
+ /* Check if set1 is a subset of set2. */
+ ase2 = get_alias_set_entry (set2);
+ if (ase2 != 0
+ && (ase2->has_zero_child
+ || (ase2->children && ase2->children->get (set1
return true;
+
+ /* As a special case we consider alias set of "void *" to be both subset
+ and superset of every alias set of a pointer. This extra symmetry does
+ not matter for alias_sets_conflict_p but it makes
aliasing_component_refs_p
+ to return true on the following testcase:
+
+ void *ptr;
+ char **ptr2=(char **)&ptr;
+ *ptr2 = ...
+
+ Additionally if a set contains universal pointer, we consider every
pointer
+ to be a subset of it, but we do not represent
Re: Do less generous pointer globbing in alias.c
> > +alias_set_entry
> > +init_alias_set_entry (alias_set_type set)
> > +{
> > + alias_set_entry ase = ggc_cleared_alloc ();
>
> no need to use cleared_alloc if you also init ->is_pointer to false.
OK, will update the patch.
>
> > + ase->alias_set = set;
> > + ase->children
> > += hash_map::create_ggc (64);
>
> that seems a bit excessive, esp. for pointers which won't end
> up with any children? So better make children lazily allocated
> in record_alias_subset.
All pointers that are not in alias set of ptr_type_node will have a child.
So there is only one childless pointer set. I will update the code though.
>
> I still wonder why you do this instead of changing alias_sets_conflict
> in the same way you changed alias_set_subset_of.
Because I would need two flags otherwise. One denoting alias sets that
are pointers (who needs special treatment for subset_of) and one denoting
alias set that contains pointer.
i.e. for:
struct {int *a,b;}
I need to have its alias set to contain all of setof(int), setof(int *),
setof(void *).
I however do not want setof(struct {int *a,b;}) to be subset of setof(void *)
Honza
>
> Patch looks ok otherwise but please leave the patch for others to
> comment on for a while.
>
> Thanks,
> Richard.
>
> > + }
> > + }
> > +}
> > + /* In LTO the rules above needs to be part of canonical type machinery.
> > + For now just punt. */
> > + else if (POINTER_TYPE_P (t) && t != ptr_type_node && in_lto_p)
> > set = get_alias_set (ptr_type_node);
> >
> >/* Otherwise make a new alias set for this type. */
> > @@ -953,6 +1052,15 @@ get_alias_set (tree t)
> >if (AGGREGATE_TYPE_P (t) || TREE_CODE (t) == COMPLEX_TYPE)
> > record_component_aliases (t);
> >
> > + /* We treat pointer types specially in alias_set_subset_of. */
> > + if (POINTER_TYPE_P (t) && set)
> > +{
> > + alias_set_entry ase = get_alias_set_entry (set);
> > + if (!ase)
> > + ase = init_alias_set_entry (set);
> > + ase->is_pointer = true;
> > +}
> > +
> >return set;
> > }
> >
> > @@ -1003,12 +,7 @@ record_alias_subset (alias_set_type supe
> > {
> >/* Create an entry for the SUPERSET, so that we have a place to
> > attach the SUBSET. */
> > - superset_entry = ggc_cleared_alloc ();
> > - superset_entry->alias_set = superset;
> > - superset_entry->children
> > - = hash_map::create_ggc (64);
> > - superset_entry->has_zero_child = 0;
> > - (*alias_sets)[superset] = superset_entry;
> > + superset_entry = init_alias_set_entry (superset);
> > }
> >
> >if (subset == 0)
> > Index: testsuite/gcc.dg/alias-8.c
> > ===
> > --- testsuite/gcc.dg/alias-8.c (revision 223772)
> > +++ testsuite/gcc.dg/alias-8.c (working copy)
> > @@ -8,5 +8,5 @@ struct s {
> > void
> > func(struct s *ptr)
> > {
> > - *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { xfail
> > *-*-* } } */
> > + *(void **)&ptr->p = 0; /* { dg-warning "type-punned pointer" "" { } } */
> > }
> > Index: testsuite/gcc.dg/pr62167.c
> > ===
> > --- testsuite/gcc.dg/pr62167.c (revision 223772)
> > +++ testsuite/gcc.dg/pr62167.c (working copy)
> > @@ -29,6 +29,8 @@ main ()
> >
> >node.prev = (void *)head;
> >
> > + asm("":"=m"(node.prev));
> > +
> >head->first = &node;
> >
> >struct node *n = head->first;
> >
> >
>
> --
> Richard Biener
> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham
> Norton, HRB 21284 (AG Nuernberg)
Re: Do less generous pointer globbing in alias.c
On Thu, 28 May 2015, Jan Hubicka wrote:
> Hi,
> here is updated version of patch. It makes alias_set_subset_of to be
> symmetric for
> ptr_type_node and other pointer type and moves the logic of creating subsets
> to get_alias_set.
>
> I tested that perlbmk works when built at -O3 x86_64
>
> Bootstrapped/regtested x86_64-linux, OK?
>
> Honza
>
> * alias.c (alias_set_entry_d): Add is_pointer.
> (alias_set_subset_of): Special case pointers.
> (init_alias_set_entry): Break out from ...
> (record_alias_subset): ... here.
> (get_alias_set): Do less generous pointer globbing.
> * gcc.dg/alias-8.c: Do not xfail.
> * gcc.dg/pr62167.c: Prevent FRE.
> Index: alias.c
> ===
> --- alias.c (revision 223772)
> +++ alias.c (working copy)
> @@ -183,10 +184,6 @@ struct GTY(()) alias_set_entry_d {
>/* The alias set number, as stored in MEM_ALIAS_SET. */
>alias_set_type alias_set;
>
> - /* Nonzero if would have a child of zero: this effectively makes this
> - alias set the same as alias set zero. */
> - int has_zero_child;
> -
>/* The children of the alias set. These are not just the immediate
> children, but, in fact, all descendants. So, if we have:
>
> @@ -195,6 +192,15 @@ struct GTY(()) alias_set_entry_d {
> continuing our example above, the children here will be all of
> `int', `double', `float', and `struct S'. */
>hash_map *children;
> +
> + /* Nonzero if would have a child of zero: this effectively makes this
> + alias set the same as alias set zero. */
> + bool has_zero_child;
> + /* Nonzero if alias set corresponds to pointer type itself (i.e. not to
> + aggregate contaiing pointer.
> + This is used for a special case where we need an universal pointer type
> + compatible with all other pointer types. */
> + bool is_pointer;
> };
> typedef struct alias_set_entry_d *alias_set_entry;
>
> @@ -460,12 +466,33 @@ alias_set_subset_of (alias_set_type set1
>if (set2 == 0)
> return true;
>
> - /* Otherwise, check if set1 is a subset of set2. */
> + /* Check if set1 is a subset of set2. */
>ase = get_alias_set_entry (set2);
>if (ase != 0
>&& (ase->has_zero_child
> || ase->children->get (set1)))
> return true;
> +
> + /* As a special case we consider alias set of "void *" to be both subset
> + and superset of every alias set of a pointer. This extra symmetry does
> + not matter for alias_sets_conflict_p but it makes
> aliasing_component_refs_p
> + to return true on the following testcase:
> +
> + void *ptr;
> + char **ptr2=(char **)&ptr;
> + *ptr2 = ...
> +
> + This makes void * truly universal pointer type. See pointer handling in
> + get_alias_set for more details. */
> + if (ase && ase->is_pointer)
> +{
> + alias_set_entry ase1 = get_alias_set_entry (set1);
> +
> + if (ase1 && ase1->is_pointer
> + && (set1 == TYPE_ALIAS_SET (ptr_type_node)
> + || set2 == TYPE_ALIAS_SET (ptr_type_node)))
> + return true;
> +}
>return false;
> }
>
> @@ -764,6 +791,21 @@ alias_ptr_types_compatible_p (tree t1, t
> == TYPE_MAIN_VARIANT (TREE_TYPE (t2)));
> }
>
> +/* Create emptry alias set entry. */
> +
> +alias_set_entry
> +init_alias_set_entry (alias_set_type set)
> +{
> + alias_set_entry ase = ggc_cleared_alloc ();
no need to use cleared_alloc if you also init ->is_pointer to false.
> + ase->alias_set = set;
> + ase->children
> += hash_map::create_ggc (64);
that seems a bit excessive, esp. for pointers which won't end
up with any children? So better make children lazily allocated
in record_alias_subset.
> + ase->has_zero_child = 0;
> + gcc_checking_assert (!get_alias_set_entry (set));
> + (*alias_sets)[set] = ase;
> + return ase;
> +}
> +
> /* Return the alias set for T, which may be either a type or an
> expression. Call language-specific routine for help, if needed. */
>
> @@ -903,35 +945,92 @@ get_alias_set (tree t)
> the pointed-to types. This issue has been reported to the
> C++ committee.
>
> - In addition to the above canonicalization issue, with LTO
> - we should also canonicalize `T (*)[]' to `T *' avoiding
> - alias issues with pointer-to element types and pointer-to
> - array types.
> -
> - Likewise we need to deal with the situation of incomplete
> - pointed-to types and make `*(struct X **)&a' and
> - `*(struct X {} **)&a' alias. Otherwise we will have to
> - guarantee that all pointer-to incomplete type variants
> - will be replaced by pointer-to complete type variants if
> - they are available.
> -
> - With LTO the convenient situation of using `void *' to
> - access and store any pointer type will also become
> - more apparent (and `void *' is just another pointer-to
> - incomplete type). Assigning alias-s
Re: Do less generous pointer globbing in alias.c
Hi,
here is updated version of patch. It makes alias_set_subset_of to be symmetric
for
ptr_type_node and other pointer type and moves the logic of creating subsets
to get_alias_set.
I tested that perlbmk works when built at -O3 x86_64
Bootstrapped/regtested x86_64-linux, OK?
Honza
* alias.c (alias_set_entry_d): Add is_pointer.
(alias_set_subset_of): Special case pointers.
(init_alias_set_entry): Break out from ...
(record_alias_subset): ... here.
(get_alias_set): Do less generous pointer globbing.
* gcc.dg/alias-8.c: Do not xfail.
* gcc.dg/pr62167.c: Prevent FRE.
Index: alias.c
===
--- alias.c (revision 223772)
+++ alias.c (working copy)
@@ -183,10 +184,6 @@ struct GTY(()) alias_set_entry_d {
/* The alias set number, as stored in MEM_ALIAS_SET. */
alias_set_type alias_set;
- /* Nonzero if would have a child of zero: this effectively makes this
- alias set the same as alias set zero. */
- int has_zero_child;
-
/* The children of the alias set. These are not just the immediate
children, but, in fact, all descendants. So, if we have:
@@ -195,6 +192,15 @@ struct GTY(()) alias_set_entry_d {
continuing our example above, the children here will be all of
`int', `double', `float', and `struct S'. */
hash_map *children;
+
+ /* Nonzero if would have a child of zero: this effectively makes this
+ alias set the same as alias set zero. */
+ bool has_zero_child;
+ /* Nonzero if alias set corresponds to pointer type itself (i.e. not to
+ aggregate contaiing pointer.
+ This is used for a special case where we need an universal pointer type
+ compatible with all other pointer types. */
+ bool is_pointer;
};
typedef struct alias_set_entry_d *alias_set_entry;
@@ -460,12 +466,33 @@ alias_set_subset_of (alias_set_type set1
if (set2 == 0)
return true;
- /* Otherwise, check if set1 is a subset of set2. */
+ /* Check if set1 is a subset of set2. */
ase = get_alias_set_entry (set2);
if (ase != 0
&& (ase->has_zero_child
|| ase->children->get (set1)))
return true;
+
+ /* As a special case we consider alias set of "void *" to be both subset
+ and superset of every alias set of a pointer. This extra symmetry does
+ not matter for alias_sets_conflict_p but it makes
aliasing_component_refs_p
+ to return true on the following testcase:
+
+ void *ptr;
+ char **ptr2=(char **)&ptr;
+ *ptr2 = ...
+
+ This makes void * truly universal pointer type. See pointer handling in
+ get_alias_set for more details. */
+ if (ase && ase->is_pointer)
+{
+ alias_set_entry ase1 = get_alias_set_entry (set1);
+
+ if (ase1 && ase1->is_pointer
+ && (set1 == TYPE_ALIAS_SET (ptr_type_node)
+ || set2 == TYPE_ALIAS_SET (ptr_type_node)))
+ return true;
+}
return false;
}
@@ -764,6 +791,21 @@ alias_ptr_types_compatible_p (tree t1, t
== TYPE_MAIN_VARIANT (TREE_TYPE (t2)));
}
+/* Create emptry alias set entry. */
+
+alias_set_entry
+init_alias_set_entry (alias_set_type set)
+{
+ alias_set_entry ase = ggc_cleared_alloc ();
+ ase->alias_set = set;
+ ase->children
+= hash_map::create_ggc (64);
+ ase->has_zero_child = 0;
+ gcc_checking_assert (!get_alias_set_entry (set));
+ (*alias_sets)[set] = ase;
+ return ase;
+}
+
/* Return the alias set for T, which may be either a type or an
expression. Call language-specific routine for help, if needed. */
@@ -903,35 +945,92 @@ get_alias_set (tree t)
the pointed-to types. This issue has been reported to the
C++ committee.
- In addition to the above canonicalization issue, with LTO
- we should also canonicalize `T (*)[]' to `T *' avoiding
- alias issues with pointer-to element types and pointer-to
- array types.
-
- Likewise we need to deal with the situation of incomplete
- pointed-to types and make `*(struct X **)&a' and
- `*(struct X {} **)&a' alias. Otherwise we will have to
- guarantee that all pointer-to incomplete type variants
- will be replaced by pointer-to complete type variants if
- they are available.
-
- With LTO the convenient situation of using `void *' to
- access and store any pointer type will also become
- more apparent (and `void *' is just another pointer-to
- incomplete type). Assigning alias-set zero to `void *'
- and all pointer-to incomplete types is a not appealing
- solution. Assigning an effective alias-set zero only
- affecting pointers might be - by recording proper subset
- relationships of all pointer alias-sets.
-
- Pointer-to function types are another grey area which
- needs caution. Globbing them all into one alias-set
- or the above effective zero set would work.
-
- For now just assign the same alias-set to all pointers.
-
Re: Do less generous pointer globbing in alias.c
> >Hmm, what about
> >
> >union t {int a; char b;};
> >
> >int a;
> >uniont t *ptr=&a;
> >*ptr = ...
> >
> >If we want to define this, aliasing_component_refs_p would IMO need to
> >be symmetrized, too.
> >I am happy leaving this undefined.
>
> Globbing all pointers was soo simple... :)
Indeed, but too restrictive ;)
The testcase above is not about globbing pointers, I do not think it is going
to be handled in defined manner by mainline (or any release).
>
> Note that we are in the middle-end here and have to find cross-language
> common grounds. People may experience regressions towards the previous
> globbing so I guess the question is which is the globbing we want to remove -
> that is, what makes the most difference in code-generation?
Yes, I expect to see some PRs with regress towards the previous globbing. I
think the globbing as proposed by my patch should be generous enough for common
bugs in user code and it is quite easy to add new rules on demand.
For high-level C++ code definitely the most important point is that you have
many different class types and we care about differentiating these (struct *a
wrt struct *b). We also want to make difference between vtbl pointer (that is
pointer to array of functions) and other stuff.
I think I will modify the patch the following way:
1) I will move the code adding subset to get_alias_set
2) I will add flag "is_pointer" to alias set datastructure
3) I will make alias_set_subset_of to additionally consider
every "is_pointer" set to be subset of alias set of ptr_type_node's set.
This will fix the symmetry with void *a; variable and incompatible pointer
write.
We need to do two things - arrange alias set to be subset of all pointer's
alias sets
and all their superset and force equivalence between pointer alias sets.
While the first can be also done by means of special flag "contains_pointer"
I think it is cleaner to keep the DAG reprsented explicitely. After all we do
not
have that many alias sets and the hash table lookups should be fast enough
(we may special case lookup in hash of size 1)
Hona
>
> Richard.
>
> >Honza
>
Re: Do less generous pointer globbing in alias.c
On May 27, 2015 5:04:13 PM GMT+02:00, Jan Hubicka wrote:
>> > Yes, so is
>> >
>> > struct foo {struct bar a;};
>> >
>> > a.a = ...
>> > ... = a;
>> >
>> > and
>> >
>> > a = ...
>> > ... = a.a;
>> >
>> > this is why conflict is symmetrization of the subset relation.
>>
>>
>> OK the statement above is true, but subsets alone are not quite right
>for use
>> in aliasing_component_refs_p
>>
>> void *a;
>> char **ptr=&a;
>> *ptr =
>>
>> is defined for us, but the structure-substructure equivalent is not.
>> I will implement the variant with extra flag after teaching and send
>updated
>> patch.
>
>Hmm, what about
>
>union t {int a; char b;};
>
>int a;
>uniont t *ptr=&a;
>*ptr = ...
>
>If we want to define this, aliasing_component_refs_p would IMO need to
>be symmetrized, too.
>I am happy leaving this undefined.
Globbing all pointers was soo simple... :)
Note that we are in the middle-end here and have to find cross-language common
grounds. People may experience regressions towards the previous globbing so I
guess the question is which is the globbing we want to remove - that is, what
makes the most difference in code-generation?
Richard.
>Honza
Re: Do less generous pointer globbing in alias.c
> > Yes, so is
> >
> > struct foo {struct bar a;};
> >
> > a.a = ...
> > ... = a;
> >
> > and
> >
> > a = ...
> > ... = a.a;
> >
> > this is why conflict is symmetrization of the subset relation.
>
>
> OK the statement above is true, but subsets alone are not quite right for use
> in aliasing_component_refs_p
>
> void *a;
> char **ptr=&a;
> *ptr =
>
> is defined for us, but the structure-substructure equivalent is not.
> I will implement the variant with extra flag after teaching and send updated
> patch.
Hmm, what about
union t {int a; char b;};
int a;
uniont t *ptr=&a;
*ptr = ...
If we want to define this, aliasing_component_refs_p would IMO need to be
symmetrized, too.
I am happy leaving this undefined.
Honza
Re: Do less generous pointer globbing in alias.c
> Yes, so is
>
> struct foo {struct bar a;};
>
> a.a = ...
> ... = a;
>
> and
>
> a = ...
> ... = a.a;
>
> this is why conflict is symmetrization of the subset relation.
OK the statement above is true, but subsets alone are not quite right for use
in aliasing_component_refs_p
void *a;
char **ptr=&a;
*ptr =
is defined for us, but the structure-substructure equivalent is not.
I will implement the variant with extra flag after teaching and send updated
patch.
Thanks,
Honza
Re: Do less generous pointer globbing in alias.c
> > Hi, this patch makes it possible for non-LTO alias oracle to TBAA
> > disambiguate pointer types. It makes void * conflicting with all of them
> > and does not put it to alias set 0. It also preserves the property that
> > qualifiers of pointer-to type should not matter to determine the alias
> > set and that pointer to array is same as pointer to array element.
> > Finally it makes pointer void * to be equivalent to void ** (and more *)
> > and to types with structural equality only.
>
> void * should be equivalent to incomplete-type * as well.
It will be in conflict with struct FOO * when FOO is incomplete.
In non-LTO build struct FOO * do not need to conflict wqith struct BAR *.
Or do I miss something here?
>
> > I think those are all globbing rules we discussed for the non-LTO patch.
> >
> > It does two things. First is kind of "canonicalization" where for a given
> > pointer
> > it looks for non-pointer pointed-to type and then rebuilds is without
> > qualifiers.
> > This is fast, because build_pointer_type will reuse existing types.
> >
> > It makes void * to conflict with everyting by making its alias set to be
> > subset
> > of alias set of any other pointer. This means that writes to void *
> > conflict
> > with writes to any other pointer without really need to glob all the
> > pointers
> > to one equivalence class.
>
> I think you need to make each pointer alias-set a subset of the one of
> void * as well because both of the following is valid:
>
> *(void *)p = ...
> ... = *(int *)p;
>
> and
>
> *(int *)p = ...
> ... = *(void *)p;
Yes, so is
struct foo {struct bar a;};
a.a = ...
... = a;
and
a = ...
... = a.a;
this is why conflict is symmetrization of the subset relation.
You can not record both edges into the DAG, because record_alias_subset
compute transitive closure and it would end up in loop. I will be hapy
to add the extra flag (has_zero_child), but I would like to make it
clear it an optimization.
>
> not sure if it's possible to create a testcase that fails if you do
> subsetting only one-way (because alias_sets_conflict queries both
> ways and I think alias_set_subset_of is not used very much, only
> by tree-ssa-alias.c:aliasing_component_refs_p which won't ever
> use it on two pointer alias sets). In theory true vs. anti-dependence
Yep, I noticed that subsets are querried by tree-ssa-alias. I will try to
think if it is safe WRT the code above.
> should use alias_set_subset_of and trigger the above cases. But
> as those queries are done wrong a lot (in the past?) we use
> alias_sets_conflict there.
>
> For efficiency you could use a new flag similar to has_zero_child
> in alias_set_entry_d ...
Yes, I can use new flag, but it should be unnecesary. The alias set 0
is also just common subset of all aliases (that is not done by the code).
>
> I see no reason for punting for LTO here.
I would rather go with non-LTO first and work on solving the canonical type
issues. Yes, I think it should work for LTO as it is and I bootstrapped and
regtested it. I only wanted to do one step at a time.
What I do not like is that build_pointer_type simply does not do the right
thing here. Consdier
struct a {int a};
struct b {char b};
Now if you LTO in struct *a and struct *b their canonical type will be the same.
If you call build_pointer_type, it will assign different canonical types to
them.
This does not lead to wrong code, because incomplete types no longer get
TYPE_CANONICAL, but I would like first to chase out the bugs out of canonical
type computation and arrange middle-end build pointer types to be the same as
LTOed-in pointer types.
>
> Btw, please check if SPEC perl still works without -fno-strict-aliasing
> (it finally did after the change to do pointer globbing).
OK, I have SPEC perl available, so I will do.
I am teaching now, but so will reply in detail afterwards. I was just hoping
to discuss the symmetry thing above. I think it is not needed.
I have no problem with moving the subset code to get_alias_set and will update
the patch (including testsuite compensation).
Honza
Re: Do less generous pointer globbing in alias.c
On Wed, 27 May 2015, Jan Hubicka wrote: > Hi, this patch makes it possible for non-LTO alias oracle to TBAA > disambiguate pointer types. It makes void * conflicting with all of them > and does not put it to alias set 0. It also preserves the property that > qualifiers of pointer-to type should not matter to determine the alias > set and that pointer to array is same as pointer to array element. > Finally it makes pointer void * to be equivalent to void ** (and more *) > and to types with structural equality only. void * should be equivalent to incomplete-type * as well. > I think those are all globbing rules we discussed for the non-LTO patch. > > It does two things. First is kind of "canonicalization" where for a given > pointer > it looks for non-pointer pointed-to type and then rebuilds is without > qualifiers. > This is fast, because build_pointer_type will reuse existing types. > > It makes void * to conflict with everyting by making its alias set to be > subset > of alias set of any other pointer. This means that writes to void * conflict > with writes to any other pointer without really need to glob all the pointers > to one equivalence class. I think you need to make each pointer alias-set a subset of the one of void * as well because both of the following is valid: *(void *)p = ... ... = *(int *)p; and *(int *)p = ... ... = *(void *)p; not sure if it's possible to create a testcase that fails if you do subsetting only one-way (because alias_sets_conflict queries both ways and I think alias_set_subset_of is not used very much, only by tree-ssa-alias.c:aliasing_component_refs_p which won't ever use it on two pointer alias sets). In theory true vs. anti-dependence should use alias_set_subset_of and trigger the above cases. But as those queries are done wrong a lot (in the past?) we use alias_sets_conflict there. For efficiency you could use a new flag similar to has_zero_child in alias_set_entry_d ... More comments inline below > This patch makes quite some difference on C++. For example on deal II the > TBAA > stats reports 4344358 disambiguations and 7008576 queries, while with the > patch > we get 5368737 and 5687399 queries (I did not chose deal II for reason, it is > just random C++ file) > > The patch bootstrap and regtests ppc64le-linux with the following testsuite > differences: > @@ -30,7 +30,9 @@ > FAIL: c-c++-common/asan/null-deref-1.c -O3 -g output pattern test, is > ASAN:SIGSEGV > FAIL: c-c++-common/asan/null-deref-1.c -Os output pattern test, is > ASAN:SIGSEGV > FAIL: gcc.dg/cpp/_Pragma3.c (test for excess errors) > +XPASS: gcc.dg/alias-8.c (test for warnings, line 11) > FAIL: gcc.dg/loop-8.c scan-rtl-dump-times loop2_invariant "Decided" 1 > +FAIL: gcc.dg/pr62167.c scan-tree-dump-not pre "Removing basic block" > FAIL: gcc.dg/sms-4.c scan-rtl-dump-times sms "SMS succeeded" 1 > XPASS: gcc.dg/guality/example.c -O0 execution test > XPASS: gcc.dg/guality/example.c -O1 execution test > @@ -304,6 +306,9 @@ > FAIL: c-c++-common/asan/null-deref-1.c -O3 -g output pattern test, is > ASAN:SIGSEGV > FAIL: g++.dg/cpp1y/vla-initlist1.C -std=gnu++11 execution test > FAIL: g++.dg/cpp1y/vla-initlist1.C -std=gnu++14 execution test > +FAIL: g++.dg/ipa/ipa-icf-4.C -std=gnu++11 scan-ipa-dump icf "Equal > symbols: [67]" > +FAIL: g++.dg/ipa/ipa-icf-4.C -std=gnu++14 scan-ipa-dump icf "Equal > symbols: [67]" > +FAIL: g++.dg/ipa/ipa-icf-4.C -std=gnu++98 scan-ipa-dump icf "Equal > symbols: [67]" > > ipa-icf-4 is about alias info now being more perceptive to block the merging. > pr62167 seems just confused. The template checks that memory stores are not > unified. It looks for BB removal message, but with the patch we get: > : > node.next = 0B; > head.0_4 = head; > node.prev = head.0_4; > head.0_4->first = &node; > k.1_7 = k; > h_8 = &heads[k.1_7]; > heads[2].first = 0B; > if (head.0_4 == h_8) > goto ; > else > goto ; > > : > goto ; > > : > p_10 = MEM[(struct head *)&heads][k.1_7].first; > > : > # p_1 = PHI > _11 = p_1 != 0B; > _12 = (int) _11; > return _12; > > before PR, the message is about the bb 5 sitting at critical edge removed. > The TBAA incompatible load it looks for is optimized away by FRE: > head->first = &node; > > struct node *n = head->first; > > struct head *h = &heads[k]; > > heads[2].first = n->next; > > if ((void*)n->prev == (void *)h) > p = h->first; > else > /* Dead tbaa-unsafe load from ((struct node *)&heads[2])->next. */ > p = n->prev->next; > > here n is known to be head->first that is known to be &node. > The testcase runtime checks that result is Ok and passes. > > Bootstrapped/regtested ppc64le-linux. > > * alias.c (get_alias_set): Do not glob all pointer types into one; > just produce euqivalence classes based on canonical type of pointed > type type; make void * equivalent to void **. > (record_c
Re: Do less generous pointer globbing in alias.c
> Hi, > this patch makes it possible for non-LTO alias oracle to TBAA disambiguate > pointer types. It makes void * conflicting with all of them and does not put > it > to alias set 0. It also preserves the property that qualifiers of pointer-to > type should not matter to determine the alias set and that pointer to array is > same as pointer to array element. Finally it makes pointer void * to be > equivalent to void ** (and more *) and to types with structural equality only. > > I think those are all globbing rules we discussed for the non-LTO patch. > > It does two things. First is kind of "canonicalization" where for a given > pointer > it looks for non-pointer pointed-to type and then rebuilds is without > qualifiers. > This is fast, because build_pointer_type will reuse existing types. > > It makes void * to conflict with everyting by making its alias set to be > subset > of alias set of any other pointer. This means that writes to void * conflict > with writes to any other pointer without really need to glob all the pointers > to one equivalence class. > > This patch makes quite some difference on C++. For example on deal II the > TBAA > stats reports 4344358 disambiguations and 7008576 queries, while with the > patch > we get 5368737 and 5687399 queries (I did not chose deal II for reason, it is > just random C++ file) Actually there is oversight in my patch - the number of queries is really number of non-disambiguations. I will fix that as obvious tomorrow. In both cases the number of querries is about 11M. The increase in number of disambiguations is 23% (earlier patch did over 30% for Firefox) Honza
Do less generous pointer globbing in alias.c
Hi,
this patch makes it possible for non-LTO alias oracle to TBAA disambiguate
pointer types. It makes void * conflicting with all of them and does not put it
to alias set 0. It also preserves the property that qualifiers of pointer-to
type should not matter to determine the alias set and that pointer to array is
same as pointer to array element. Finally it makes pointer void * to be
equivalent to void ** (and more *) and to types with structural equality only.
I think those are all globbing rules we discussed for the non-LTO patch.
It does two things. First is kind of "canonicalization" where for a given
pointer
it looks for non-pointer pointed-to type and then rebuilds is without
qualifiers.
This is fast, because build_pointer_type will reuse existing types.
It makes void * to conflict with everyting by making its alias set to be subset
of alias set of any other pointer. This means that writes to void * conflict
with writes to any other pointer without really need to glob all the pointers
to one equivalence class.
This patch makes quite some difference on C++. For example on deal II the TBAA
stats reports 4344358 disambiguations and 7008576 queries, while with the patch
we get 5368737 and 5687399 queries (I did not chose deal II for reason, it is
just random C++ file)
The patch bootstrap and regtests ppc64le-linux with the following testsuite
differences:
@@ -30,7 +30,9 @@
FAIL: c-c++-common/asan/null-deref-1.c -O3 -g output pattern test, is
ASAN:SIGSEGV
FAIL: c-c++-common/asan/null-deref-1.c -Os output pattern test, is
ASAN:SIGSEGV
FAIL: gcc.dg/cpp/_Pragma3.c (test for excess errors)
+XPASS: gcc.dg/alias-8.c (test for warnings, line 11)
FAIL: gcc.dg/loop-8.c scan-rtl-dump-times loop2_invariant "Decided" 1
+FAIL: gcc.dg/pr62167.c scan-tree-dump-not pre "Removing basic block"
FAIL: gcc.dg/sms-4.c scan-rtl-dump-times sms "SMS succeeded" 1
XPASS: gcc.dg/guality/example.c -O0 execution test
XPASS: gcc.dg/guality/example.c -O1 execution test
@@ -304,6 +306,9 @@
FAIL: c-c++-common/asan/null-deref-1.c -O3 -g output pattern test, is
ASAN:SIGSEGV
FAIL: g++.dg/cpp1y/vla-initlist1.C -std=gnu++11 execution test
FAIL: g++.dg/cpp1y/vla-initlist1.C -std=gnu++14 execution test
+FAIL: g++.dg/ipa/ipa-icf-4.C -std=gnu++11 scan-ipa-dump icf "Equal symbols:
[67]"
+FAIL: g++.dg/ipa/ipa-icf-4.C -std=gnu++14 scan-ipa-dump icf "Equal symbols:
[67]"
+FAIL: g++.dg/ipa/ipa-icf-4.C -std=gnu++98 scan-ipa-dump icf "Equal symbols:
[67]"
ipa-icf-4 is about alias info now being more perceptive to block the merging.
pr62167 seems just confused. The template checks that memory stores are not
unified. It looks for BB removal message, but with the patch we get:
:
node.next = 0B;
head.0_4 = head;
node.prev = head.0_4;
head.0_4->first = &node;
k.1_7 = k;
h_8 = &heads[k.1_7];
heads[2].first = 0B;
if (head.0_4 == h_8)
goto ;
else
goto ;
:
goto ;
:
p_10 = MEM[(struct head *)&heads][k.1_7].first;
:
# p_1 = PHI
_11 = p_1 != 0B;
_12 = (int) _11;
return _12;
before PR, the message is about the bb 5 sitting at critical edge removed.
The TBAA incompatible load it looks for is optimized away by FRE:
head->first = &node;
struct node *n = head->first;
struct head *h = &heads[k];
heads[2].first = n->next;
if ((void*)n->prev == (void *)h)
p = h->first;
else
/* Dead tbaa-unsafe load from ((struct node *)&heads[2])->next. */
p = n->prev->next;
here n is known to be head->first that is known to be &node.
The testcase runtime checks that result is Ok and passes.
Bootstrapped/regtested ppc64le-linux.
* alias.c (get_alias_set): Do not glob all pointer types into one;
just produce euqivalence classes based on canonical type of pointed
type type; make void * equivalent to void **.
(record_component_aliases): Make void * to conflict with all other
pointer types.
Index: alias.c
===
--- alias.c (revision 223633)
+++ alias.c (working copy)
@@ -903,35 +906,79 @@ get_alias_set (tree t)
the pointed-to types. This issue has been reported to the
C++ committee.
- In addition to the above canonicalization issue, with LTO
- we should also canonicalize `T (*)[]' to `T *' avoiding
- alias issues with pointer-to element types and pointer-to
- array types.
-
- Likewise we need to deal with the situation of incomplete
- pointed-to types and make `*(struct X **)&a' and
- `*(struct X {} **)&a' alias. Otherwise we will have to
- guarantee that all pointer-to incomplete type variants
- will be replaced by pointer-to complete type variants if
- they are available.
-
- With LTO the convenient situation of using `void *' to
- access and store any pointer type will also become
- more apparent (and `void *' is just another pointer-to
- incomplete type). Assigning alias-set z
