Re: [tuples][patch] Tuplified pass_sra

["Diego Novillo" <dnovillo at google dot com>]

On Tue, Apr 8, 2008 at 19:57, Oleg Ryjkov <olegr@google.com> wrote:

>  This patch tuplififies pass_sra. Tested on i686-linux. There
>  were couple of testcases fixed in the gcc testsute, which
>  tested for certain optimizations to occur. Also I noticed 2
>  new failures in the fortran testsuite, however after some
>  analyzing, I concluded that pass_sra simply exposed bugs that
>  were already there.

Yes, I've seen these failures in other Fortran tests.  This is
the ICE in output_die, right?  The inliner is also tickling it.
I'll have a look.

> Index: ChangeLog.tuples
>  ===================================================================
>  --- ChangeLog.tuples    (revision 134115)
>  +++ ChangeLog.tuples    (working copy)
>  @@ -1,3 +1,35 @@
>  +2008-04-08  Oleg Ryjkov  <olegr@google.com>
>  +
>  +       * tree-sra.c (sra_build_assignment): Tuplified.
>  +       (mark_all_v_defs): Removed.
>  +       (mark_all_v_defs_seq, mark_all_v_defs_stmt): New functions.
>  +       (sra_walk_expr): Tuplified.
>  +       (sra_walk_tree_list): Removed.
>  +       (sra_walk_call_expr, sra_walk_gimple_asm,
>  +       sra_walk_gimple_modifY_stmt, ): Tuplified and renamed.
>  +       (sra_walk_gimple_call, sra_walk_gimple_asm,
>  +       sra_walk_gimple_assign): New names for tuplified functions.
>  +       (sra_walk_function, find_candidates_for_sra, scan_use, scan_copy,
>  +       scan_ldst, instantiate_element, decide_instantiations,
>  +       mark_all_v_defs_1, sra_build_assignment, sra_build_bf_assignment,
>  +       sra_build_elt_assignment, generate_copy_inout,
>  +       generate_element_copy, generate_element_zero,
>  +       generate_one_element_init, generate_element_init_1): Tuplified.
>  +       (insert_edge_copies): Removed.
>  +       (insert_edge_copies_stmt, insert_edge_copies_seq): New functions.
>  +       (sra_insert_before, sra_insert_after, sra_replace,
>  +       sra_explode_bitfield_assignment, sra_sync_for_bitfield_assignment,
>  +       scalarize_use, scalarize_copy, scalarize_init, mark_no_trap,
>  +       scalarize_ldst, scalarize_parms, scalarize_function): Tuplified.
>  +       (tree_sra, tree_sra_early): Enabled
>  +       (sra_init_cache): Removed extra space.
>  +       * tree-flow.h (insert_edge_copies_stmt, insert_edge_copies_seq):
>  +       Synced with the definitions.
>  +       * gimple.h (gimple_asm_input_op_ptr, gimple_asm_output_op_ptr,
>  +       gimple_return_retval_ptr): New functions.
>  +       * passes.c (init_optimization_passes): Enabled pass_sra,
>  +       pass_early_sra.
>  +

OK with:

>  @@ -2825,70 +2801,71 @@ generate_element_init (struct sra_elt *e
>     abnormal edges will be ignored.  */
>
>   void
>  -insert_edge_copies (tree stmt, basic_block bb)
>  +insert_edge_copies_stmt (gimple stmt, basic_block bb)
>   {
>    edge e;
>    edge_iterator ei;
>  -  bool first_copy;
>
>  -  first_copy = true;
>    FOR_EACH_EDGE (e, ei, bb->succs)
>  -    {
>  -      /* We don't need to insert copies on abnormal edges.  The
>  -        value of the scalar replacement is not guaranteed to
>  -        be valid through an abnormal edge.  */
>  -      if (!(e->flags & EDGE_ABNORMAL))
>  -       {
>  -         if (first_copy)
>  -           {
>  -             bsi_insert_on_edge (e, stmt);
>  -             first_copy = false;
>  -           }
>  -         else
>  -           bsi_insert_on_edge (e, unsave_expr_now (stmt));
>  -       }
>  -    }
>  +    /* We don't need to insert copies on abnormal edges.  The
>  +       value of the scalar replacement is not guaranteed to
>  +       be valid through an abnormal edge.  */
>  +    if (!(e->flags & EDGE_ABNORMAL))
>  +      gsi_insert_on_edge (e, gimple_deep_copy (stmt));

s/gimple_deep_copy/gimple_copy/  (this was changed after your
patch).

Why are you undoing the logic for not creating a copy the first
time?  That should be kept the same.


>  +          /* RHS must be a single operand. */
>  +          gcc_assert (!gimple_assign_rhs2 (stmt));

s/!gimple_assign_rhs2/gimple_assign_single_p/

>  @@ -1945,6 +1945,16 @@ gimple_asm_input_op (const_gimple gs, si
>    return gimple_op (gs, index);
>   }
>
>  +/* Return input operand INDEX of GIMPLE_ASM GS.  */

s/input operand/a pointer to input operand/

>  @@ -1968,6 +1978,16 @@ gimple_asm_output_op (const_gimple gs, s
>    return gimple_op (gs, index + gs->gimple_asm.ni);
>   }
>
>  +/* Return output operand INDEX of GIMPLE_ASM GS.  */

s/output operand/a pointer to output operand/

>  @@ -3030,6 +3050,16 @@ gimple_omp_continue_set_control_use (gim
>
>   /* Return the return value for GIMPLE_RETURN GS.  */
>
>  +static inline tree *
>  +gimple_return_retval_ptr (const_gimple gs)

Needs comment.


Thanks.  Diego.