> 
> 2012-11-05  Martin Jambor  <mjam...@suse.cz>
> 
>       PR tree-optimization/53787
>       * ipa-cp.c (ipcp_value_source): New field offset.
>       (ipcp_agg_lattice): New type.
>       (ipcp_param_lattices): Likewise, move virt_call from ipcp_lattice here.
>       (ipcp_agg_lattice_pool): New variable.
>       (ipa_get_parm_lattices): New function.
>       (ipa_get_lattice): Turned into ipa_get_scalar_lat, use the above.
>       Adjusted all callers.
>       (print_lattice): New function.
>       (print_all_lattices): Use the above, also print aggregate lattices.
>       (set_agg_lats_to_bottom): New function.
>       (set_agg_lats_contain_variable): Likewise.
>       (set_all_contains_variable): Likewise.
>       (initialize_node_lattices): Also handle aggregate lattices, set
>       virt_call in ipcp_param_lattices.
>       (add_value_source): Handle offsets.
>       (add_value_to_lattice): Likewise.
>       (add_scalar_value_to_lattice): New function.
>       (propagate_vals_accross_pass_through): Use add_scalar_value_to_lattice.
>       (propagate_vals_accross_ancestor): Likewise.
>       (propagate_accross_jump_function): Renamed to
>       propagate_scalar_accross_jump_function, use
>       add_scalar_value_to_lattice.
>       (set_check_aggs_by_ref): New function.
>       (merge_agg_lats_step): Likewise.
>       (set_chain_of_aglats_contains_variable): Likewise.
>       (merge_aggregate_lattices): Likewise.
>       (propagate_constants_accross_call): Also handle aggregate lattices.
>       (hint_time_bonus): New function.
>       (context_independent_aggregate_values): Likewise.
>       (gather_context_independent_values): Also handle agggregate values.
>       (agg_jmp_p_vec_for_t_vec): New function.
>       (estimate_local_effects): Also handle agggregate values.
>       (add_all_node_vals_to_toposort): Likewise.
>       (ipcp_propagate_stage): Use struct ipcp_param_lattices.
>       (get_clone_agg_value): New function.
>       (cgraph_edge_brings_value_p): Also handle agggregate values.
>       (create_specialized_node): Likewise.
>       (find_more_values_for_callers_subset): Rename to
>       find_more_scalar_values_for_callers_subset.  Modify dump.
>       (copy_plats_to_inter): New function.
>       (intersect_with_plats): Likewise.
>       (agg_replacements_to_vector): Likewise.
>       (intersect_with_agg_replacements): Likewise.
>       (find_aggregate_values_for_callers_subset): Likewise.
>       (known_aggs_to_agg_replacement_list): Likewise.
>       (cgraph_edge_brings_all_scalars_for_node): Likewise.
>       (cgraph_edge_brings_all_agg_vals_for_node): Likewise.
>       (perhaps_add_new_callers): Old functionality moved to
>       cgraph_edge_brings_all_scalars_for_node, call it and
>       cgraph_edge_brings_all_agg_vals_for_node.
>       (ipcp_val_in_agg_replacements_p): New function.
>       (decide_about_value): New function.
>       (decide_whether_version_node): A lot of functionality moved to
>       decide_about_value.  Also handle agggregate values.
>       (ipcp_driver): Also allocate ipcp_agg_lattice_pool.
>       (pass_ipa_cp): Fill in new entries.
>       * ipa-prop.c (ipa_node_agg_replacements): New variable.
>       (free_parms_ainfo): New function.
>       (ipa_analyze_node): Use free_parms_ainfo to free stuff.
>       (ipa_find_agg_cst_for_param): Do not rely on offset ordering.
>       (ipa_set_node_agg_value_chain): New function.
>       (ipa_node_removal_hook): Also handle ipa_node_agg_replacements.
>       (ipa_node_duplication_hook): Likewise.
>       (ipa_free_all_structures_after_ipa_cp): Also free ipcp_agg_lattice_pool.
>       (ipa_free_all_structures_after_iinln): Likewise.
>       (ipa_dump_agg_replacement_values): New function.
>       (write_agg_replacement_chain): Likewise.
>       (read_agg_replacement_chain): Likewise.
>       (ipa_prop_write_all_agg_replacement): Likewise.
>       (read_replacements_section): Likewise.
>       (ipa_prop_read_all_agg_replacement): Likewise.
>       (adjust_agg_replacement_values): Likewise.
>       (ipcp_transform_function): Likewise.
>       * ipa-prop.h: Also define heap vector of ipa_agg_jf_item_t and of
>       ipa_agg_jump_function_t.
>       (ipa_node_params): Make lattices an array of ipcp_param_lattices.
>       (ipa_agg_replacement_value): New type and its vector.
>       (ipa_set_node_agg_value_chain) Declare.
>       (ipa_node_agg_replacements): Likewise.
>       (ipa_get_agg_replacements_for_node): New function.
>       (ipcp_agg_lattice_pool): Declare.
>       (ipa_dump_agg_replacement_values): Likewise.
>       (ipa_prop_write_all_agg_replacement): Likewise.
>       (ipa_prop_read_all_agg_replacement): Likewise.
>       (ipcp_transform_function): Likewise.
>       * ipa-inline-analysis.c (estimate_ipcp_clone_size_and_time): Pass around
>       known aggregates and hints.
>       * ipa-inline.h: include ipa-prop.h.
>       (estimate_ipcp_clone_size_and_time): Adjust declaration.
>       * lto-streamer.h (lto_section_type): New item
>       LTO_section_ipcp_transform.
>       * lto-section-in.c (lto_section_name): New element ipcp_trans.
>       * params.def (PARAM_IPA_CP_LOOP_HINT_BONUS): New parameter.
>       * Makefile.in (IPA_INLINE_H): New.  Use everywhee instead of
>       ipa-inline.h.
> 
>       * testsuite/gcc.dg/ipa/ipa-5.c: Adjust.
>       * testsuite/gcc.dg/ipa/ipcp-agg-1.c: New test.
>       * testsuite/gcc.dg/ipa/ipcp-agg-2.c: Likewise.
>       * testsuite/gcc.dg/ipa/ipcp-agg-3.c: Likewise.
>       * testsuite/gcc.dg/ipa/ipcp-agg-4.c: Likewise.
>       * testsuite/gcc.dg/ipa/ipcp-agg-5.c: Likewise.
>       * testsuite/gcc.dg/ipa/ipcp-agg-6.c: Likewise.
>       * testsuite/gfortran.dg/pr48636.f90: Add -fno-ipa-cp.
>       * testsuite/gfortran.dg/pr48636-2.f90: New test.
>       * testsuite/gfortran.dg/pr53787.f90: Likewise.
> 
> 
> Index: src/gcc/ipa-cp.c
> ===================================================================
> *** src.orig/gcc/ipa-cp.c
> --- src/gcc/ipa-cp.c
> + /* The current interface in ipa-inline-analysis requires a pointer vector.
> +    Create it.
> + 
> +    FIXME: That interface should be re-worked, this is slightly silly.  
> Still,
> +    I'd like to discuss how to change it first and this demonstrates the
> +    issue.  */

