| Message ID | 5632448E.5030509@mentor.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 29 Oct 2015, Tom de Vries wrote: > On 29/10/15 14:12, Richard Biener wrote: > > On Thu, 29 Oct 2015, Tom de Vries wrote: > > > > > >On 29/10/15 12:13, Richard Biener wrote: > > > > > >On Wed, 28 Oct 2015, Tom de Vries wrote: > > > > > > > > > > > > > > > >On 28/10/15 16:35, Richard Biener wrote: > > > > > > > > > > > > > >On Tue, 27 Oct 2015, Tom de Vries wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >On 27/10/15 13:24, Tom de Vries wrote: > > > > > > > > > > > > > > > > > > > > > >Thinking it over a bit more, I > > > > > > > > > > > > realized the constraint > > > > > > > > > > > > > >handling started > > > > > > > > > > > > > > > > > > > > > >to be messy. I've reworked the patch > > > > > > > > > > > > series to simplify that > > > > > > > > > > > > > >first. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1 Simplify constraint > > > > > > > > > > > > handling > > > > > > > > > > > > > > > > > > > > > > 2 Rename > > > > > > > > > > > > make_restrict_var_constraints to > > > > > > > > > > > > > > > > > > > > > >make_param_constraints > > > > > > > > > > > > > > > > > > > > > > 3 Add recursion to > > > > > > > > > > > > make_param_constraints > > > > > > > > > > > > > > > > > > > > > > 4 Add handle_param > > > > > > > > > > > > parameter to > > > > > > > > > > > > > >create_variable_info_for_1 > > > > > > > > > > > > > > > > > > > > > > 5 Handle recursive > > > > > > > > > > > > restrict pointer in > > > > > > > > > > > > > > > > > > > > > >create_variable_info_for_1 > > > > > > > > > > > > > > > > > > > > > > 6 Handle restrict struct > > > > > > > > > > > > fields recursively > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >Currently doing bootstrap and regtest > > > > > > > > > > > > on x86_64. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >I'll repost the patch series in reply > > > > > > > > > > > > to this message. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >This patch gets rid of this bit of code in > > > > > > > > > > > >intra_create_variable_infos: > > > > > > > > > > > > > > > > > >... > > > > > > > > > > > > > > > > > > if (restrict_pointer_p) > > > > > > > > > > > > > > > > > > make_constraint_from_global_restrict > > > > > > > > > > (p, "PARM_RESTRICT"); > > > > > > > > > > > > > > > > > > else > > > > > > > > > > > > > > > > > >.. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >I already proposed to remove it here ( > > > > > > > > > > > > > > > > > > > > > > > > > > > >https://gcc.gnu.org/ml/gcc-patches/2015-10/msg02426.html > > > > > > > > > > ) but > > > > > > > > > > > >there is a > > > > > > > > > > > > > > > > > >problem with that approach: It can happen > > > > > > > > > > that restrict_pointer_p > > > > > > > > > > > >is true, > > > > > > > > > > > > > > > > > >but > > > > > > > > > > > > > > > > > >p->only_restrict_pointers is false. This > > > > > > > > > > happens with fipa-pta, > > > > > > > > > > > >when > > > > > > > > > > > > > > > > > >create_function_info_for created a varinfo > > > > > > > > > > for the parameter > > > > > > > > > > > >before > > > > > > > > > > > > > > > > > >intra_create_variable_infos was called. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >The patch handles that case now by setting > > > > > > > > > > > >p->only_restrict_pointers. > > > > > > > > > > > > > > > > > > > > > > > > > > > >Hmm, but ... restrict only has an effect in non-IPA > > > > > > > > mode. > > > > > > > > > > > > > > > > > > > >Indeed, I also realized that by now. > > > > > > > > > > > > > > > > > > > >I wrote attached patch to make that explicit and simplify > > > > > > fipa-pta. > > > > > > > > > > > > > > > > > > > >OK for trunk if bootstrap and reg-test succeeds? > > > > > > > >First, there was an error in the patch, it tested for flag_ipa_pta (so it > > > also > > > >affected ealias), but it was supposed to test for in_ipa mode. That is > > > fixed > > > >in attached version. > > > > > > > > > >I don't see the patch simplifies anything but only removes spurious > > > > > >settings by adding IMHO redundant checks. > > > > > > > >Consider testcase: > > > >... > > > >int __attribute__((noinline, noclone)) > > > >foo (int *__restrict__ a, int *__restrict__ b) > > > >{ > > > > *a = 1; > > > > *b = 2; > > > >} > > > > > > > >int __attribute__((noinline, noclone)) > > > >bar (int *a, int *b) > > > >{ > > > > foo (a, b); > > > >} > > > >... > > > > > > > >The impact of this patch in the pta dump (focusing on the constraints > > > bit) is: > > > >... > > > > Generating constraints for foo (foo) > > > > > > > >-foo.arg0 = &PARM_NOALIAS(20) > > > >-PARM_NOALIAS(20) = NONLOCAL > > > >-foo.arg1 = &PARM_NOALIAS(21) > > > >-PARM_NOALIAS(21) = NONLOCAL > > > >+foo.arg0 = &NONLOCAL > > > >+foo.arg1 = &NONLOCAL > > > >... > > > > > > > >That's the kind of simplifications I'm trying to achieve. > > Yes, but as I said we should refactor things to avoid calling > > the intra constraints generation from the IPA path. > > Ah, I see. > > So, like this? OK for trunk if bootstrap and reg-test succeeds? Yes, like this. But you miss to apply the same to the static chain, and the varargs "rest". Ok with that change. Richard.
Add initial constraints in create_function_info_for 2015-10-29 Tom de Vries <tom@codesourcery.com> * tree-ssa-structalias.c (ipa_pta_execute): Add extra arg to call to create_function_info_for. Dump constraints generated during create_function_info_for. Move intra_create_variable_infos call and function-return-values-escape bit to ... (create_function_info_for): ... here, and merge intra_create_variable_infos call with argument loop. Add and handle nonlocal_p parameter. --- gcc/tree-ssa-structalias.c | 94 ++++++++++++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 36 deletions(-) diff --git a/gcc/tree-ssa-structalias.c b/gcc/tree-ssa-structalias.c index 3bef607..3a0891c 100644 --- a/gcc/tree-ssa-structalias.c +++ b/gcc/tree-ssa-structalias.c @@ -5421,10 +5421,12 @@ count_num_arguments (tree decl, bool *is_varargs) } /* Creation function node for DECL, using NAME, and return the index - of the variable we've created for the function. */ + of the variable we've created for the function. If NONLOCAL_p, create + initial constraints. */ static varinfo_t -create_function_info_for (tree decl, const char *name, bool add_id) +create_function_info_for (tree decl, const char *name, bool add_id, + bool nonlocal_p) { struct function *fn = DECL_STRUCT_FUNCTION (decl); varinfo_t vi, prev_vi; @@ -5536,6 +5538,28 @@ create_function_info_for (tree decl, const char *name, bool add_id) insert_vi_for_tree (DECL_RESULT (decl), resultvi); } + /* We also need to make function return values escape. Nothing + escapes by returning from main though. */ + if (nonlocal_p + && !MAIN_NAME_P (DECL_NAME (decl))) + { + varinfo_t fi, rvi; + fi = lookup_vi_for_tree (decl); + rvi = first_vi_for_offset (fi, fi_result); + if (rvi && rvi->offset == fi_result) + { + struct constraint_expr includes; + struct constraint_expr var; + includes.var = escaped_id; + includes.offset = 0; + includes.type = SCALAR; + var.var = rvi->id; + var.offset = 0; + var.type = SCALAR; + process_constraint (new_constraint (includes, var)); + } + } + /* Set up variables for each argument. */ arg = DECL_ARGUMENTS (decl); for (i = 0; i < num_args; i++) @@ -5562,6 +5586,11 @@ create_function_info_for (tree decl, const char *name, bool add_id) gcc_assert (prev_vi->offset < argvi->offset); prev_vi->next = argvi->id; prev_vi = argvi; + + if (nonlocal_p + && argvi->may_have_pointers) + make_constraint_from (argvi, nonlocal_id); + if (arg) { insert_vi_for_tree (arg, argvi); @@ -7317,8 +7346,34 @@ ipa_pta_execute (void) gcc_assert (!node->clone_of); + /* For externally visible or attribute used annotated functions use + local constraints for their arguments. + For local functions we see all callers and thus do not need initial + constraints for parameters. */ + bool nonlocal_p = (node->used_from_other_partition + || node->externally_visible + || node->force_output + || node->address_taken); + vi = create_function_info_for (node->decl, - alias_get_name (node->decl), false); + alias_get_name (node->decl), false, + nonlocal_p); + if (dump_file + && from != constraints.length ()) + { + fprintf (dump_file, + "Generating intial constraints for %s", node->name ()); + if (DECL_ASSEMBLER_NAME_SET_P (node->decl)) + fprintf (dump_file, " (%s)", + IDENTIFIER_POINTER + (DECL_ASSEMBLER_NAME (node->decl))); + fprintf (dump_file, "\n\n"); + dump_constraints (dump_file, from); + fprintf (dump_file, "\n"); + + from = constraints.length (); + } + node->call_for_symbol_thunks_and_aliases (associate_varinfo_to_alias, vi, true); } @@ -7365,39 +7420,6 @@ ipa_pta_execute (void) func = DECL_STRUCT_FUNCTION (node->decl); gcc_assert (cfun == NULL); - /* For externally visible or attribute used annotated functions use - local constraints for their arguments. - For local functions we see all callers and thus do not need initial - constraints for parameters. */ - if (node->used_from_other_partition - || node->externally_visible - || node->force_output - || node->address_taken) - { - intra_create_variable_infos (func); - - /* We also need to make function return values escape. Nothing - escapes by returning from main though. */ - if (!MAIN_NAME_P (DECL_NAME (node->decl))) - { - varinfo_t fi, rvi; - fi = lookup_vi_for_tree (node->decl); - rvi = first_vi_for_offset (fi, fi_result); - if (rvi && rvi->offset == fi_result) - { - struct constraint_expr includes; - struct constraint_expr var; - includes.var = escaped_id; - includes.offset = 0; - includes.type = SCALAR; - var.var = rvi->id; - var.offset = 0; - var.type = SCALAR; - process_constraint (new_constraint (includes, var)); - } - } - } - /* Build constriants for the function body. */ FOR_EACH_BB_FN (bb, func) { -- 1.9.1