From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1611) id 89431383EB09; Wed, 14 Dec 2022 00:04:26 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 89431383EB09 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1670976266; bh=eR7cgqtK4xL4sO7lOYjKAclpRT2qyT2oxrPqSLeEa7I=; h=From:To:Subject:Date:From; b=KuegimKOB60LmRhiYFAAEaPrwX0/pKo4m6Wrd0Z9C2qpR2S6vS2bGNV1Aac8XlH1v 9atZXlqF84WWiZtkNlJDO12gqOrX8ee/rxmvS83m3EMNamwfpdxALgRdzBFsWlnJne kKw++XxzVGUOigP5iIWQHH8sreG7IFbLGN2A9F30= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Martin Jambor To: gcc-cvs@gcc.gnu.org Subject: [gcc r13-4687] ipa-sra: Treat REFERENCE_TYPES as always dereferencable X-Act-Checkin: gcc X-Git-Author: Martin Jambor X-Git-Refname: refs/heads/master X-Git-Oldrev: 095a13eda2caf6842096a3ab78b2081c50fe8799 X-Git-Newrev: 10478270fe0c39c59eb0f35d19356a63bdf3a2ad Message-Id: <20221214000426.89431383EB09@sourceware.org> Date: Wed, 14 Dec 2022 00:04:26 +0000 (GMT) List-Id: https://gcc.gnu.org/g:10478270fe0c39c59eb0f35d19356a63bdf3a2ad commit r13-4687-g10478270fe0c39c59eb0f35d19356a63bdf3a2ad Author: Martin Jambor Date: Wed Dec 14 00:33:06 2022 +0100 ipa-sra: Treat REFERENCE_TYPES as always dereferencable C++ and especially Fortran pass data by references which are not pointers potentially pointing anywhere and so can be assumed to be safely dereferencable. This patch teaches IPA-SRA to treat them as such and avoid the dance we do to prove that we can move loads from them to the caller. When we do not know that a dereference will happen all the time, we need a heuristics so that we do not force memory accesses that normally happen only rarely. The patch simply uses the (possibly guessed) profile and checks whether the (expected) number of loads is at least half of function invocations invocations - the half is now configurable with a param as requested by Honza. gcc/ChangeLog: 2022-12-13 Martin Jambor PR ipa/103585 * params.opt (ipa-sra-deref-prob-threshold): New parameter. * doc/invoke.texi (ipa-sra-deref-prob-threshold): Document it. * ipa-sra.cc (struct gensum_param_access): New field load_count. (struct gensum_param_desc): New field safe_ref, adjusted comments. (by_ref_count): Renamed to unsafe_by_ref_count, adjusted all uses. (dump_gensum_access): Dump the new field. (dump_gensum_param_descriptor): Likewise. (create_parameter_descriptors): Set safe_ref field, move setting by_ref forward. Only increment unsafe_by_ref_count for unsafe by_ref parameters. (allocate_access): Initialize new field. (mark_param_dereference): Adjust indentation. Only add data to bb_dereferences for unsafe by_ref parameters. (scan_expr_access): For loads, accumulate BB counts. (dereference_probable_p): New function. (check_gensum_access): Fix leading comment, add parameter FUN. Check cumulative counts of loads for safe by_ref accesses instead of dereferences. (process_scan_results): Do not propagate dereference distances for safe by_ref parameters. Pass fun to check_gensum_access. Safe by_ref params do not need the postdominance check. gcc/testsuite/ChangeLog: 2022-11-11 Martin Jambor * g++.dg/ipa/ipa-sra-5.C: New test Diff: --- gcc/doc/invoke.texi | 5 ++ gcc/ipa-sra.cc | 101 +++++++++++++++++++++++++---------- gcc/params.opt | 4 ++ gcc/testsuite/g++.dg/ipa/ipa-sra-5.C | 23 ++++++++ 4 files changed, 105 insertions(+), 28 deletions(-) diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 13371972fd1..0459714d100 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -15509,6 +15509,11 @@ the parameter is reserved exclusively for debug insns created by @option{-fvar-tracking-assignments}, but debug insns may get (non-overlapping) uids above it if the reserved range is exhausted. +@item ipa-sra-deref-prob-threshold +IPA-SRA replaces a pointer which is known not be NULL with one or more +new parameters only when the probability (in percent, relative to +function entry) of it being dereferenced is higher than this parameter. + @item ipa-sra-ptr-growth-factor IPA-SRA replaces a pointer to an aggregate with one or more new parameters only when their cumulative size is less or equal to diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc index 0f137e810fe..866f52e89e0 100644 --- a/gcc/ipa-sra.cc +++ b/gcc/ipa-sra.cc @@ -151,6 +151,8 @@ struct gensum_param_access arguments. */ tree alias_ptr_type; + /* Cumulative count of all loads. */ + profile_count load_count; /* Have there been writes to or reads from this exact location except for as arguments to a function call that can be tracked. */ bool nonarg; @@ -207,8 +209,13 @@ struct gensum_param_desc by reference that is a candidate for being converted to a set of parameters passing those data by value. */ bool split_candidate; - /* Is this a parameter passing stuff by reference? */ + /* Is this a parameter passing stuff by reference (either a pointer or a + source language reference type)? */ bool by_ref; + /* If this parameter passes stuff by reference, can it be safely dereferenced + without performing further checks (for example because it is a + REFERENCE_TYPE)? */ + bool safe_ref; /* The number of this parameter as they are ordered in function decl. */ int param_number; @@ -561,7 +568,7 @@ int aa_walking_limit; accessed in that BB. */ HOST_WIDE_INT *bb_dereferences = NULL; /* How many by-reference parameters there are in the current function. */ -int by_ref_count; +int unsafe_by_ref_count; /* Bitmap of BBs that can cause the function to "stop" progressing by returning, throwing externally, looping infinitely or calling a function @@ -643,6 +650,8 @@ dump_gensum_access (FILE *f, gensum_param_access *access, unsigned indent) print_generic_expr (f, access->type); fprintf (f, ", alias_ptr_type: "); print_generic_expr (f, access->alias_ptr_type); + fprintf (f, ", load_count: "); + access->load_count.dump (f); fprintf (f, ", nonarg: %u, reverse: %u\n", access->nonarg, access->reverse); for (gensum_param_access *ch = access->first_child; ch; @@ -692,7 +701,8 @@ dump_gensum_param_descriptor (FILE *f, gensum_param_desc *desc) return; } if (desc->by_ref) - fprintf (f, " by_ref with %u pass throughs\n", desc->ptr_pt_count); + fprintf (f, " %s by_ref with %u pass throughs\n", + desc->safe_ref ? "safe" : "unsafe", desc->ptr_pt_count); for (gensum_param_access *acc = desc->accesses; acc; acc = acc->next_sibling) dump_gensum_access (f, acc, 2); @@ -1140,6 +1150,8 @@ create_parameter_descriptors (cgraph_node *node, if (POINTER_TYPE_P (type)) { + desc->by_ref = true; + desc->safe_ref = (TREE_CODE (type) == REFERENCE_TYPE); type = TREE_TYPE (type); if (TREE_CODE (type) == FUNCTION_TYPE @@ -1187,7 +1199,6 @@ create_parameter_descriptors (cgraph_node *node, "nonarg uses\n"); continue; } - desc->by_ref = true; } else if (!AGGREGATE_TYPE_P (type)) { @@ -1231,8 +1242,8 @@ create_parameter_descriptors (cgraph_node *node, ret = true; desc->split_candidate = true; - if (desc->by_ref) - desc->deref_index = by_ref_count++; + if (desc->by_ref && !desc->safe_ref) + desc->deref_index = unsafe_by_ref_count++; } return ret; } @@ -1301,6 +1312,7 @@ allocate_access (gensum_param_desc *desc, memset (access, 0, sizeof (*access)); access->offset = offset; access->size = size; + access->load_count = profile_count::zero (); return access; } @@ -1555,15 +1567,16 @@ asm_visit_addr (gimple *, tree op, tree, void *) static void mark_param_dereference (gensum_param_desc *desc, HOST_WIDE_INT dist, - basic_block bb) + basic_block bb) { gcc_assert (desc->by_ref); gcc_checking_assert (desc->split_candidate); - if (bitmap_bit_p (final_bbs, bb->index)) + if (desc->safe_ref + || bitmap_bit_p (final_bbs, bb->index)) return; - int idx = bb->index * by_ref_count + desc->deref_index; + int idx = bb->index * unsafe_by_ref_count + desc->deref_index; if (bb_dereferences[idx] < dist) bb_dereferences[idx] = dist; } @@ -1811,6 +1824,8 @@ scan_expr_access (tree expr, gimple *stmt, isra_scan_context ctx, other. */ access->nonarg = true; } + else if (ctx == ISRA_CTX_LOAD && bb->count.initialized_p ()) + access->load_count += bb->count; if (!access->type) { @@ -2092,9 +2107,9 @@ dump_dereferences_table (FILE *f, struct function *fun, const char *str) if (bb != EXIT_BLOCK_PTR_FOR_FN (fun)) { int i; - for (i = 0; i < by_ref_count; i++) + for (i = 0; i < unsafe_by_ref_count; i++) { - int idx = bb->index * by_ref_count + i; + int idx = bb->index * unsafe_by_ref_count + i; fprintf (f, " %4" HOST_WIDE_INT_PRINT "d", bb_dereferences[idx]); } } @@ -2139,15 +2154,15 @@ propagate_dereference_distances (struct function *fun) if (bitmap_bit_p (final_bbs, bb->index)) continue; - for (i = 0; i < by_ref_count; i++) + for (i = 0; i < unsafe_by_ref_count; i++) { - int idx = bb->index * by_ref_count + i; + int idx = bb->index * unsafe_by_ref_count + i; bool first = true; HOST_WIDE_INT inh = 0; FOR_EACH_EDGE (e, ei, bb->succs) { - int succ_idx = e->dest->index * by_ref_count + i; + int succ_idx = e->dest->index * unsafe_by_ref_count + i; if (e->dest == EXIT_BLOCK_PTR_FOR_FN (fun)) continue; @@ -2184,13 +2199,24 @@ propagate_dereference_distances (struct function *fun) "Dereference table after propagation:\n"); } -/* Perform basic checks on ACCESS to PARM described by DESC and all its - children, return true if the parameter cannot be split, otherwise return - true and update *TOTAL_SIZE and *ONLY_CALLS. ENTRY_BB_INDEX must be the - index of the entry BB in the function of PARM. */ +/* Return true if the ACCESS loads happen frequently enough in FUN to risk + moving them to the caller and only pass the result. */ static bool -check_gensum_access (tree parm, gensum_param_desc *desc, +dereference_probable_p (struct function *fun, gensum_param_access *access) +{ + int threshold = opt_for_fn (fun->decl, param_ipa_sra_deref_prob_threshold); + return access->load_count + >= ENTRY_BLOCK_PTR_FOR_FN (fun)->count.apply_scale (threshold, 100); +} + +/* Perform basic checks on ACCESS to PARM (of FUN) described by DESC and all + its children, return true if the parameter cannot be split, otherwise return + false and update *NONARG_ACC_SIZE and *ONLY_CALLS. ENTRY_BB_INDEX must be + the index of the entry BB in the function of PARM. */ + +static bool +check_gensum_access (struct function *fun, tree parm, gensum_param_desc *desc, gensum_param_access *access, HOST_WIDE_INT *nonarg_acc_size, bool *only_calls, int entry_bb_index) @@ -2218,19 +2244,31 @@ check_gensum_access (tree parm, gensum_param_desc *desc, if (desc->by_ref) { - int idx = (entry_bb_index * by_ref_count + desc->deref_index); - if ((access->offset + access->size) > bb_dereferences[idx]) + if (desc->safe_ref) { - disqualify_split_candidate (desc, "Would create a possibly " - "illegal dereference in a caller."); - return true; + if (!dereference_probable_p (fun, access)) + { + disqualify_split_candidate (desc, "Dereferences in callers " + "would happen much more frequently."); + return true; + } + } + else + { + int idx = (entry_bb_index * unsafe_by_ref_count + desc->deref_index); + if ((access->offset + access->size) > bb_dereferences[idx]) + { + disqualify_split_candidate (desc, "Would create a possibly " + "illegal dereference in a caller."); + return true; + } } } for (gensum_param_access *ch = access->first_child; ch; ch = ch->next_sibling) - if (check_gensum_access (parm, desc, ch, nonarg_acc_size, only_calls, + if (check_gensum_access (fun, parm, desc, ch, nonarg_acc_size, only_calls, entry_bb_index)) return true; @@ -2288,6 +2326,7 @@ process_scan_results (cgraph_node *node, struct function *fun, if (!dereferences_propagated && desc->by_ref + && !desc->safe_ref && desc->accesses) { propagate_dereference_distances (fun); @@ -2302,8 +2341,8 @@ process_scan_results (cgraph_node *node, struct function *fun, for (gensum_param_access *acc = desc->accesses; acc; acc = acc->next_sibling) - if (check_gensum_access (parm, desc, acc, &nonarg_acc_size, &only_calls, - entry_bb_index)) + if (check_gensum_access (fun, parm, desc, acc, &nonarg_acc_size, + &only_calls, entry_bb_index)) { check_failed = true; break; @@ -2394,6 +2433,12 @@ process_scan_results (cgraph_node *node, struct function *fun, if (!uses_memory_as_obtained) continue; + if (desc->safe_ref) + { + csum->m_arg_flow[argidx].safe_to_import_accesses = true; + continue; + } + /* Post-dominator check placed last, hoping that it usually won't be needed. */ if (!pdoms_calculated) @@ -4124,7 +4169,7 @@ ipa_sra_summarize_function (cgraph_node *node) cfun_pushed = true; final_bbs = BITMAP_ALLOC (NULL); bb_dereferences = XCNEWVEC (HOST_WIDE_INT, - by_ref_count + unsafe_by_ref_count * last_basic_block_for_fn (fun)); aa_walking_limit = opt_for_fn (node->decl, param_ipa_max_aa_steps); scan_function (node, fun); diff --git a/gcc/params.opt b/gcc/params.opt index 397ec0bd128..a574f17bddb 100644 --- a/gcc/params.opt +++ b/gcc/params.opt @@ -282,6 +282,10 @@ Maximum number of different predicates used to track properties of loops in IPA Common Joined UInteger Var(param_ipa_max_switch_predicate_bounds) Init(5) Param Optimization Maximal number of boundary endpoints of case ranges of switch statement used during IPA function summary generation. +-param=ipa-sra-deref-prob-threshold= +Common Joined UInteger Var(param_ipa_sra_deref_prob_threshold) Init(50) IntegerRange(0, 100) Param Optimization +Minimum probability (in percent) of dereferencing of a function pointer parameter for it to be considered for replacement with simple values. + -param=ipa-sra-max-replacements= Common Joined UInteger Var(param_ipa_sra_max_replacements) Optimization Init(8) IntegerRange(0, 16) Param Maximum pieces that IPA-SRA tracks per formal parameter, as a consequence, also the maximum number of replacements of a formal parameter. diff --git a/gcc/testsuite/g++.dg/ipa/ipa-sra-5.C b/gcc/testsuite/g++.dg/ipa/ipa-sra-5.C new file mode 100644 index 00000000000..26a221e1734 --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/ipa-sra-5.C @@ -0,0 +1,23 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-ipa-sra" } */ + +volatile int vi; + +static void __attribute__((noinline)) +foo (int c, int &r) +{ + int i; + if (c) + i = r; + else + i = 0; + vi = i; +} + +void +bar (int c, int j) +{ + foo (c, j); +} + +/* { dg-final { scan-ipa-dump "Will split parameter" "sra" } } */