| Message ID | 20150302075343.GA67273@kam.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
On 03/02/2015 08:53 AM, Jan Hubicka wrote: > Hi, > this is a variant of patch I commited. > It takes care to load the constructor to memory in sem_variable::equals > and donot touch it earlier. I also made sem_variable::parse to skip volatile > and reigster variables. > > Bootstrapped/regtested x86_64-linux, lto-bootstrapped with -fmerge-all-constants > comitted. > > Honza Thank you Honza for patch improvement and new test cases you added. Martin > > 2015-02-28 Martin Liska <mliska@suse.cz> > Jan Hubicka <hubicka@ucw.cz> > > * ipa-icf.c (sem_variable::equals): Improve debug output; > get variable constructor. > (sem_variable::parse): Do not filter out too early; give up on > volatile and register vars. > (sem_item_optimizer::filter_removed_items): Filter out nonreadonly > variables. > * ipa-icf.h (sem_variable::init): Do not set ctor. > (sem_variable::ctor): Remove. > > gcc/testsuite/ChangeLog: > > 2015-02-28 Martin Liska <mliska@suse.cz> > Jan Hubicka <hubicka@ucw.cz> > > * gcc.dg/ipa/ipa-icf-35.c: New test. > * gcc.dg/ipa/ipa-icf-36.c: New test. > * gcc.dg/ipa/ipa-icf-37.c: New test. > Index: ipa-icf.c > =================================================================== > --- ipa-icf.c (revision 221096) > +++ ipa-icf.c (working copy) > @@ -1448,18 +1452,29 @@ sem_variable::equals_wpa (sem_item *item > > /* Returns true if the item equals to ITEM given as argument. */ > > +/* Returns true if the item equals to ITEM given as argument. */ > + > bool > sem_variable::equals (sem_item *item, > - hash_map <symtab_node *, sem_item *> & ARG_UNUSED (ignored_nodes)) > + hash_map <symtab_node *, sem_item *> &) > { > gcc_assert (item->type == VAR); > + bool ret; > > - sem_variable *v = static_cast<sem_variable *>(item); > - > - if (!ctor || !v->ctor) > - return return_false_with_msg ("ctor is missing for semantic variable"); > + if (DECL_INITIAL (decl) == error_mark_node && in_lto_p) > + dyn_cast <varpool_node *>(node)->get_constructor (); > + if (DECL_INITIAL (item->decl) == error_mark_node && in_lto_p) > + dyn_cast <varpool_node *>(item->node)->get_constructor (); > + > + ret = sem_variable::equals (DECL_INITIAL (decl), > + DECL_INITIAL (item->node->decl)); > + if (dump_file && (dump_flags & TDF_DETAILS)) > + fprintf (dump_file, > + "Equals called for vars:%s:%s (%u:%u) (%s:%s) with result: %s\n\n", > + name(), item->name (), node->order, item->node->order, asm_name (), > + item->asm_name (), ret ? "true" : "false"); > > - return sem_variable::equals (ctor, v->ctor); > + return ret; > } > > /* Compares trees T1 and T2 for semantic equality. */ > @@ -1653,24 +1668,7 @@ sem_variable::equals (tree t1, tree t2) > sem_variable * > sem_variable::parse (varpool_node *node, bitmap_obstack *stack) > { > - tree decl = node->decl; > - > - if (node->alias) > - return NULL; > - > - bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl); > - if (!readonly) > - return NULL; > - > - bool can_handle = DECL_VIRTUAL_P (decl) > - || flag_merge_constants >= 2 > - || (!TREE_ADDRESSABLE (decl) && !node->externally_visible); > - > - if (!can_handle || DECL_EXTERNAL (decl)) > - return NULL; > - > - tree ctor = ctor_for_folding (decl); > - if (!ctor) > + if (TREE_THIS_VOLATILE (node->decl) || DECL_HARD_REGISTER (node->decl)) > return NULL; > > sem_variable *v = new sem_variable (node, 0, stack); > @@ -1686,8 +1684,8 @@ hashval_t > sem_variable::get_hash (void) > { > if (hash) > - return hash; > > + return hash; > /* All WPA streamed in symbols should have their hashes computed at compile > time. At this point, the constructor may not be in memory at all. > DECL_INITIAL (decl) would be error_mark_node in that case. */ > @@ -2155,7 +2153,14 @@ sem_item_optimizer::filter_removed_items > if (!flag_ipa_icf_variables) > remove_item (item); > else > - filtered.safe_push (item); > + { > + /* Filter out non-readonly variables. */ > + tree decl = item->decl; > + if (TREE_READONLY (decl)) > + filtered.safe_push (item); > + else > + remove_item (item); > + } > } > } > > Index: ipa-icf.h > =================================================================== > --- ipa-icf.h (revision 221096) > +++ ipa-icf.h (working copy) > @@ -393,7 +393,6 @@ public: > inline virtual void init (void) > { > decl = get_node ()->decl; > - ctor = ctor_for_folding (decl); > } > > virtual hashval_t get_hash (void); > @@ -415,9 +414,6 @@ public: > /* Parser function that visits a varpool NODE. */ > static sem_variable *parse (varpool_node *node, bitmap_obstack *stack); > > - /* Variable constructor. */ > - tree ctor; > - > private: > /* Iterates though a constructor and identifies tree references > we are interested in semantic function equality. */ > Index: testsuite/gcc.dg/ipa/ipa-icf-35.c > =================================================================== > --- testsuite/gcc.dg/ipa/ipa-icf-35.c (revision 0) > +++ testsuite/gcc.dg/ipa/ipa-icf-35.c (revision 0) > @@ -0,0 +1,31 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-ipa-icf" } */ > + > +void f1() > +{ > +} > + > +void f2() > +{ > +} > + > +static void (*a)(void)=&f1; > +static void (*b)(void)=&f1; > +static void (*c)(void)=&f2; > +static void (*d)(void)=&f2; > + > +int main() > +{ > + a(); > + b(); > + c(); > + d(); > + > + return 0; > +} > + > +/* { dg-final { scan-ipa-dump "Equal symbols: 3" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Semantic equality hit:f2->f1" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Semantic equality hit:d->c" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Semantic equality hit:b->a" "icf" } } */ > +/* { dg-final { cleanup-ipa-dump "icf" } } */ > Index: testsuite/gcc.dg/ipa/ipa-icf-36.c > =================================================================== > --- testsuite/gcc.dg/ipa/ipa-icf-36.c (revision 0) > +++ testsuite/gcc.dg/ipa/ipa-icf-36.c (revision 0) > @@ -0,0 +1,36 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-ipa-icf -fmerge-all-constants" } */ > +static int a; > +static int b; > +static const int c = 2; > +static const int d = 2; > +static char * e = "test"; > +static char * f = "test"; > +static int g[3]={1,2,3}; > +static int h[3]={1,2,3}; > +static const int *i=&c; > +static const int *j=&c; > +static const int *k=&d; > +int t(int tt) > +{ > + switch (tt) > + { > + case 1: return a; > + case 2: return b; > + case 3: return c; > + case 4: return d; > + case 5: return e[1]; > + case 6: return f[1]; > + case 7: return g[1]; > + case 8: return h[1]; > + case 9: return i[0]; > + case 10: return j[0]; > + case 11: return k[0]; > + } > +} > +/* { dg-final { scan-ipa-dump "Equal symbols: 6" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Semantic equality hit:b->a" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Semantic equality hit:d->c" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Semantic equality hit:f->e" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Semantic equality hit:h->g" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Semantic equality hit:k->i" "icf" } } */ > Index: testsuite/gcc.dg/ipa/ipa-icf-37.c > =================================================================== > --- testsuite/gcc.dg/ipa/ipa-icf-37.c (revision 0) > +++ testsuite/gcc.dg/ipa/ipa-icf-37.c (revision 0) > @@ -0,0 +1,36 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-ipa-icf" } */ > +static int a; > +static int b; > +static const int c = 2; > +static const int d = 2; > +static char * e = "test"; > +static char * f = "test"; > +static int g[3]={1,2,3}; > +static int h[3]={1,2,3}; > +static const int *i=&c; > +static const int *j=&c; > +static const int *k=&d; > +int t(int tt) > +{ > + switch (tt) > + { > + case 1: return a; > + case 2: return b; > + case 3: return c; > + case 4: return d; > + case 5: return e[1]; > + case 6: return f[1]; > + case 7: return g[1]; > + case 8: return h[1]; > + case 9: return i[0]; > + case 10: return j[0]; > + case 11: return k[0]; > + } > +} > +/* { dg-final { scan-ipa-dump "Equal symbols: 5" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Semantic equality hit:b->a" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Semantic equality hit:d->c" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Semantic equality hit:f->e" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Semantic equality hit:h->g" "icf" } } */ > +/* { dg-final { scan-ipa-dump "Semantic equality hit:j->i" "icf" } } */ >
On Sun, Mar 1, 2015 at 11:53 PM, Jan Hubicka <hubicka@ucw.cz> wrote: > Hi, > this is a variant of patch I commited. > It takes care to load the constructor to memory in sem_variable::equals > and donot touch it earlier. I also made sem_variable::parse to skip volatile > and reigster variables. > > Bootstrapped/regtested x86_64-linux, lto-bootstrapped with -fmerge-all-constants > comitted. > > Honza > > 2015-02-28 Martin Liska <mliska@suse.cz> > Jan Hubicka <hubicka@ucw.cz> > > * ipa-icf.c (sem_variable::equals): Improve debug output; > get variable constructor. > (sem_variable::parse): Do not filter out too early; give up on > volatile and register vars. > (sem_item_optimizer::filter_removed_items): Filter out nonreadonly > variables. > * ipa-icf.h (sem_variable::init): Do not set ctor. > (sem_variable::ctor): Remove. > This caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65334
> > This caused: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65334 This is another alias issue exposed by ICF. Vectorizer attempts to increase an alignment of alias by modifying its DECL_ALIGN, but it doesnot really achieve it - when outputting the definition we use DECL_ALIGN of the ultimate_alias_target. Frankly, increasing alias target is not safe thing to do: alias may be static, while target interposable and we do not want code touching actual target to assume greater alignment. I suppose only way to get this right is to make varasm to walk all aliases and figure largest alignment assumed. I am workign on a patch. Honza > > -- > H.J.
Index: ipa-icf.c =================================================================== --- ipa-icf.c (revision 221096) +++ ipa-icf.c (working copy) @@ -1448,18 +1452,29 @@ sem_variable::equals_wpa (sem_item *item /* Returns true if the item equals to ITEM given as argument. */ +/* Returns true if the item equals to ITEM given as argument. */ + bool sem_variable::equals (sem_item *item, - hash_map <symtab_node *, sem_item *> & ARG_UNUSED (ignored_nodes)) + hash_map <symtab_node *, sem_item *> &) { gcc_assert (item->type == VAR); + bool ret; - sem_variable *v = static_cast<sem_variable *>(item); - - if (!ctor || !v->ctor) - return return_false_with_msg ("ctor is missing for semantic variable"); + if (DECL_INITIAL (decl) == error_mark_node && in_lto_p) + dyn_cast <varpool_node *>(node)->get_constructor (); + if (DECL_INITIAL (item->decl) == error_mark_node && in_lto_p) + dyn_cast <varpool_node *>(item->node)->get_constructor (); + + ret = sem_variable::equals (DECL_INITIAL (decl), + DECL_INITIAL (item->node->decl)); + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, + "Equals called for vars:%s:%s (%u:%u) (%s:%s) with result: %s\n\n", + name(), item->name (), node->order, item->node->order, asm_name (), + item->asm_name (), ret ? "true" : "false"); - return sem_variable::equals (ctor, v->ctor); + return ret; } /* Compares trees T1 and T2 for semantic equality. */ @@ -1653,24 +1668,7 @@ sem_variable::equals (tree t1, tree t2) sem_variable * sem_variable::parse (varpool_node *node, bitmap_obstack *stack) { - tree decl = node->decl; - - if (node->alias) - return NULL; - - bool readonly = TYPE_P (decl) ? TYPE_READONLY (decl) : TREE_READONLY (decl); - if (!readonly) - return NULL; - - bool can_handle = DECL_VIRTUAL_P (decl) - || flag_merge_constants >= 2 - || (!TREE_ADDRESSABLE (decl) && !node->externally_visible); - - if (!can_handle || DECL_EXTERNAL (decl)) - return NULL; - - tree ctor = ctor_for_folding (decl); - if (!ctor) + if (TREE_THIS_VOLATILE (node->decl) || DECL_HARD_REGISTER (node->decl)) return NULL; sem_variable *v = new sem_variable (node, 0, stack); @@ -1686,8 +1684,8 @@ hashval_t sem_variable::get_hash (void) { if (hash) - return hash; + return hash; /* All WPA streamed in symbols should have their hashes computed at compile time. At this point, the constructor may not be in memory at all. DECL_INITIAL (decl) would be error_mark_node in that case. */ @@ -2155,7 +2153,14 @@ sem_item_optimizer::filter_removed_items if (!flag_ipa_icf_variables) remove_item (item); else - filtered.safe_push (item); + { + /* Filter out non-readonly variables. */ + tree decl = item->decl; + if (TREE_READONLY (decl)) + filtered.safe_push (item); + else + remove_item (item); + } } } Index: ipa-icf.h =================================================================== --- ipa-icf.h (revision 221096) +++ ipa-icf.h (working copy) @@ -393,7 +393,6 @@ public: inline virtual void init (void) { decl = get_node ()->decl; - ctor = ctor_for_folding (decl); } virtual hashval_t get_hash (void); @@ -415,9 +414,6 @@ public: /* Parser function that visits a varpool NODE. */ static sem_variable *parse (varpool_node *node, bitmap_obstack *stack); - /* Variable constructor. */ - tree ctor; - private: /* Iterates though a constructor and identifies tree references we are interested in semantic function equality. */ Index: testsuite/gcc.dg/ipa/ipa-icf-35.c =================================================================== --- testsuite/gcc.dg/ipa/ipa-icf-35.c (revision 0) +++ testsuite/gcc.dg/ipa/ipa-icf-35.c (revision 0) @@ -0,0 +1,31 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-ipa-icf" } */ + +void f1() +{ +} + +void f2() +{ +} + +static void (*a)(void)=&f1; +static void (*b)(void)=&f1; +static void (*c)(void)=&f2; +static void (*d)(void)=&f2; + +int main() +{ + a(); + b(); + c(); + d(); + + return 0; +} + +/* { dg-final { scan-ipa-dump "Equal symbols: 3" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:f2->f1" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:d->c" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:b->a" "icf" } } */ +/* { dg-final { cleanup-ipa-dump "icf" } } */ Index: testsuite/gcc.dg/ipa/ipa-icf-36.c =================================================================== --- testsuite/gcc.dg/ipa/ipa-icf-36.c (revision 0) +++ testsuite/gcc.dg/ipa/ipa-icf-36.c (revision 0) @@ -0,0 +1,36 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-ipa-icf -fmerge-all-constants" } */ +static int a; +static int b; +static const int c = 2; +static const int d = 2; +static char * e = "test"; +static char * f = "test"; +static int g[3]={1,2,3}; +static int h[3]={1,2,3}; +static const int *i=&c; +static const int *j=&c; +static const int *k=&d; +int t(int tt) +{ + switch (tt) + { + case 1: return a; + case 2: return b; + case 3: return c; + case 4: return d; + case 5: return e[1]; + case 6: return f[1]; + case 7: return g[1]; + case 8: return h[1]; + case 9: return i[0]; + case 10: return j[0]; + case 11: return k[0]; + } +} +/* { dg-final { scan-ipa-dump "Equal symbols: 6" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:b->a" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:d->c" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:f->e" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:h->g" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:k->i" "icf" } } */ Index: testsuite/gcc.dg/ipa/ipa-icf-37.c =================================================================== --- testsuite/gcc.dg/ipa/ipa-icf-37.c (revision 0) +++ testsuite/gcc.dg/ipa/ipa-icf-37.c (revision 0) @@ -0,0 +1,36 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-ipa-icf" } */ +static int a; +static int b; +static const int c = 2; +static const int d = 2; +static char * e = "test"; +static char * f = "test"; +static int g[3]={1,2,3}; +static int h[3]={1,2,3}; +static const int *i=&c; +static const int *j=&c; +static const int *k=&d; +int t(int tt) +{ + switch (tt) + { + case 1: return a; + case 2: return b; + case 3: return c; + case 4: return d; + case 5: return e[1]; + case 6: return f[1]; + case 7: return g[1]; + case 8: return h[1]; + case 9: return i[0]; + case 10: return j[0]; + case 11: return k[0]; + } +} +/* { dg-final { scan-ipa-dump "Equal symbols: 5" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:b->a" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:d->c" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:f->e" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:h->g" "icf" } } */ +/* { dg-final { scan-ipa-dump "Semantic equality hit:j->i" "icf" } } */