Message ID | 57547ef8-eefd-8dba-b936-75b16a290d80@suse.cz |
---|---|
State | New |
Headers | show |
Series | Fix PTA info in IPA ICF (PR ipa/84658). | expand |
On Mon, Mar 12, 2018 at 9:42 AM, Martin Liška <mliska@suse.cz> wrote: > Hi. > > This is what I was recommended in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84658#c18. > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Ready to be installed? Please do not remove the DECL_PT_UID setting (set_alias_uids). That's required to make followup PTA runs compute correct points-to sets. Otherwise... + /* The above get's us to 99% I guess, at least catching the + address compares. Below also gets us aliasing correct + but as said we're giving leeway to the situation with + readonly vars anyway, so ... */ + basic_block bb; + FOR_EACH_BB_FN (bb, DECL_STRUCT_FUNCTION (cnode->decl)) + for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi); + gsi_next (&gsi)) so if you want to keep this (as the comment says I think it's not strictly necessary) then you can replace DECL_STRUCT_FUNCTION (cnode->decl) with 'fn' you compute earlier. Otherwise looks ok. Thanks, Richard. > Martin > > gcc/ChangeLog: > > 2018-03-12 Martin Liska <mliska@suse.cz> > > PR ipa/84658. > * ipa-icf.c (set_alias_uids): Remove. > (sem_variable::merge): Remove usage. > (sem_item_optimizer::sem_item_optimizer): Initialize new > vector. > (sem_item_optimizer::~sem_item_optimizer): Release it. > (sem_item_optimizer::merge_classes): Register variable aliases. > (sem_item_optimizer::fixup_pt_set): New function. > (sem_item_optimizer::fixup_points_to_sets): Likewise. > * ipa-icf.h: Declare new variables and functions. > > gcc/testsuite/ChangeLog: > > 2018-03-12 Martin Liska <mliska@suse.cz> > > PR ipa/84658. > * g++.dg/ipa/pr84658.C: New test. > --- > gcc/ipa-icf.c | 88 +++++++++++++++++++++++++++++--------- > gcc/ipa-icf.h | 10 +++++ > gcc/testsuite/g++.dg/ipa/pr84658.C | 30 +++++++++++++ > 3 files changed, 108 insertions(+), 20 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/ipa/pr84658.C > >
On 03/12/2018 11:06 AM, Richard Biener wrote: > On Mon, Mar 12, 2018 at 9:42 AM, Martin Liška <mliska@suse.cz> wrote: >> Hi. >> >> This is what I was recommended in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84658#c18. >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >> >> Ready to be installed? > > Please do not remove the DECL_PT_UID setting (set_alias_uids). That's required > to make followup PTA runs compute correct points-to sets. > > Otherwise... > > + /* The above get's us to 99% I guess, at least catching the > + address compares. Below also gets us aliasing correct > + but as said we're giving leeway to the situation with > + readonly vars anyway, so ... */ > + basic_block bb; > + FOR_EACH_BB_FN (bb, DECL_STRUCT_FUNCTION (cnode->decl)) > + for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi); > + gsi_next (&gsi)) > > so if you want to keep this (as the comment says I think it's not > strictly necessary) then you can replace DECL_STRUCT_FUNCTION (cnode->decl) > with 'fn' you compute earlier. > > Otherwise looks ok. Good, done that. I'm attaching final version of patch I'm going to install. Thanks for review, Martin > > Thanks, > Richard. > >> Martin >> >> gcc/ChangeLog: >> >> 2018-03-12 Martin Liska <mliska@suse.cz> >> >> PR ipa/84658. >> * ipa-icf.c (set_alias_uids): Remove. >> (sem_variable::merge): Remove usage. >> (sem_item_optimizer::sem_item_optimizer): Initialize new >> vector. >> (sem_item_optimizer::~sem_item_optimizer): Release it. >> (sem_item_optimizer::merge_classes): Register variable aliases. >> (sem_item_optimizer::fixup_pt_set): New function. >> (sem_item_optimizer::fixup_points_to_sets): Likewise. >> * ipa-icf.h: Declare new variables and functions. >> >> gcc/testsuite/ChangeLog: >> >> 2018-03-12 Martin Liska <mliska@suse.cz> >> >> PR ipa/84658. >> * g++.dg/ipa/pr84658.C: New test. >> --- >> gcc/ipa-icf.c | 88 +++++++++++++++++++++++++++++--------- >> gcc/ipa-icf.h | 10 +++++ >> gcc/testsuite/g++.dg/ipa/pr84658.C | 30 +++++++++++++ >> 3 files changed, 108 insertions(+), 20 deletions(-) >> create mode 100644 gcc/testsuite/g++.dg/ipa/pr84658.C >> >> From 27acff689225028a262eec09c213be38eb323470 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Fri, 9 Mar 2018 13:23:32 +0100 Subject: [PATCH] Fix PTA info in IPA ICF (PR ipa/84658). gcc/ChangeLog: 2018-03-12 Martin Liska <mliska@suse.cz> PR ipa/84658. * (sem_item_optimizer::sem_item_optimizer): Initialize new vector. (sem_item_optimizer::~sem_item_optimizer): Release it. (sem_item_optimizer::merge_classes): Register variable aliases. (sem_item_optimizer::fixup_pt_set): New function. (sem_item_optimizer::fixup_points_to_sets): Likewise. * ipa-icf.h: Declare new variables and functions. gcc/testsuite/ChangeLog: 2018-03-12 Martin Liska <mliska@suse.cz> PR ipa/84658. * g++.dg/ipa/pr84658.C: New test. --- gcc/ipa-icf.c | 112 ++++++++++++++++++++++++++++++------- gcc/ipa-icf.h | 12 ++++ gcc/testsuite/g++.dg/ipa/pr84658.C | 30 ++++++++++ 3 files changed, 134 insertions(+), 20 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ipa/pr84658.C diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c index b9f2bf30744..1376a54e95e 100644 --- a/gcc/ipa-icf.c +++ b/gcc/ipa-icf.c @@ -2133,23 +2133,6 @@ sem_variable::get_hash (void) return m_hash; } -/* Set all points-to UIDs of aliases pointing to node N as UID. */ - -static void -set_alias_uids (symtab_node *n, int uid) -{ - ipa_ref *ref; - FOR_EACH_ALIAS (n, ref) - { - if (dump_file) - fprintf (dump_file, " Setting points-to UID of [%s] as %d\n", - xstrdup_for_dump (ref->referring->asm_name ()), uid); - - SET_DECL_PT_UID (ref->referring->decl, uid); - set_alias_uids (ref->referring, uid); - } -} - /* Merges instance with an ALIAS_ITEM, where alias, thunk or redirection can be applied. */ @@ -2276,7 +2259,6 @@ sem_variable::merge (sem_item *alias_item) if (dump_file) fprintf (dump_file, "Unified; Variable alias has been created.\n"); - set_alias_uids (original, DECL_UID (original->decl)); return true; } } @@ -2296,7 +2278,7 @@ unsigned int sem_item_optimizer::class_id = 0; sem_item_optimizer::sem_item_optimizer () : worklist (0), m_classes (0), m_classes_count (0), m_cgraph_node_hooks (NULL), - m_varpool_node_hooks (NULL) + m_varpool_node_hooks (NULL), m_merged_variables () { m_items.create (0); bitmap_obstack_initialize (&m_bmstack); @@ -2321,6 +2303,7 @@ sem_item_optimizer::~sem_item_optimizer () m_items.release (); bitmap_obstack_release (&m_bmstack); + m_merged_variables.release (); } /* Write IPA ICF summary for symbols. */ @@ -3572,13 +3555,102 @@ sem_item_optimizer::merge_classes (unsigned int prev_class_count) } if (dbg_cnt (merged_ipa_icf)) - merged_p |= source->merge (alias); + { + bool merged = source->merge (alias); + merged_p |= merged; + + if (merged && alias->type == VAR) + { + symtab_pair p = symtab_pair (source->node, alias->node); + m_merged_variables.safe_push (p); + } + } } } + if (!m_merged_variables.is_empty ()) + fixup_points_to_sets (); + return merged_p; } +/* Fixup points to set PT. */ + +void +sem_item_optimizer::fixup_pt_set (struct pt_solution *pt) +{ + if (pt->vars == NULL) + return; + + unsigned i; + symtab_pair *item; + FOR_EACH_VEC_ELT (m_merged_variables, i, item) + if (bitmap_bit_p (pt->vars, DECL_UID (item->second->decl))) + bitmap_set_bit (pt->vars, DECL_UID (item->first->decl)); +} + +/* Set all points-to UIDs of aliases pointing to node N as UID. */ + +static void +set_alias_uids (symtab_node *n, int uid) +{ + ipa_ref *ref; + FOR_EACH_ALIAS (n, ref) + { + if (dump_file) + fprintf (dump_file, " Setting points-to UID of [%s] as %d\n", + xstrdup_for_dump (ref->referring->asm_name ()), uid); + + SET_DECL_PT_UID (ref->referring->decl, uid); + set_alias_uids (ref->referring, uid); + } +} + +/* Fixup points to analysis info. */ + +void +sem_item_optimizer::fixup_points_to_sets (void) +{ + /* TODO: remove in GCC 9 and trigger PTA re-creation after IPA passes. */ + + cgraph_node *cnode; + return; + + FOR_EACH_DEFINED_FUNCTION (cnode) + { + tree name; + unsigned i; + function *fn = DECL_STRUCT_FUNCTION (cnode->decl); + FOR_EACH_SSA_NAME (i, name, fn) + if (POINTER_TYPE_P (TREE_TYPE (name)) + && SSA_NAME_PTR_INFO (name)) + fixup_pt_set (&SSA_NAME_PTR_INFO (name)->pt); + fixup_pt_set (&fn->gimple_df->escaped); + + /* The above get's us to 99% I guess, at least catching the + address compares. Below also gets us aliasing correct + but as said we're giving leeway to the situation with + readonly vars anyway, so ... */ + basic_block bb; + FOR_EACH_BB_FN (bb, fn) + for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi); + gsi_next (&gsi)) + { + gcall *call = dyn_cast<gcall *> (gsi_stmt (gsi)); + if (call) + { + fixup_pt_set (gimple_call_use_set (call)); + fixup_pt_set (gimple_call_clobber_set (call)); + } + } + } + + unsigned i; + symtab_pair *item; + FOR_EACH_VEC_ELT (m_merged_variables, i, item) + set_alias_uids (item->first, DECL_UID (item->first->decl)); +} + /* Dump function prints all class members to a FILE with an INDENT. */ void diff --git a/gcc/ipa-icf.h b/gcc/ipa-icf.h index e3dcde2b4d1..622aebc00c0 100644 --- a/gcc/ipa-icf.h +++ b/gcc/ipa-icf.h @@ -141,6 +141,8 @@ public: unsigned int index; }; +typedef std::pair<symtab_node *, symtab_node *> symtab_pair; + /* Semantic item is a base class that encapsulates all shared functionality for both semantic function and variable items. */ class sem_item @@ -563,6 +565,12 @@ private: processed. */ bool merge_classes (unsigned int prev_class_count); + /* Fixup points to analysis info. */ + void fixup_points_to_sets (void); + + /* Fixup points to set PT. */ + void fixup_pt_set (struct pt_solution *pt); + /* Adds a newly created congruence class CLS to worklist. */ void worklist_push (congruence_class *cls); @@ -632,6 +640,10 @@ private: /* Bitmap stack. */ bitmap_obstack m_bmstack; + + /* Vector of merged variables. Needed for fixup of points-to-analysis + info. */ + vec <symtab_pair> m_merged_variables; }; // class sem_item_optimizer } // ipa_icf namespace diff --git a/gcc/testsuite/g++.dg/ipa/pr84658.C b/gcc/testsuite/g++.dg/ipa/pr84658.C new file mode 100644 index 00000000000..6846e1832bd --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr84658.C @@ -0,0 +1,30 @@ +/* PR ipa/84658 */ +/* { dg-do run } */ +/* { dg-options "-O2 -fmerge-all-constants -std=c++11" } */ + +const int kTestCasesFoo[] = { 0, 1, 2, 3, 4, 5, 8, 15, 16, 17, 512, 1020, 1021, 1022, 1023, 1024 }; +const int kTestCasesBar[] = { 0, 1, 2, 3, 4, 5, 8, 15, 16, 17, 512, 1020, 1021, 1022, 1023, 1024 }; + +void Foo() { + __builtin_printf("foo:"); + for (int count : kTestCasesFoo) { + __builtin_printf("%d,", count); + } + __builtin_printf(";\n"); +} + +void Bar() { + __builtin_printf("bar:"); + for (int count : kTestCasesBar) { + __builtin_printf("%d,", count); + } + __builtin_printf(";\n"); +} + +int main() { + Foo(); + Bar(); +} + +/* { dg-output "foo:0,1,2,3,4,5,8,15,16,17,512,1020,1021,1022,1023,1024,;(\n|\n\r|\r)*" } */ +/* { dg-output "bar:0,1,2,3,4,5,8,15,16,17,512,1020,1021,1022,1023,1024,;(\n|\n\r|\r)*" } */
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c index b9f2bf30744..4145e7e8440 100644 --- a/gcc/ipa-icf.c +++ b/gcc/ipa-icf.c @@ -2133,23 +2133,6 @@ sem_variable::get_hash (void) return m_hash; } -/* Set all points-to UIDs of aliases pointing to node N as UID. */ - -static void -set_alias_uids (symtab_node *n, int uid) -{ - ipa_ref *ref; - FOR_EACH_ALIAS (n, ref) - { - if (dump_file) - fprintf (dump_file, " Setting points-to UID of [%s] as %d\n", - xstrdup_for_dump (ref->referring->asm_name ()), uid); - - SET_DECL_PT_UID (ref->referring->decl, uid); - set_alias_uids (ref->referring, uid); - } -} - /* Merges instance with an ALIAS_ITEM, where alias, thunk or redirection can be applied. */ @@ -2276,7 +2259,6 @@ sem_variable::merge (sem_item *alias_item) if (dump_file) fprintf (dump_file, "Unified; Variable alias has been created.\n"); - set_alias_uids (original, DECL_UID (original->decl)); return true; } } @@ -2296,7 +2278,7 @@ unsigned int sem_item_optimizer::class_id = 0; sem_item_optimizer::sem_item_optimizer () : worklist (0), m_classes (0), m_classes_count (0), m_cgraph_node_hooks (NULL), - m_varpool_node_hooks (NULL) + m_varpool_node_hooks (NULL), m_merged_variables () { m_items.create (0); bitmap_obstack_initialize (&m_bmstack); @@ -2321,6 +2303,7 @@ sem_item_optimizer::~sem_item_optimizer () m_items.release (); bitmap_obstack_release (&m_bmstack); + m_merged_variables.release (); } /* Write IPA ICF summary for symbols. */ @@ -3572,13 +3555,78 @@ sem_item_optimizer::merge_classes (unsigned int prev_class_count) } if (dbg_cnt (merged_ipa_icf)) - merged_p |= source->merge (alias); + { + bool merged = source->merge (alias); + merged_p |= merged; + + if (merged && alias->type == VAR) + m_merged_variables.safe_push (std::pair<tree, tree> + (source->decl, alias->decl)); + } } } + if (!m_merged_variables.is_empty ()) + fixup_points_to_sets (); + return merged_p; } +/* Fixup points to set PT. */ + +void +sem_item_optimizer::fixup_pt_set (struct pt_solution *pt) +{ + if (pt->vars == NULL) + return; + + unsigned i; + std::pair<tree, tree> *item; + FOR_EACH_VEC_ELT (m_merged_variables, i, item) + if (bitmap_bit_p (pt->vars, DECL_UID (item->second))) + bitmap_set_bit (pt->vars, DECL_UID (item->first)); +} + +/* Fixup points to analysis info. */ + +void +sem_item_optimizer::fixup_points_to_sets (void) +{ + /* TODO: remove in GCC 9 and trigger PTA re-creation after IPA passes. */ + + cgraph_node *cnode; + return; + + FOR_EACH_DEFINED_FUNCTION (cnode) + { + tree name; + unsigned i; + function *fn = DECL_STRUCT_FUNCTION (cnode->decl); + FOR_EACH_SSA_NAME (i, name, fn) + if (POINTER_TYPE_P (TREE_TYPE (name)) + && SSA_NAME_PTR_INFO (name)) + fixup_pt_set (&SSA_NAME_PTR_INFO (name)->pt); + fixup_pt_set (&fn->gimple_df->escaped); + + /* The above get's us to 99% I guess, at least catching the + address compares. Below also gets us aliasing correct + but as said we're giving leeway to the situation with + readonly vars anyway, so ... */ + basic_block bb; + FOR_EACH_BB_FN (bb, DECL_STRUCT_FUNCTION (cnode->decl)) + for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi); + gsi_next (&gsi)) + { + gcall *call = dyn_cast<gcall *> (gsi_stmt (gsi)); + if (call) + { + fixup_pt_set (gimple_call_use_set (call)); + fixup_pt_set (gimple_call_clobber_set (call)); + } + } + } +} + /* Dump function prints all class members to a FILE with an INDENT. */ void diff --git a/gcc/ipa-icf.h b/gcc/ipa-icf.h index e3dcde2b4d1..9ac4f37eb1f 100644 --- a/gcc/ipa-icf.h +++ b/gcc/ipa-icf.h @@ -563,6 +563,12 @@ private: processed. */ bool merge_classes (unsigned int prev_class_count); + /* Fixup points to analysis info. */ + void fixup_points_to_sets (void); + + /* Fixup points to set PT. */ + void fixup_pt_set (struct pt_solution *pt); + /* Adds a newly created congruence class CLS to worklist. */ void worklist_push (congruence_class *cls); @@ -632,6 +638,10 @@ private: /* Bitmap stack. */ bitmap_obstack m_bmstack; + + /* Vector of merged variables. Needed for fixup of points-to-analysis + info. */ + vec <std::pair<tree, tree> > m_merged_variables; }; // class sem_item_optimizer } // ipa_icf namespace diff --git a/gcc/testsuite/g++.dg/ipa/pr84658.C b/gcc/testsuite/g++.dg/ipa/pr84658.C new file mode 100644 index 00000000000..6846e1832bd --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/pr84658.C @@ -0,0 +1,30 @@ +/* PR ipa/84658 */ +/* { dg-do run } */ +/* { dg-options "-O2 -fmerge-all-constants -std=c++11" } */ + +const int kTestCasesFoo[] = { 0, 1, 2, 3, 4, 5, 8, 15, 16, 17, 512, 1020, 1021, 1022, 1023, 1024 }; +const int kTestCasesBar[] = { 0, 1, 2, 3, 4, 5, 8, 15, 16, 17, 512, 1020, 1021, 1022, 1023, 1024 }; + +void Foo() { + __builtin_printf("foo:"); + for (int count : kTestCasesFoo) { + __builtin_printf("%d,", count); + } + __builtin_printf(";\n"); +} + +void Bar() { + __builtin_printf("bar:"); + for (int count : kTestCasesBar) { + __builtin_printf("%d,", count); + } + __builtin_printf(";\n"); +} + +int main() { + Foo(); + Bar(); +} + +/* { dg-output "foo:0,1,2,3,4,5,8,15,16,17,512,1020,1021,1022,1023,1024,;(\n|\n\r|\r)*" } */ +/* { dg-output "bar:0,1,2,3,4,5,8,15,16,17,512,1020,1021,1022,1023,1024,;(\n|\n\r|\r)*" } */