| Message ID | ri6leh79kk9.fsf@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] ipa-cp: Avoid long linear searches through DECL_ARGUMENTS | expand |
On Mon, 29 May 2023, Martin Jambor wrote: > Hi, > > PRs 68930 and 92497 show that when IPA-CP figures out constants in > aggregate parameters or when passed by reference but the loads happen > in an inlined function the information is lost. This happens even > when the inlined function itself was known to have - or even cloned to > have - such constants in incoming parameters because the transform > phase of IPA passes is not run on them. See discussion in the bugs > for reasons why. > > Honza suggested that we can plug the results of IPA-CP analysis into > value numbering, so that FRE can figure out that some loads fetch > known constants. This is what this patch does. > > This version of the patch uses the new way we represent aggregate > constants discovered IPA-CP and so avoids linear scan to find them. > Similarly, it depends on the previous patch which avoids potentially > slow linear look ups of indices of PARM_DECLs when there are many of > them. > > Bootstrapped, LTO-bootstrapped and LTO-profiledbootstrapped and tested > on x86_64-linux. OK for trunk? > > Thanks, > > Martin > > > gcc/ChangeLog: > > 2023-05-26 Martin Jambor <mjambor@suse.cz> > > PR ipa/68930 > PR ipa/92497 > * ipa-prop.h (ipcp_get_aggregate_const): Declare. > * ipa-prop.cc (ipcp_get_aggregate_const): New function. > (ipcp_transform_function): Do not deallocate transformation info. > * tree-ssa-sccvn.cc: Include alloc-pool.h, symbol-summary.h and > ipa-prop.h. > (vn_reference_lookup_2): When hitting default-def vuse, query > IPA-CP transformation info for any known constants. > > gcc/testsuite/ChangeLog: > > 2022-09-05 Martin Jambor <mjambor@suse.cz> > > PR ipa/68930 > PR ipa/92497 > * gcc.dg/ipa/pr92497-1.c: New test. > * gcc.dg/ipa/pr92497-2.c: Likewise. > --- > gcc/ipa-prop.cc | 33 +++++++++++++++++++++---- > gcc/ipa-prop.h | 3 +++ > gcc/testsuite/gcc.dg/ipa/pr92497-1.c | 26 ++++++++++++++++++++ > gcc/testsuite/gcc.dg/ipa/pr92497-2.c | 26 ++++++++++++++++++++ > gcc/tree-ssa-sccvn.cc | 36 +++++++++++++++++++++++++++- > 5 files changed, 118 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/ipa/pr92497-1.c > create mode 100644 gcc/testsuite/gcc.dg/ipa/pr92497-2.c > > diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc > index f0976e363f7..fb2c0c0466b 100644 > --- a/gcc/ipa-prop.cc > +++ b/gcc/ipa-prop.cc > @@ -5765,6 +5765,34 @@ ipcp_modif_dom_walker::before_dom_children (basic_block bb) > return NULL; > } > > +/* If IPA-CP discovered a constant in parameter PARM at OFFSET of a given SIZE > + - whether passed by reference or not is given by BY_REF - return that > + constant. Otherwise return NULL_TREE. */ > + > +tree > +ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, > + HOST_WIDE_INT bit_offset, HOST_WIDE_INT bit_size) > +{ > + cgraph_node *node = cgraph_node::get (func->decl); > + ipcp_transformation *ts = ipcp_get_transformation_summary (node); > + > + if (!ts || !ts->m_agg_values) > + return NULL_TREE; > + > + int index = ts->get_param_index (func->decl, parm); > + if (index < 0) > + return NULL_TREE; > + > + ipa_argagg_value_list avl (ts); > + unsigned unit_offset = bit_offset / BITS_PER_UNIT; > + tree v = avl.get_value (index, unit_offset, by_ref); > + if (!v > + || maybe_ne (tree_to_poly_int64 (TYPE_SIZE (TREE_TYPE (v))), bit_size)) > + return NULL_TREE; > + > + return v; > +} > + > /* Return true if we have recorded VALUE and MASK about PARM. > Set VALUE and MASk accordingly. */ > > @@ -6037,11 +6065,6 @@ ipcp_transform_function (struct cgraph_node *node) > free_ipa_bb_info (bi); > fbi.bb_infos.release (); > > - ipcp_transformation *s = ipcp_transformation_sum->get (node); > - s->m_agg_values = NULL; > - s->bits = NULL; > - s->m_vr = NULL; > - > vec_free (descriptors); > if (cfg_changed) > delete_unreachable_blocks_update_callgraph (node, false); > diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h > index 211b12ff6b3..f68fa4a12dd 100644 > --- a/gcc/ipa-prop.h > +++ b/gcc/ipa-prop.h > @@ -1221,6 +1221,9 @@ void ipa_dump_param (FILE *, class ipa_node_params *info, int i); > void ipa_release_body_info (struct ipa_func_body_info *); > tree ipa_get_callee_param_type (struct cgraph_edge *e, int i); > bool ipcp_get_parm_bits (tree, tree *, widest_int *); > +tree ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, > + HOST_WIDE_INT bit_offset, > + HOST_WIDE_INT bit_size); > bool unadjusted_ptr_and_unit_offset (tree op, tree *ret, > poly_int64 *offset_ret); > > diff --git a/gcc/testsuite/gcc.dg/ipa/pr92497-1.c b/gcc/testsuite/gcc.dg/ipa/pr92497-1.c > new file mode 100644 > index 00000000000..eb8f2e75fd0 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/ipa/pr92497-1.c > @@ -0,0 +1,26 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2 -fno-early-inlining" } */ > + > +struct a {int a;}; > +static int //__attribute__ ((noinline)) > +foo (struct a a) > +{ > + if (!__builtin_constant_p (a.a)) > + __builtin_abort (); > + return a.a; > +} > + > +static int __attribute__ ((noinline)) > +bar (struct a a) > +{ > + return foo(a); > +} > + > +volatile int r; > + > +int main() > +{ > + struct a a={1}; > + r = bar (a); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/ipa/pr92497-2.c b/gcc/testsuite/gcc.dg/ipa/pr92497-2.c > new file mode 100644 > index 00000000000..56e611df8f9 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/ipa/pr92497-2.c > @@ -0,0 +1,26 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2 -fno-early-inlining -fno-ipa-sra" } */ > + > +struct a {int a;}; > +static int //__attribute__ ((noinline)) > +foo (struct a *a) > +{ > + if (!__builtin_constant_p (a->a)) > + __builtin_abort (); > + return a->a; > +} > + > +static int __attribute__ ((noinline)) > +bar (struct a *a) > +{ > + return foo(a); > +} > + > +volatile int r; > + > +int main() > +{ > + struct a a={1}; > + r = bar (&a); > + return 0; > +} > diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc > index 27c84e78fcf..33215b5fc82 100644 > --- a/gcc/tree-ssa-sccvn.cc > +++ b/gcc/tree-ssa-sccvn.cc > @@ -74,6 +74,9 @@ along with GCC; see the file COPYING3. If not see > #include "ipa-modref-tree.h" > #include "ipa-modref.h" > #include "tree-ssa-sccvn.h" > +#include "alloc-pool.h" > +#include "symbol-summary.h" > +#include "ipa-prop.h" > > /* This algorithm is based on the SCC algorithm presented by Keith > Cooper and L. Taylor Simpson in "SCC-Based Value numbering" > @@ -2327,7 +2330,7 @@ vn_walk_cb_data::push_partial_def (pd_data pd, > with the current VUSE and performs the expression lookup. */ > > static void * > -vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) > +vn_reference_lookup_2 (ao_ref *op, tree vuse, void *data_) > { > vn_walk_cb_data *data = (vn_walk_cb_data *)data_; > vn_reference_t vr = data->vr; > @@ -2361,6 +2364,37 @@ vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) > return *slot; > } > > + if (SSA_NAME_IS_DEFAULT_DEF (vuse)) > + { > + HOST_WIDE_INT offset, size; > + tree v = NULL_TREE; > + if (op->base && TREE_CODE (op->base) == PARM_DECL > + && op->offset.is_constant (&offset) > + && op->size.is_constant (&size) > + && op->max_size_known_p () > + && known_eq (op->size, op->max_size)) > + v = ipcp_get_aggregate_const (cfun, op->base, false, offset, size); We've talked about partial definition support, this does not have this implemented AFAICS. But that means you cannot simply do ->finish () without verifying data->partial_defs.is_empty (). > + else if (op->ref) > + { does this ever happen to imrpove things? There's the remote possibility op->base isn't initialized yet, for this reason above you should use ao_ref_base (op) instead of accessing op->base directly. > + HOST_WIDE_INT offset, size; > + bool reverse; > + tree base = get_ref_base_and_extent_hwi (op->ref, &offset, > + &size, &reverse); > + if (base > + && TREE_CODE (base) == MEM_REF > + && integer_zerop (TREE_OPERAND (base, 1)) > + && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME And this then should be done within the above branch as well, just keyed off base == MEM_REF. Richard. > + && SSA_NAME_IS_DEFAULT_DEF (TREE_OPERAND (base, 0)) > + && (TREE_CODE (SSA_NAME_VAR (TREE_OPERAND (base, 0))) > + == PARM_DECL)) > + v = ipcp_get_aggregate_const (cfun, > + SSA_NAME_VAR (TREE_OPERAND (base, 0)), > + true, offset, size); > + } > + if (v) > + return data->finish (vr->set, vr->base_set, v); > + } > + > return NULL; > } > >
Hi, thanks for looking at this. On Fri, Jun 02 2023, Richard Biener wrote: > On Mon, 29 May 2023, Martin Jambor wrote: > [...] >> diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc >> index 27c84e78fcf..33215b5fc82 100644 >> --- a/gcc/tree-ssa-sccvn.cc >> +++ b/gcc/tree-ssa-sccvn.cc >> @@ -74,6 +74,9 @@ along with GCC; see the file COPYING3. If not see >> #include "ipa-modref-tree.h" >> #include "ipa-modref.h" >> #include "tree-ssa-sccvn.h" >> +#include "alloc-pool.h" >> +#include "symbol-summary.h" >> +#include "ipa-prop.h" >> >> /* This algorithm is based on the SCC algorithm presented by Keith >> Cooper and L. Taylor Simpson in "SCC-Based Value numbering" >> @@ -2327,7 +2330,7 @@ vn_walk_cb_data::push_partial_def (pd_data pd, >> with the current VUSE and performs the expression lookup. */ >> >> static void * >> -vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) >> +vn_reference_lookup_2 (ao_ref *op, tree vuse, void *data_) >> { >> vn_walk_cb_data *data = (vn_walk_cb_data *)data_; >> vn_reference_t vr = data->vr; >> @@ -2361,6 +2364,37 @@ vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) >> return *slot; >> } >> >> + if (SSA_NAME_IS_DEFAULT_DEF (vuse)) >> + { >> + HOST_WIDE_INT offset, size; >> + tree v = NULL_TREE; >> + if (op->base && TREE_CODE (op->base) == PARM_DECL >> + && op->offset.is_constant (&offset) >> + && op->size.is_constant (&size) >> + && op->max_size_known_p () >> + && known_eq (op->size, op->max_size)) >> + v = ipcp_get_aggregate_const (cfun, op->base, false, offset, size); > > We've talked about partial definition support, this does not > have this implemented AFAICS. But that means you cannot simply > do ->finish () without verifying data->partial_defs.is_empty (). > You are right, partial definitions are not implemented. I have added the is_empty check to the patch. I'll continue looking into adding the support as a follow-up. >> + else if (op->ref) >> + { > > does this ever happen to imrpove things? Yes, this branch is necessary for propagation of all known constants passed in memory pointed to by a POINTER_TYPE_P parameter. It handles the second testcase added by the patch. > There's the remote > possibility op->base isn't initialized yet, for this reason > above you should use ao_ref_base (op) instead of accessing > op->base directly. OK > >> + HOST_WIDE_INT offset, size; >> + bool reverse; >> + tree base = get_ref_base_and_extent_hwi (op->ref, &offset, >> + &size, &reverse); >> + if (base >> + && TREE_CODE (base) == MEM_REF >> + && integer_zerop (TREE_OPERAND (base, 1)) >> + && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME > > And this then should be done within the above branch as well, > just keyed off base == MEM_REF. I am sorry but I don't understand this comment, can you please try to re-phrase it? The previous branch handles direct accesses to PARM_DECLs, MEM_REFs don't need to be there at all. Updated (bootstrap and testing passing) patch is below for reference, but I obviously expect to incorporate the above comment as well before proposing to push it. Thanks, Martin Subject: [PATCH 2/2] ipa-cp: Feed results of IPA-CP into value numbering PRs 68930 and 92497 show that when IPA-CP figures out constants in aggregate parameters or when passed by reference but the loads happen in an inlined function the information is lost. This happens even when the inlined function itself was known to have - or even cloned to have - such constants in incoming parameters because the transform phase of IPA passes is not run on them. See discussion in the bugs for reasons why. Honza suggested that we can plug the results of IPA-CP analysis into value numbering, so that FRE can figure out that some loads fetch known constants. This is what this patch attempts to do. This version of the patch uses the new way we represent aggregate constants discovered IPA-CP and so avoids linear scan to find them. Similarly, it depends on the previous patch which avoids potentially slow linear look ups of indices of PARM_DECLs when there are many of them. gcc/ChangeLog: 2023-06-07 Martin Jambor <mjambor@suse.cz> PR ipa/68930 PR ipa/92497 * ipa-prop.h (ipcp_get_aggregate_const): Declare. * ipa-prop.cc (ipcp_get_aggregate_const): New function. (ipcp_transform_function): Do not deallocate transformation info. * tree-ssa-sccvn.cc: Include alloc-pool.h, symbol-summary.h and ipa-prop.h. (vn_reference_lookup_2): When hitting default-def vuse, query IPA-CP transformation info for any known constants. gcc/testsuite/ChangeLog: 2023-06-07 Martin Jambor <mjambor@suse.cz> PR ipa/68930 PR ipa/92497 * gcc.dg/ipa/pr92497-1.c: New test. * gcc.dg/ipa/pr92497-2.c: Likewise. --- gcc/ipa-prop.cc | 33 +++++++++++++++++++++---- gcc/ipa-prop.h | 3 +++ gcc/testsuite/gcc.dg/ipa/pr92497-1.c | 26 +++++++++++++++++++ gcc/testsuite/gcc.dg/ipa/pr92497-2.c | 26 +++++++++++++++++++ gcc/tree-ssa-sccvn.cc | 37 +++++++++++++++++++++++++++- 5 files changed, 119 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/ipa/pr92497-1.c create mode 100644 gcc/testsuite/gcc.dg/ipa/pr92497-2.c diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc index f0976e363f7..fb2c0c0466b 100644 --- a/gcc/ipa-prop.cc +++ b/gcc/ipa-prop.cc @@ -5765,6 +5765,34 @@ ipcp_modif_dom_walker::before_dom_children (basic_block bb) return NULL; } +/* If IPA-CP discovered a constant in parameter PARM at OFFSET of a given SIZE + - whether passed by reference or not is given by BY_REF - return that + constant. Otherwise return NULL_TREE. */ + +tree +ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, + HOST_WIDE_INT bit_offset, HOST_WIDE_INT bit_size) +{ + cgraph_node *node = cgraph_node::get (func->decl); + ipcp_transformation *ts = ipcp_get_transformation_summary (node); + + if (!ts || !ts->m_agg_values) + return NULL_TREE; + + int index = ts->get_param_index (func->decl, parm); + if (index < 0) + return NULL_TREE; + + ipa_argagg_value_list avl (ts); + unsigned unit_offset = bit_offset / BITS_PER_UNIT; + tree v = avl.get_value (index, unit_offset, by_ref); + if (!v + || maybe_ne (tree_to_poly_int64 (TYPE_SIZE (TREE_TYPE (v))), bit_size)) + return NULL_TREE; + + return v; +} + /* Return true if we have recorded VALUE and MASK about PARM. Set VALUE and MASk accordingly. */ @@ -6037,11 +6065,6 @@ ipcp_transform_function (struct cgraph_node *node) free_ipa_bb_info (bi); fbi.bb_infos.release (); - ipcp_transformation *s = ipcp_transformation_sum->get (node); - s->m_agg_values = NULL; - s->bits = NULL; - s->m_vr = NULL; - vec_free (descriptors); if (cfg_changed) delete_unreachable_blocks_update_callgraph (node, false); diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index c570e8ec25a..9f25b44c71b 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -1232,6 +1232,9 @@ void ipa_dump_param (FILE *, class ipa_node_params *info, int i); void ipa_release_body_info (struct ipa_func_body_info *); tree ipa_get_callee_param_type (struct cgraph_edge *e, int i); bool ipcp_get_parm_bits (tree, tree *, widest_int *); +tree ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, + HOST_WIDE_INT bit_offset, + HOST_WIDE_INT bit_size); bool unadjusted_ptr_and_unit_offset (tree op, tree *ret, poly_int64 *offset_ret); diff --git a/gcc/testsuite/gcc.dg/ipa/pr92497-1.c b/gcc/testsuite/gcc.dg/ipa/pr92497-1.c new file mode 100644 index 00000000000..dcece15963c --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/pr92497-1.c @@ -0,0 +1,26 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fno-early-inlining" } */ + +struct a {int a;}; +static int +foo (struct a a) +{ + if (!__builtin_constant_p (a.a)) + __builtin_abort (); + return a.a; +} + +static int __attribute__ ((noinline)) +bar (struct a a) +{ + return foo(a); +} + +volatile int r; + +int main() +{ + struct a a={1}; + r = bar (a); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/ipa/pr92497-2.c b/gcc/testsuite/gcc.dg/ipa/pr92497-2.c new file mode 100644 index 00000000000..c64090d1a7a --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/pr92497-2.c @@ -0,0 +1,26 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fno-early-inlining -fno-ipa-sra" } */ + +struct a {int a;}; +static int +foo (struct a *a) +{ + if (!__builtin_constant_p (a->a)) + __builtin_abort (); + return a->a; +} + +static int __attribute__ ((noinline)) +bar (struct a *a) +{ + return foo(a); +} + +volatile int r; + +int main() +{ + struct a a={1}; + r = bar (&a); + return 0; +} diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc index 27c84e78fcf..ce88b9bc95f 100644 --- a/gcc/tree-ssa-sccvn.cc +++ b/gcc/tree-ssa-sccvn.cc @@ -74,6 +74,9 @@ along with GCC; see the file COPYING3. If not see #include "ipa-modref-tree.h" #include "ipa-modref.h" #include "tree-ssa-sccvn.h" +#include "alloc-pool.h" +#include "symbol-summary.h" +#include "ipa-prop.h" /* This algorithm is based on the SCC algorithm presented by Keith Cooper and L. Taylor Simpson in "SCC-Based Value numbering" @@ -2327,7 +2330,7 @@ vn_walk_cb_data::push_partial_def (pd_data pd, with the current VUSE and performs the expression lookup. */ static void * -vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) +vn_reference_lookup_2 (ao_ref *op, tree vuse, void *data_) { vn_walk_cb_data *data = (vn_walk_cb_data *)data_; vn_reference_t vr = data->vr; @@ -2361,6 +2364,38 @@ vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) return *slot; } + if (SSA_NAME_IS_DEFAULT_DEF (vuse)) + { + HOST_WIDE_INT offset, size; + tree v = NULL_TREE; + if (ao_ref_base (op) && TREE_CODE (ao_ref_base (op)) == PARM_DECL + && op->offset.is_constant (&offset) + && op->size.is_constant (&size) + && op->max_size_known_p () + && known_eq (op->size, op->max_size)) + v = ipcp_get_aggregate_const (cfun, ao_ref_base (op), false, offset, + size); + else if (op->ref) + { + HOST_WIDE_INT offset, size; + bool reverse; + tree base = get_ref_base_and_extent_hwi (op->ref, &offset, + &size, &reverse); + if (base + && TREE_CODE (base) == MEM_REF + && integer_zerop (TREE_OPERAND (base, 1)) + && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME + && SSA_NAME_IS_DEFAULT_DEF (TREE_OPERAND (base, 0)) + && (TREE_CODE (SSA_NAME_VAR (TREE_OPERAND (base, 0))) + == PARM_DECL)) + v = ipcp_get_aggregate_const (cfun, + SSA_NAME_VAR (TREE_OPERAND (base, 0)), + true, offset, size); + } + if (v && data->partial_defs.is_empty ()) + return data->finish (vr->set, vr->base_set, v); + } + return NULL; }
On Fri, 9 Jun 2023, Martin Jambor wrote: > Hi, > > thanks for looking at this. > > On Fri, Jun 02 2023, Richard Biener wrote: > > On Mon, 29 May 2023, Martin Jambor wrote: > > > > [...] > > >> diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc > >> index 27c84e78fcf..33215b5fc82 100644 > >> --- a/gcc/tree-ssa-sccvn.cc > >> +++ b/gcc/tree-ssa-sccvn.cc > >> @@ -74,6 +74,9 @@ along with GCC; see the file COPYING3. If not see > >> #include "ipa-modref-tree.h" > >> #include "ipa-modref.h" > >> #include "tree-ssa-sccvn.h" > >> +#include "alloc-pool.h" > >> +#include "symbol-summary.h" > >> +#include "ipa-prop.h" > >> > >> /* This algorithm is based on the SCC algorithm presented by Keith > >> Cooper and L. Taylor Simpson in "SCC-Based Value numbering" > >> @@ -2327,7 +2330,7 @@ vn_walk_cb_data::push_partial_def (pd_data pd, > >> with the current VUSE and performs the expression lookup. */ > >> > >> static void * > >> -vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) > >> +vn_reference_lookup_2 (ao_ref *op, tree vuse, void *data_) > >> { > >> vn_walk_cb_data *data = (vn_walk_cb_data *)data_; > >> vn_reference_t vr = data->vr; > >> @@ -2361,6 +2364,37 @@ vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) > >> return *slot; > >> } > >> > >> + if (SSA_NAME_IS_DEFAULT_DEF (vuse)) > >> + { > >> + HOST_WIDE_INT offset, size; > >> + tree v = NULL_TREE; > >> + if (op->base && TREE_CODE (op->base) == PARM_DECL > >> + && op->offset.is_constant (&offset) > >> + && op->size.is_constant (&size) > >> + && op->max_size_known_p () > >> + && known_eq (op->size, op->max_size)) > >> + v = ipcp_get_aggregate_const (cfun, op->base, false, offset, size); > > > > We've talked about partial definition support, this does not > > have this implemented AFAICS. But that means you cannot simply > > do ->finish () without verifying data->partial_defs.is_empty (). > > > > You are right, partial definitions are not implemented. I have added > the is_empty check to the patch. I'll continue looking into adding the > support as a follow-up. > > >> + else if (op->ref) > >> + { > > > > does this ever happen to imrpove things? > > Yes, this branch is necessary for propagation of all known constants > passed in memory pointed to by a POINTER_TYPE_P parameter. It handles > the second testcase added by the patch. > > > There's the remote > > possibility op->base isn't initialized yet, for this reason > > above you should use ao_ref_base (op) instead of accessing > > op->base directly. > > OK > > > > >> + HOST_WIDE_INT offset, size; > >> + bool reverse; > >> + tree base = get_ref_base_and_extent_hwi (op->ref, &offset, > >> + &size, &reverse); > >> + if (base > >> + && TREE_CODE (base) == MEM_REF > >> + && integer_zerop (TREE_OPERAND (base, 1)) > >> + && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME > > > > And this then should be done within the above branch as well, > > just keyed off base == MEM_REF. > > I am sorry but I don't understand this comment, can you please try to > re-phrase it? The previous branch handles direct accesses to > PARM_DECLs, MEM_REFs don't need to be there at all. See below > Updated (bootstrap and testing passing) patch is below for reference, > but I obviously expect to incorporate the above comment as well before > proposing to push it. > > Thanks, > > Martin > > > Subject: [PATCH 2/2] ipa-cp: Feed results of IPA-CP into value numbering > > PRs 68930 and 92497 show that when IPA-CP figures out constants in > aggregate parameters or when passed by reference but the loads happen > in an inlined function the information is lost. This happens even > when the inlined function itself was known to have - or even cloned to > have - such constants in incoming parameters because the transform > phase of IPA passes is not run on them. See discussion in the bugs > for reasons why. > > Honza suggested that we can plug the results of IPA-CP analysis into > value numbering, so that FRE can figure out that some loads fetch > known constants. This is what this patch attempts to do. > > This version of the patch uses the new way we represent aggregate > constants discovered IPA-CP and so avoids linear scan to find them. > Similarly, it depends on the previous patch which avoids potentially > slow linear look ups of indices of PARM_DECLs when there are many of > them. > > gcc/ChangeLog: > > 2023-06-07 Martin Jambor <mjambor@suse.cz> > > PR ipa/68930 > PR ipa/92497 > * ipa-prop.h (ipcp_get_aggregate_const): Declare. > * ipa-prop.cc (ipcp_get_aggregate_const): New function. > (ipcp_transform_function): Do not deallocate transformation info. > * tree-ssa-sccvn.cc: Include alloc-pool.h, symbol-summary.h and > ipa-prop.h. > (vn_reference_lookup_2): When hitting default-def vuse, query > IPA-CP transformation info for any known constants. > > gcc/testsuite/ChangeLog: > > 2023-06-07 Martin Jambor <mjambor@suse.cz> > > PR ipa/68930 > PR ipa/92497 > * gcc.dg/ipa/pr92497-1.c: New test. > * gcc.dg/ipa/pr92497-2.c: Likewise. > --- > gcc/ipa-prop.cc | 33 +++++++++++++++++++++---- > gcc/ipa-prop.h | 3 +++ > gcc/testsuite/gcc.dg/ipa/pr92497-1.c | 26 +++++++++++++++++++ > gcc/testsuite/gcc.dg/ipa/pr92497-2.c | 26 +++++++++++++++++++ > gcc/tree-ssa-sccvn.cc | 37 +++++++++++++++++++++++++++- > 5 files changed, 119 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/ipa/pr92497-1.c > create mode 100644 gcc/testsuite/gcc.dg/ipa/pr92497-2.c > > diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc > index f0976e363f7..fb2c0c0466b 100644 > --- a/gcc/ipa-prop.cc > +++ b/gcc/ipa-prop.cc > @@ -5765,6 +5765,34 @@ ipcp_modif_dom_walker::before_dom_children (basic_block bb) > return NULL; > } > > +/* If IPA-CP discovered a constant in parameter PARM at OFFSET of a given SIZE > + - whether passed by reference or not is given by BY_REF - return that > + constant. Otherwise return NULL_TREE. */ > + > +tree > +ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, > + HOST_WIDE_INT bit_offset, HOST_WIDE_INT bit_size) > +{ > + cgraph_node *node = cgraph_node::get (func->decl); > + ipcp_transformation *ts = ipcp_get_transformation_summary (node); > + > + if (!ts || !ts->m_agg_values) > + return NULL_TREE; > + > + int index = ts->get_param_index (func->decl, parm); > + if (index < 0) > + return NULL_TREE; > + > + ipa_argagg_value_list avl (ts); > + unsigned unit_offset = bit_offset / BITS_PER_UNIT; > + tree v = avl.get_value (index, unit_offset, by_ref); > + if (!v > + || maybe_ne (tree_to_poly_int64 (TYPE_SIZE (TREE_TYPE (v))), bit_size)) > + return NULL_TREE; > + > + return v; > +} > + > /* Return true if we have recorded VALUE and MASK about PARM. > Set VALUE and MASk accordingly. */ > > @@ -6037,11 +6065,6 @@ ipcp_transform_function (struct cgraph_node *node) > free_ipa_bb_info (bi); > fbi.bb_infos.release (); > > - ipcp_transformation *s = ipcp_transformation_sum->get (node); > - s->m_agg_values = NULL; > - s->bits = NULL; > - s->m_vr = NULL; > - > vec_free (descriptors); > if (cfg_changed) > delete_unreachable_blocks_update_callgraph (node, false); > diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h > index c570e8ec25a..9f25b44c71b 100644 > --- a/gcc/ipa-prop.h > +++ b/gcc/ipa-prop.h > @@ -1232,6 +1232,9 @@ void ipa_dump_param (FILE *, class ipa_node_params *info, int i); > void ipa_release_body_info (struct ipa_func_body_info *); > tree ipa_get_callee_param_type (struct cgraph_edge *e, int i); > bool ipcp_get_parm_bits (tree, tree *, widest_int *); > +tree ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, > + HOST_WIDE_INT bit_offset, > + HOST_WIDE_INT bit_size); > bool unadjusted_ptr_and_unit_offset (tree op, tree *ret, > poly_int64 *offset_ret); > > diff --git a/gcc/testsuite/gcc.dg/ipa/pr92497-1.c b/gcc/testsuite/gcc.dg/ipa/pr92497-1.c > new file mode 100644 > index 00000000000..dcece15963c > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/ipa/pr92497-1.c > @@ -0,0 +1,26 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2 -fno-early-inlining" } */ > + > +struct a {int a;}; > +static int > +foo (struct a a) > +{ > + if (!__builtin_constant_p (a.a)) > + __builtin_abort (); > + return a.a; > +} > + > +static int __attribute__ ((noinline)) > +bar (struct a a) > +{ > + return foo(a); > +} > + > +volatile int r; > + > +int main() > +{ > + struct a a={1}; > + r = bar (a); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/ipa/pr92497-2.c b/gcc/testsuite/gcc.dg/ipa/pr92497-2.c > new file mode 100644 > index 00000000000..c64090d1a7a > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/ipa/pr92497-2.c > @@ -0,0 +1,26 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2 -fno-early-inlining -fno-ipa-sra" } */ > + > +struct a {int a;}; > +static int > +foo (struct a *a) > +{ > + if (!__builtin_constant_p (a->a)) > + __builtin_abort (); > + return a->a; > +} > + > +static int __attribute__ ((noinline)) > +bar (struct a *a) > +{ > + return foo(a); > +} > + > +volatile int r; > + > +int main() > +{ > + struct a a={1}; > + r = bar (&a); > + return 0; > +} > diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc > index 27c84e78fcf..ce88b9bc95f 100644 > --- a/gcc/tree-ssa-sccvn.cc > +++ b/gcc/tree-ssa-sccvn.cc > @@ -74,6 +74,9 @@ along with GCC; see the file COPYING3. If not see > #include "ipa-modref-tree.h" > #include "ipa-modref.h" > #include "tree-ssa-sccvn.h" > +#include "alloc-pool.h" > +#include "symbol-summary.h" > +#include "ipa-prop.h" > > /* This algorithm is based on the SCC algorithm presented by Keith > Cooper and L. Taylor Simpson in "SCC-Based Value numbering" > @@ -2327,7 +2330,7 @@ vn_walk_cb_data::push_partial_def (pd_data pd, > with the current VUSE and performs the expression lookup. */ > > static void * > -vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) > +vn_reference_lookup_2 (ao_ref *op, tree vuse, void *data_) > { > vn_walk_cb_data *data = (vn_walk_cb_data *)data_; > vn_reference_t vr = data->vr; > @@ -2361,6 +2364,38 @@ vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) > return *slot; > } > > + if (SSA_NAME_IS_DEFAULT_DEF (vuse) && data->partial_defs.is_empty ()) ^^ do this check early > + { > + HOST_WIDE_INT offset, size; > + tree v = NULL_TREE; tree base = ao_ref_base (op); if ((TREE_CODE (base) == PARM_DECL || TREE_CODE (base) == MEM_REF) handle both kind of bases with ... > + && op->offset.is_constant (&offset) > + && op->size.is_constant (&size) > + && op->max_size_known_p () > + && known_eq (op->size, op->max_size)) ^^^ this preconditions (would have been missing in the MEM_REF branch before) { if (TREE_CODE (base) == PARM_DECL) > + v = ipcp_get_aggregate_const (cfun, base, false, offset, > + size); > + else if (TREE_CODE (base) == MEM_REF > + && integer_zerop (TREE_OPERAND (base, 1)) > + && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME > + && SSA_NAME_IS_DEFAULT_DEF (TREE_OPERAND (base, 0)) > + && (TREE_CODE (SSA_NAME_VAR (TREE_OPERAND (base, 0))) > + == PARM_DECL)) > + v = ipcp_get_aggregate_const (cfun, > + SSA_NAME_VAR (TREE_OPERAND (base, 0)), > + true, offset, size); > + } > + if (v) ... not here. > + return data->finish (vr->set, vr->base_set, v); > + } > + > return NULL; > } > >
Hello Richi, it took me quite time to get back to this but it might have actually helped because it forced me to re-read the code around and in turn simplify the patch. On Mon, Jun 12 2023, Richard Biener wrote: > On Fri, 9 Jun 2023, Martin Jambor wrote: > [...] >> @@ -2327,7 +2330,7 @@ vn_walk_cb_data::push_partial_def (pd_data pd, >> with the current VUSE and performs the expression lookup. */ >> >> static void * >> -vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) >> +vn_reference_lookup_2 (ao_ref *op, tree vuse, void *data_) >> { >> vn_walk_cb_data *data = (vn_walk_cb_data *)data_; >> vn_reference_t vr = data->vr; >> @@ -2361,6 +2364,38 @@ vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) >> return *slot; >> } >> >> + if (SSA_NAME_IS_DEFAULT_DEF (vuse) > && data->partial_defs.is_empty ()) > > ^^ do this check early The check is actually done right at the beginning of the function already so I simply removed it. > >> + { >> + HOST_WIDE_INT offset, size; >> + tree v = NULL_TREE; > tree base = ao_ref_base (op); > if ((TREE_CODE (base) == PARM_DECL > || TREE_CODE (base) == MEM_REF) > > handle both kind of bases with ... > >> + && op->offset.is_constant (&offset) >> + && op->size.is_constant (&size) >> + && op->max_size_known_p () >> + && known_eq (op->size, op->max_size)) > > ^^^ this preconditions (would have been missing in the MEM_REF branch > before) I missed that call to ao_ref_base fills in these fields - and in the pointer case that they are not filled in without it. I hope the patch below is the simplified version you wanted. The patch passed bootstrap and testing and also LTO bootstrap on x86_64-linux. Thanks, Martin PRs 68930 and 92497 show that when IPA-CP figures out constants in aggregate parameters or when passed by reference but the loads happen in an inlined function the information is lost. This happens even when the inlined function itself was known to have - or even cloned to have - such constants in incoming parameters because the transform phase of IPA passes is not run on them. See discussion in the bugs for reasons why. Honza suggested that we can plug the results of IPA-CP analysis into value numbering, so that FRE can figure out that some loads fetch known constants. This is what this patch attempts to do. The patch does not attempt to populate partial_defs with information from IPA-CP, this can be hopefully added as a follow-up. gcc/ChangeLog: 2023-08-11 Martin Jambor <mjambor@suse.cz> PR ipa/68930 PR ipa/92497 * ipa-prop.h (ipcp_get_aggregate_const): Declare. * ipa-prop.cc (ipcp_get_aggregate_const): New function. (ipcp_transform_function): Do not deallocate transformation info. * tree-ssa-sccvn.cc: Include alloc-pool.h, symbol-summary.h and ipa-prop.h. (vn_reference_lookup_2): When hitting default-def vuse, query IPA-CP transformation info for any known constants. gcc/testsuite/ChangeLog: 2023-06-07 Martin Jambor <mjambor@suse.cz> PR ipa/68930 PR ipa/92497 * gcc.dg/ipa/pr92497-1.c: New test. * gcc.dg/ipa/pr92497-2.c: Likewise. --- gcc/ipa-prop.cc | 33 +++++++++++++++++++++++---- gcc/ipa-prop.h | 3 +++ gcc/testsuite/gcc.dg/ipa/pr92497-1.c | 26 +++++++++++++++++++++ gcc/testsuite/gcc.dg/ipa/pr92497-2.c | 26 +++++++++++++++++++++ gcc/tree-ssa-sccvn.cc | 34 +++++++++++++++++++++++++++- 5 files changed, 116 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/ipa/pr92497-1.c create mode 100644 gcc/testsuite/gcc.dg/ipa/pr92497-2.c diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc index 4f6ed7b89bd..9efaa5cb848 100644 --- a/gcc/ipa-prop.cc +++ b/gcc/ipa-prop.cc @@ -5760,6 +5760,34 @@ ipcp_modif_dom_walker::before_dom_children (basic_block bb) return NULL; } +/* If IPA-CP discovered a constant in parameter PARM at OFFSET of a given SIZE + - whether passed by reference or not is given by BY_REF - return that + constant. Otherwise return NULL_TREE. */ + +tree +ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, + HOST_WIDE_INT bit_offset, HOST_WIDE_INT bit_size) +{ + cgraph_node *node = cgraph_node::get (func->decl); + ipcp_transformation *ts = ipcp_get_transformation_summary (node); + + if (!ts || !ts->m_agg_values) + return NULL_TREE; + + int index = ts->get_param_index (func->decl, parm); + if (index < 0) + return NULL_TREE; + + ipa_argagg_value_list avl (ts); + unsigned unit_offset = bit_offset / BITS_PER_UNIT; + tree v = avl.get_value (index, unit_offset, by_ref); + if (!v + || maybe_ne (tree_to_poly_int64 (TYPE_SIZE (TREE_TYPE (v))), bit_size)) + return NULL_TREE; + + return v; +} + /* Return true if we have recorded VALUE and MASK about PARM. Set VALUE and MASk accordingly. */ @@ -6031,11 +6059,6 @@ ipcp_transform_function (struct cgraph_node *node) free_ipa_bb_info (bi); fbi.bb_infos.release (); - ipcp_transformation *s = ipcp_transformation_sum->get (node); - s->m_agg_values = NULL; - s->bits = NULL; - s->m_vr = NULL; - vec_free (descriptors); if (cfg_changed) delete_unreachable_blocks_update_callgraph (node, false); diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index 410c951a256..7e033d2a7b8 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -1235,6 +1235,9 @@ void ipa_dump_param (FILE *, class ipa_node_params *info, int i); void ipa_release_body_info (struct ipa_func_body_info *); tree ipa_get_callee_param_type (struct cgraph_edge *e, int i); bool ipcp_get_parm_bits (tree, tree *, widest_int *); +tree ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, + HOST_WIDE_INT bit_offset, + HOST_WIDE_INT bit_size); bool unadjusted_ptr_and_unit_offset (tree op, tree *ret, poly_int64 *offset_ret); diff --git a/gcc/testsuite/gcc.dg/ipa/pr92497-1.c b/gcc/testsuite/gcc.dg/ipa/pr92497-1.c new file mode 100644 index 00000000000..dcece15963c --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/pr92497-1.c @@ -0,0 +1,26 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fno-early-inlining" } */ + +struct a {int a;}; +static int +foo (struct a a) +{ + if (!__builtin_constant_p (a.a)) + __builtin_abort (); + return a.a; +} + +static int __attribute__ ((noinline)) +bar (struct a a) +{ + return foo(a); +} + +volatile int r; + +int main() +{ + struct a a={1}; + r = bar (a); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/ipa/pr92497-2.c b/gcc/testsuite/gcc.dg/ipa/pr92497-2.c new file mode 100644 index 00000000000..c64090d1a7a --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/pr92497-2.c @@ -0,0 +1,26 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fno-early-inlining -fno-ipa-sra" } */ + +struct a {int a;}; +static int +foo (struct a *a) +{ + if (!__builtin_constant_p (a->a)) + __builtin_abort (); + return a->a; +} + +static int __attribute__ ((noinline)) +bar (struct a *a) +{ + return foo(a); +} + +volatile int r; + +int main() +{ + struct a a={1}; + r = bar (&a); + return 0; +} diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc index 32e06fae3b9..b1678911671 100644 --- a/gcc/tree-ssa-sccvn.cc +++ b/gcc/tree-ssa-sccvn.cc @@ -74,6 +74,9 @@ along with GCC; see the file COPYING3. If not see #include "ipa-modref-tree.h" #include "ipa-modref.h" #include "tree-ssa-sccvn.h" +#include "alloc-pool.h" +#include "symbol-summary.h" +#include "ipa-prop.h" /* This algorithm is based on the SCC algorithm presented by Keith Cooper and L. Taylor Simpson in "SCC-Based Value numbering" @@ -2327,7 +2330,7 @@ vn_walk_cb_data::push_partial_def (pd_data pd, with the current VUSE and performs the expression lookup. */ static void * -vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) +vn_reference_lookup_2 (ao_ref *op, tree vuse, void *data_) { vn_walk_cb_data *data = (vn_walk_cb_data *)data_; vn_reference_t vr = data->vr; @@ -2361,6 +2364,35 @@ vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) return *slot; } + if (SSA_NAME_IS_DEFAULT_DEF (vuse)) + { + HOST_WIDE_INT op_offset, op_size; + tree v = NULL_TREE; + tree base = ao_ref_base (op); + + if (base + && op->offset.is_constant (&op_offset) + && op->size.is_constant (&op_size) + && op->max_size_known_p () + && known_eq (op->size, op->max_size)) + { + if (TREE_CODE (base) == PARM_DECL) + v = ipcp_get_aggregate_const (cfun, base, false, op_offset, + op_size); + else if (TREE_CODE (base) == MEM_REF + && integer_zerop (TREE_OPERAND (base, 1)) + && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME + && SSA_NAME_IS_DEFAULT_DEF (TREE_OPERAND (base, 0)) + && (TREE_CODE (SSA_NAME_VAR (TREE_OPERAND (base, 0))) + == PARM_DECL)) + v = ipcp_get_aggregate_const (cfun, + SSA_NAME_VAR (TREE_OPERAND (base, 0)), + true, op_offset, op_size); + } + if (v) + return data->finish (vr->set, vr->base_set, v); + } + return NULL; }
On Sun, 13 Aug 2023, Martin Jambor wrote: > Hello Richi, > > it took me quite time to get back to this but it might have actually > helped because it forced me to re-read the code around and in turn > simplify the patch. > > On Mon, Jun 12 2023, Richard Biener wrote: > > On Fri, 9 Jun 2023, Martin Jambor wrote: > > > > [...] > > >> @@ -2327,7 +2330,7 @@ vn_walk_cb_data::push_partial_def (pd_data pd, > >> with the current VUSE and performs the expression lookup. */ > >> > >> static void * > >> -vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) > >> +vn_reference_lookup_2 (ao_ref *op, tree vuse, void *data_) > >> { > >> vn_walk_cb_data *data = (vn_walk_cb_data *)data_; > >> vn_reference_t vr = data->vr; > >> @@ -2361,6 +2364,38 @@ vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) > >> return *slot; > >> } > >> > >> + if (SSA_NAME_IS_DEFAULT_DEF (vuse) > > && data->partial_defs.is_empty ()) > > > > ^^ do this check early > > The check is actually done right at the beginning of the function > already so I simply removed it. > > > > >> + { > >> + HOST_WIDE_INT offset, size; > >> + tree v = NULL_TREE; > > tree base = ao_ref_base (op); > > if ((TREE_CODE (base) == PARM_DECL > > || TREE_CODE (base) == MEM_REF) > > > > handle both kind of bases with ... > > > >> + && op->offset.is_constant (&offset) > >> + && op->size.is_constant (&size) > >> + && op->max_size_known_p () > >> + && known_eq (op->size, op->max_size)) > > > > ^^^ this preconditions (would have been missing in the MEM_REF branch > > before) > > I missed that call to ao_ref_base fills in these fields - and in the > pointer case that they are not filled in without it. I hope the patch > below is the simplified version you wanted. Yes, looks good now. > The patch passed bootstrap and testing and also LTO bootstrap on > x86_64-linux. OK. Thanks, Richard. > Thanks, > > Martin > > > > PRs 68930 and 92497 show that when IPA-CP figures out constants in > aggregate parameters or when passed by reference but the loads happen > in an inlined function the information is lost. This happens even > when the inlined function itself was known to have - or even cloned to > have - such constants in incoming parameters because the transform > phase of IPA passes is not run on them. See discussion in the bugs > for reasons why. > > Honza suggested that we can plug the results of IPA-CP analysis into > value numbering, so that FRE can figure out that some loads fetch > known constants. This is what this patch attempts to do. The patch > does not attempt to populate partial_defs with information from > IPA-CP, this can be hopefully added as a follow-up. > > gcc/ChangeLog: > > 2023-08-11 Martin Jambor <mjambor@suse.cz> > > PR ipa/68930 > PR ipa/92497 > * ipa-prop.h (ipcp_get_aggregate_const): Declare. > * ipa-prop.cc (ipcp_get_aggregate_const): New function. > (ipcp_transform_function): Do not deallocate transformation info. > * tree-ssa-sccvn.cc: Include alloc-pool.h, symbol-summary.h and > ipa-prop.h. > (vn_reference_lookup_2): When hitting default-def vuse, query > IPA-CP transformation info for any known constants. > > gcc/testsuite/ChangeLog: > > 2023-06-07 Martin Jambor <mjambor@suse.cz> > > PR ipa/68930 > PR ipa/92497 > * gcc.dg/ipa/pr92497-1.c: New test. > * gcc.dg/ipa/pr92497-2.c: Likewise. > --- > gcc/ipa-prop.cc | 33 +++++++++++++++++++++++---- > gcc/ipa-prop.h | 3 +++ > gcc/testsuite/gcc.dg/ipa/pr92497-1.c | 26 +++++++++++++++++++++ > gcc/testsuite/gcc.dg/ipa/pr92497-2.c | 26 +++++++++++++++++++++ > gcc/tree-ssa-sccvn.cc | 34 +++++++++++++++++++++++++++- > 5 files changed, 116 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/ipa/pr92497-1.c > create mode 100644 gcc/testsuite/gcc.dg/ipa/pr92497-2.c > > diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc > index 4f6ed7b89bd..9efaa5cb848 100644 > --- a/gcc/ipa-prop.cc > +++ b/gcc/ipa-prop.cc > @@ -5760,6 +5760,34 @@ ipcp_modif_dom_walker::before_dom_children (basic_block bb) > return NULL; > } > > +/* If IPA-CP discovered a constant in parameter PARM at OFFSET of a given SIZE > + - whether passed by reference or not is given by BY_REF - return that > + constant. Otherwise return NULL_TREE. */ > + > +tree > +ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, > + HOST_WIDE_INT bit_offset, HOST_WIDE_INT bit_size) > +{ > + cgraph_node *node = cgraph_node::get (func->decl); > + ipcp_transformation *ts = ipcp_get_transformation_summary (node); > + > + if (!ts || !ts->m_agg_values) > + return NULL_TREE; > + > + int index = ts->get_param_index (func->decl, parm); > + if (index < 0) > + return NULL_TREE; > + > + ipa_argagg_value_list avl (ts); > + unsigned unit_offset = bit_offset / BITS_PER_UNIT; > + tree v = avl.get_value (index, unit_offset, by_ref); > + if (!v > + || maybe_ne (tree_to_poly_int64 (TYPE_SIZE (TREE_TYPE (v))), bit_size)) > + return NULL_TREE; > + > + return v; > +} > + > /* Return true if we have recorded VALUE and MASK about PARM. > Set VALUE and MASk accordingly. */ > > @@ -6031,11 +6059,6 @@ ipcp_transform_function (struct cgraph_node *node) > free_ipa_bb_info (bi); > fbi.bb_infos.release (); > > - ipcp_transformation *s = ipcp_transformation_sum->get (node); > - s->m_agg_values = NULL; > - s->bits = NULL; > - s->m_vr = NULL; > - > vec_free (descriptors); > if (cfg_changed) > delete_unreachable_blocks_update_callgraph (node, false); > diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h > index 410c951a256..7e033d2a7b8 100644 > --- a/gcc/ipa-prop.h > +++ b/gcc/ipa-prop.h > @@ -1235,6 +1235,9 @@ void ipa_dump_param (FILE *, class ipa_node_params *info, int i); > void ipa_release_body_info (struct ipa_func_body_info *); > tree ipa_get_callee_param_type (struct cgraph_edge *e, int i); > bool ipcp_get_parm_bits (tree, tree *, widest_int *); > +tree ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, > + HOST_WIDE_INT bit_offset, > + HOST_WIDE_INT bit_size); > bool unadjusted_ptr_and_unit_offset (tree op, tree *ret, > poly_int64 *offset_ret); > > diff --git a/gcc/testsuite/gcc.dg/ipa/pr92497-1.c b/gcc/testsuite/gcc.dg/ipa/pr92497-1.c > new file mode 100644 > index 00000000000..dcece15963c > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/ipa/pr92497-1.c > @@ -0,0 +1,26 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2 -fno-early-inlining" } */ > + > +struct a {int a;}; > +static int > +foo (struct a a) > +{ > + if (!__builtin_constant_p (a.a)) > + __builtin_abort (); > + return a.a; > +} > + > +static int __attribute__ ((noinline)) > +bar (struct a a) > +{ > + return foo(a); > +} > + > +volatile int r; > + > +int main() > +{ > + struct a a={1}; > + r = bar (a); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/ipa/pr92497-2.c b/gcc/testsuite/gcc.dg/ipa/pr92497-2.c > new file mode 100644 > index 00000000000..c64090d1a7a > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/ipa/pr92497-2.c > @@ -0,0 +1,26 @@ > +/* { dg-do run } */ > +/* { dg-options "-O2 -fno-early-inlining -fno-ipa-sra" } */ > + > +struct a {int a;}; > +static int > +foo (struct a *a) > +{ > + if (!__builtin_constant_p (a->a)) > + __builtin_abort (); > + return a->a; > +} > + > +static int __attribute__ ((noinline)) > +bar (struct a *a) > +{ > + return foo(a); > +} > + > +volatile int r; > + > +int main() > +{ > + struct a a={1}; > + r = bar (&a); > + return 0; > +} > diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc > index 32e06fae3b9..b1678911671 100644 > --- a/gcc/tree-ssa-sccvn.cc > +++ b/gcc/tree-ssa-sccvn.cc > @@ -74,6 +74,9 @@ along with GCC; see the file COPYING3. If not see > #include "ipa-modref-tree.h" > #include "ipa-modref.h" > #include "tree-ssa-sccvn.h" > +#include "alloc-pool.h" > +#include "symbol-summary.h" > +#include "ipa-prop.h" > > /* This algorithm is based on the SCC algorithm presented by Keith > Cooper and L. Taylor Simpson in "SCC-Based Value numbering" > @@ -2327,7 +2330,7 @@ vn_walk_cb_data::push_partial_def (pd_data pd, > with the current VUSE and performs the expression lookup. */ > > static void * > -vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) > +vn_reference_lookup_2 (ao_ref *op, tree vuse, void *data_) > { > vn_walk_cb_data *data = (vn_walk_cb_data *)data_; > vn_reference_t vr = data->vr; > @@ -2361,6 +2364,35 @@ vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) > return *slot; > } > > + if (SSA_NAME_IS_DEFAULT_DEF (vuse)) > + { > + HOST_WIDE_INT op_offset, op_size; > + tree v = NULL_TREE; > + tree base = ao_ref_base (op); > + > + if (base > + && op->offset.is_constant (&op_offset) > + && op->size.is_constant (&op_size) > + && op->max_size_known_p () > + && known_eq (op->size, op->max_size)) > + { > + if (TREE_CODE (base) == PARM_DECL) > + v = ipcp_get_aggregate_const (cfun, base, false, op_offset, > + op_size); > + else if (TREE_CODE (base) == MEM_REF > + && integer_zerop (TREE_OPERAND (base, 1)) > + && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME > + && SSA_NAME_IS_DEFAULT_DEF (TREE_OPERAND (base, 0)) > + && (TREE_CODE (SSA_NAME_VAR (TREE_OPERAND (base, 0))) > + == PARM_DECL)) > + v = ipcp_get_aggregate_const (cfun, > + SSA_NAME_VAR (TREE_OPERAND (base, 0)), > + true, op_offset, op_size); > + } > + if (v) > + return data->finish (vr->set, vr->base_set, v); > + } > + > return NULL; > } > >
diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc index f0976e363f7..fb2c0c0466b 100644 --- a/gcc/ipa-prop.cc +++ b/gcc/ipa-prop.cc @@ -5765,6 +5765,34 @@ ipcp_modif_dom_walker::before_dom_children (basic_block bb) return NULL; } +/* If IPA-CP discovered a constant in parameter PARM at OFFSET of a given SIZE + - whether passed by reference or not is given by BY_REF - return that + constant. Otherwise return NULL_TREE. */ + +tree +ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, + HOST_WIDE_INT bit_offset, HOST_WIDE_INT bit_size) +{ + cgraph_node *node = cgraph_node::get (func->decl); + ipcp_transformation *ts = ipcp_get_transformation_summary (node); + + if (!ts || !ts->m_agg_values) + return NULL_TREE; + + int index = ts->get_param_index (func->decl, parm); + if (index < 0) + return NULL_TREE; + + ipa_argagg_value_list avl (ts); + unsigned unit_offset = bit_offset / BITS_PER_UNIT; + tree v = avl.get_value (index, unit_offset, by_ref); + if (!v + || maybe_ne (tree_to_poly_int64 (TYPE_SIZE (TREE_TYPE (v))), bit_size)) + return NULL_TREE; + + return v; +} + /* Return true if we have recorded VALUE and MASK about PARM. Set VALUE and MASk accordingly. */ @@ -6037,11 +6065,6 @@ ipcp_transform_function (struct cgraph_node *node) free_ipa_bb_info (bi); fbi.bb_infos.release (); - ipcp_transformation *s = ipcp_transformation_sum->get (node); - s->m_agg_values = NULL; - s->bits = NULL; - s->m_vr = NULL; - vec_free (descriptors); if (cfg_changed) delete_unreachable_blocks_update_callgraph (node, false); diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index 211b12ff6b3..f68fa4a12dd 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -1221,6 +1221,9 @@ void ipa_dump_param (FILE *, class ipa_node_params *info, int i); void ipa_release_body_info (struct ipa_func_body_info *); tree ipa_get_callee_param_type (struct cgraph_edge *e, int i); bool ipcp_get_parm_bits (tree, tree *, widest_int *); +tree ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, + HOST_WIDE_INT bit_offset, + HOST_WIDE_INT bit_size); bool unadjusted_ptr_and_unit_offset (tree op, tree *ret, poly_int64 *offset_ret); diff --git a/gcc/testsuite/gcc.dg/ipa/pr92497-1.c b/gcc/testsuite/gcc.dg/ipa/pr92497-1.c new file mode 100644 index 00000000000..eb8f2e75fd0 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/pr92497-1.c @@ -0,0 +1,26 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fno-early-inlining" } */ + +struct a {int a;}; +static int //__attribute__ ((noinline)) +foo (struct a a) +{ + if (!__builtin_constant_p (a.a)) + __builtin_abort (); + return a.a; +} + +static int __attribute__ ((noinline)) +bar (struct a a) +{ + return foo(a); +} + +volatile int r; + +int main() +{ + struct a a={1}; + r = bar (a); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/ipa/pr92497-2.c b/gcc/testsuite/gcc.dg/ipa/pr92497-2.c new file mode 100644 index 00000000000..56e611df8f9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/pr92497-2.c @@ -0,0 +1,26 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fno-early-inlining -fno-ipa-sra" } */ + +struct a {int a;}; +static int //__attribute__ ((noinline)) +foo (struct a *a) +{ + if (!__builtin_constant_p (a->a)) + __builtin_abort (); + return a->a; +} + +static int __attribute__ ((noinline)) +bar (struct a *a) +{ + return foo(a); +} + +volatile int r; + +int main() +{ + struct a a={1}; + r = bar (&a); + return 0; +} diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc index 27c84e78fcf..33215b5fc82 100644 --- a/gcc/tree-ssa-sccvn.cc +++ b/gcc/tree-ssa-sccvn.cc @@ -74,6 +74,9 @@ along with GCC; see the file COPYING3. If not see #include "ipa-modref-tree.h" #include "ipa-modref.h" #include "tree-ssa-sccvn.h" +#include "alloc-pool.h" +#include "symbol-summary.h" +#include "ipa-prop.h" /* This algorithm is based on the SCC algorithm presented by Keith Cooper and L. Taylor Simpson in "SCC-Based Value numbering" @@ -2327,7 +2330,7 @@ vn_walk_cb_data::push_partial_def (pd_data pd, with the current VUSE and performs the expression lookup. */ static void * -vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) +vn_reference_lookup_2 (ao_ref *op, tree vuse, void *data_) { vn_walk_cb_data *data = (vn_walk_cb_data *)data_; vn_reference_t vr = data->vr; @@ -2361,6 +2364,37 @@ vn_reference_lookup_2 (ao_ref *op ATTRIBUTE_UNUSED, tree vuse, void *data_) return *slot; } + if (SSA_NAME_IS_DEFAULT_DEF (vuse)) + { + HOST_WIDE_INT offset, size; + tree v = NULL_TREE; + if (op->base && TREE_CODE (op->base) == PARM_DECL + && op->offset.is_constant (&offset) + && op->size.is_constant (&size) + && op->max_size_known_p () + && known_eq (op->size, op->max_size)) + v = ipcp_get_aggregate_const (cfun, op->base, false, offset, size); + else if (op->ref) + { + HOST_WIDE_INT offset, size; + bool reverse; + tree base = get_ref_base_and_extent_hwi (op->ref, &offset, + &size, &reverse); + if (base + && TREE_CODE (base) == MEM_REF + && integer_zerop (TREE_OPERAND (base, 1)) + && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME + && SSA_NAME_IS_DEFAULT_DEF (TREE_OPERAND (base, 0)) + && (TREE_CODE (SSA_NAME_VAR (TREE_OPERAND (base, 0))) + == PARM_DECL)) + v = ipcp_get_aggregate_const (cfun, + SSA_NAME_VAR (TREE_OPERAND (base, 0)), + true, offset, size); + } + if (v) + return data->finish (vr->set, vr->base_set, v); + } + return NULL; }