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?
Thanks,
- Tom
Add initial constraints in create_function_info_for
2015-10-29 Tom de Vries <t...@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