Hmm, any proposals?

I read the propagation part. It seems a lot of work and did not find anything 
that seems
wrong. You know the code better than anyone else :)

> + /* Function body transformation phase.  */
> + 
> + unsigned int
> + ipcp_transform_function (struct cgraph_node *node)
> + {
> +   VEC (ipa_param_descriptor_t, heap) *descriptors = NULL;
> +   struct param_analysis_info *parms_ainfo;
> +   struct ipa_agg_replacement_value *aggval;
> +   gimple_stmt_iterator gsi;
> +   basic_block bb;
> +   int param_count;
> +   bool cfg_changed = false, something_changed = false;
> + 
> +   gcc_checking_assert (cfun);
> +   gcc_checking_assert (current_function_decl);
> + 
> +   if (dump_file)
> +     fprintf (dump_file, "Modification phase of node %s/%i\n",
> +          cgraph_node_name (node), node->uid);
> + 
> +   aggval = ipa_get_agg_replacements_for_node (node);
> +   if (!aggval)
> +       return 0;
> +   param_count = count_formal_params (node->symbol.decl);
> +   if (param_count == 0)
> +     return 0;
> +   adjust_agg_replacement_values (node, aggval);
> +   if (dump_file)
> +     ipa_dump_agg_replacement_values (dump_file, aggval);
> +   parms_ainfo = XALLOCAVEC (struct param_analysis_info, param_count);
> +   memset (parms_ainfo, 0, sizeof (struct param_analysis_info) * 
> param_count);
> +   VEC_safe_grow_cleared (ipa_param_descriptor_t, heap,
> +                      descriptors, param_count);
> +   ipa_populate_param_decls (node, descriptors);
> + 
> +   FOR_EACH_BB (bb)
> +     for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> +       {
> +     struct ipa_agg_replacement_value *v;
> +     gimple stmt = gsi_stmt (gsi);
> +     tree rhs, val;
> +     HOST_WIDE_INT offset;
> +     int index;
> +     bool by_ref;
> + 
> +     if (is_gimple_debug (stmt)
> +         || !gimple_assign_single_p (stmt)
> +         || !gimple_vuse (stmt))

I think you can check the new gimple_load_p predicate.
The calls/asm statements and friends don't need updating because we always go 
through
an temporary when handling gimple register and you do not propagate anything 
else?

> +       continue;
> +     rhs = gimple_assign_rhs1 (stmt);
> +     if (!is_gimple_reg_type (TREE_TYPE (rhs))
> +         /* V_C_E can do things like convert an array of integers to one
> +                bigger integer and similar things we do not handle below.  */
> +             || TREE_CODE (rhs) == VIEW_CONVERT_EXPR)
> +       continue;

Hmm, shouldn't you look for the inner references (i.e. skip the handled 
components)?
> +     if (!ipa_load_from_parm_agg_1 (descriptors, parms_ainfo, stmt,
> +                                    rhs, &index, &offset, &by_ref))
> +       continue;
> +     for (v = aggval; v; v = v->next)
> +       if (v->index == index
> +           && v->offset == offset)
> +         break;
> +     if (!v)
> +       continue;
> + 
> +     gcc_checking_assert (is_gimple_ip_invariant (v->value));
> +     if (!useless_type_conversion_p (TREE_TYPE (rhs), TREE_TYPE (v->value)))
> +       {
> +         if (fold_convertible_p (TREE_TYPE (rhs), v->value))
> +           val = fold_build1 (NOP_EXPR, TREE_TYPE (rhs), v->value);
> +         else if (TYPE_SIZE (TREE_TYPE (rhs))
> +                  == TYPE_SIZE (TREE_TYPE (v->value)))
> +           val = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (rhs), v->value);
> +         else
> +           {
> +             if (dump_file)
> +               {
> +                 fprintf (dump_file, "    const ");
> +                 print_generic_expr (dump_file, v->value, 0);
> +                 fprintf (dump_file, "  can't be converted to type of ");
> +                 print_generic_expr (dump_file, rhs, 0);
> +                 fprintf (dump_file, "\n");
> +               }
> +             continue;
> +           }
> +       }
> +     else
> +       val = v->value;
> + 
> +     if (dump_file && (dump_flags & TDF_DETAILS))
> +       {
> +         fprintf (dump_file, "Modifying stmt:\n  ");
> +         print_gimple_stmt (dump_file, stmt, 0, 0);
> +       }
> +     gimple_assign_set_rhs_from_tree (&gsi, val);
> +     update_stmt (stmt);
> + 
> +     if (dump_file && (dump_flags & TDF_DETAILS))
> +       {
> +         fprintf (dump_file, "into:\n  ");
> +         print_gimple_stmt (dump_file, stmt, 0, 0);
> +         fprintf (dump_file, "\n");
> +       }
> + 
> +     something_changed = true;
> +     if (maybe_clean_eh_stmt (stmt)
> +         && gimple_purge_dead_eh_edges (gimple_bb (stmt)))
> +       cfg_changed = true;
> +       }
> + 
> +   VEC_replace (ipa_agg_replacement_value_p, ipa_node_agg_replacements,
> +            node->uid, NULL);
> +   free_parms_ainfo (parms_ainfo, param_count);
> +   VEC_free (ipa_param_descriptor_t, heap, descriptors);
> + 
> +   if (!something_changed)
> +     return 0;
> +   else if (cfg_changed)
> +     return TODO_update_ssa | TODO_cleanup_cfg;
> +   else
> +     return TODO_update_ssa;

