Re: Do less generous pointer globbing in alias.c

2015-06-10 Thread Martin Liška
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:
   bb 2:
   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 bb 3;
   else
 goto bb 5;
 
   bb 5:
   goto bb 4;
 
   bb 3:
   p_10 = MEM[(struct head *)heads][k.1_7].first;
 
   bb 4:
   # p_1 = PHI p_10(3), node(5)
   _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 

Re: Do less generous pointer globbing in alias.c

2015-05-31 Thread Andreas Schwab
Jan Hubicka hubi...@ucw.cz 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 
ipa_icf::sem_item_optimizer::parse_funcs_and_vars()+1312) 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 
ipa_icf::sem_item_optimizer::parse_funcs_and_vars()+1312) at 
../../gcc/bitmap.c:288
map = 0x8401880020420020
#1  0x4192bf10 in ipa_icf::sem_item::setup (this=0x20ab0350, 
stack=0x4194cf20 
ipa_icf::sem_item_optimizer::parse_funcs_and_vars()+1312) 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 
ipa_icf::sem_item_optimizer::parse_funcs_and_vars()+1312) at 
../../gcc/ipa-icf.c:1847
No locals.
#3  0x4194c690 in sem_function (stack=0x6008c4d8 dump_flags, 
hash=0, node=0x7fff, this=0x20966920)
at ../../gcc/ipa-icf.c:286
No locals.
#4  ipa_icf::sem_function::parse (node=0x7fff, 
stack=0x6008c4d8 dump_flags) at ../../gcc/ipa-icf.c:1701
fndecl = optimized out
func = optimized out
__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

2015-05-31 Thread Andreas Schwab
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 tree_check_failed(tree_node const*, char const*, int, 
char const*, ...);;
   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 
ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+368;;
   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 
ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+400
   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 
ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+208;;
   0x4194c540 +192:   [MIB]   nop.m 0x0
   0x4194c541 +193:   tbit.z p7,p6=r14,17
   0x4194c542 +194: (p06) br.cond.dptk.few 0x4194c5f0 
ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+368
   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 tree_contains_struct_check_failed(tree_node const*, 
tree_node_structure_enum, char const*, int, char const*);;
   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 
ipa_icf::sem_function::parse(cgraph_node*, bitmap_obstack*)+464;;
   0x4194c5d0 +336:   [MIB]   ld8 r38=[r38]
   0x4194c5d1 +337:   nop.i 0x0
   0x4194c5d2 +338:   br.call.sptk.many 
b0=0x41534f40 private_lookup_attribute_by_prefix(char const*, unsigned 
long, tree_node*);;
   0x4194c5e0 +352:   [MIB]   mov r1=r37
   0x4194c5e1 +353:   cmp.eq p6,p7=0,r8
   0x4194c5e2 +354: (p06) br.cond.spnt.few 0x4194c650 

Re: Do less generous pointer globbing in alias.c

2015-05-31 Thread Jan Hubicka
 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

2015-05-30 Thread Andreas Schwab
Jan Hubicka hubi...@ucw.cz 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

2015-05-30 Thread Jan Hubicka
 Jan Hubicka hubi...@ucw.cz 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

2015-05-30 Thread H.J. Lu
On Thu, May 28, 2015 at 1:12 PM, Jan Hubicka hubi...@ucw.cz 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  hongjiu...@intel.com
+
+ * gcc.dg/alias-8.c: Fix dg-warning.
+
 2015-05-30  Jan Hubicka  hubi...@ucw.cz

  * 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

2015-05-28 Thread Jan Hubicka
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_mapint, int, alias_set_traits *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 

Re: Do less generous pointer globbing in alias.c

2015-05-28 Thread Jan Hubicka
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_mapint, int, alias_set_traits *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_allocalias_set_entry_d ();
+  ase-alias_set = set;
+  ase-children
+= hash_mapint, int, alias_set_traits::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 

Re: Do less generous pointer globbing in alias.c

2015-05-28 Thread Richard Biener
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_mapint, int, alias_set_traits *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_allocalias_set_entry_d ();

no need to use cleared_alloc if you also init -is_pointer to false.

 +  ase-alias_set = set;
 +  ase-children
 += hash_mapint, int, alias_set_traits::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-set zero to `void *'
 - and all pointer-to incomplete types is a not 

Re: Do less generous pointer globbing in alias.c

2015-05-28 Thread Jan Hubicka
  +alias_set_entry
  +init_alias_set_entry (alias_set_type set)
  +{
  +  alias_set_entry ase = ggc_cleared_allocalias_set_entry_d ();
 
 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_mapint, int, alias_set_traits::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_allocalias_set_entry_d ();
  -  superset_entry-alias_set = superset;
  -  superset_entry-children
  -   = hash_mapint, int, alias_set_traits::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 rguent...@suse.de
 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

2015-05-27 Thread Richard Biener
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:
   bb 2:
   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 bb 3;
   else
 goto bb 5;
 
   bb 5:
   goto bb 4;
 
   bb 3:
   p_10 = MEM[(struct head *)heads][k.1_7].first;
 
   bb 4:
   # p_1 = PHI p_10(3), node(5)
   _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: 

Re: Do less generous pointer globbing in alias.c

2015-05-27 Thread Jan Hubicka
 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

2015-05-27 Thread Jan Hubicka
 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

2015-05-27 Thread Jan Hubicka
  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

2015-05-27 Thread Richard Biener
On May 27, 2015 5:04:13 PM GMT+02:00, Jan Hubicka hubi...@ucw.cz 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

2015-05-27 Thread Jan Hubicka
  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

2015-05-27 Thread Jan Hubicka
 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

2015-05-26 Thread Jan Hubicka
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:
  bb 2:
  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 bb 3;
  else
goto bb 5;

  bb 5:
  goto bb 4;

  bb 3:
  p_10 = MEM[(struct head *)heads][k.1_7].first;

  bb 4:
  # p_1 = PHI p_10(3), node(5)
  _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