Hmm, perhaps only virtuals needs to be updated?
> + }
> Index: src/gcc/testsuite/gcc.dg/ipa/ipcp-agg-1.c
> ===================================================================
> *** /dev/null
> --- src/gcc/testsuite/gcc.dg/ipa/ipcp-agg-1.c
> ***************
> *** 0 ****
> --- 1,35 ----
> + /* { dg-do compile } */
> + /* { dg-options "-O3 -fno-ipa-sra -fdump-ipa-cp-details"  } */
> + /* { dg-add-options bind_pic_locally } */
> + 
> + struct S
> + {
> +   int a, b, c;
> + };
> + 
> + void *blah(int, void *);
> + 
> + static void __attribute__ ((noinline))
> + foo (struct S *p)
> + {
> +   int i, c = p->c;
> +   int b = p->b;
> +   void *v = (void *) p;
> + 
> +   for (i= 0; i< c; i++)
> +     v = blah(b + i, v);
> + }
> + 
> + void
> + entry (void)
> + {
> +   struct S s;
> +   s.a = 1;
> +   s.b = 64;
> +   s.c = 32;
> +   foo (&s);
> + }
> + 
> + /* { dg-final { scan-ipa-dump "Creating a specialized node of foo.*for all 
> known contexts" "cp" } } */
> + /* { dg-final { scan-ipa-dump-times "Aggregate replacements:" 2 "cp" } } */
> + /* { dg-final { cleanup-ipa-dump "cp" } } */

I think it would be nice to update at least some of the testcases to verify 
that .optimized dump has propagated
code in it.  (you can perhaps make some constant folding to happen).
I think it would be likely bug that the transform pass will miss its job.
> + 
> + /*
> + Local variables:
> + mode:c++
> + End:
> + */
Probably remove this?
Why the transform part lives in ipa-prop instead of ipa-cp.c?

The patch looks OK, thanks for the hard work!
Honza

Reply via email to