Fix parloop issues (PR tree-optimization/46099)

Hi!

This patch fixes two issues:
1) -g shouldn't affect code generation, so we certainly don't want
   &var in DEBUG stmts result in extra vars passed from function to the
   parallel fn - fixed by first going through all non-debug stmts,
   then if there are any debug stmts walking them in second pass, without
   adding anything into the hash table and without gimplifying into new
   stmts
2) since MEM_REF introduction take_address_of didn't really cache things,
   as possibly needed insns were always being added on entry edge.
   We only need setting of the SSA_NAME in cache on the entry edge, the
   rest should be just added immediately before current stmt if needed

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2010-11-04  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/46099
	* tree-parloops.c (take_address_of): Add GSI argument.  Return NULL
	if it is NULL and uid wasn't found in the hash table.  Just fold the
	result if it is NULL otherwise.  Insert other potentially needed
	stmts right before current stmt instead of on the entry edge.
	(struct elv_data): Add gsi and reset fields.
	(eliminate_local_variables_1): Adjust caller.  If take_address_of
	failed for debug stmt, set dta->reset and return.
	(eliminate_local_variables_stmt): Change STMT argument for GSI,
	pass GSI through to the callback, handle resetting of debug stmts.
	(eliminate_local_variables): Adjust caller.  Process debug stmts
	in second phase.

	* gcc.dg/autopar/pr46099.c: New test.


	Jakub

On Thu, Nov 4, 2010 at 7:57 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> This patch fixes two issues:
> 1) -g shouldn't affect code generation, so we certainly don't want
>   &var in DEBUG stmts result in extra vars passed from function to the
>   parallel fn - fixed by first going through all non-debug stmts,
>   then if there are any debug stmts walking them in second pass, without
>   adding anything into the hash table and without gimplifying into new
>   stmts
> 2) since MEM_REF introduction take_address_of didn't really cache things,
>   as possibly needed insns were always being added on entry edge.
>   We only need setting of the SSA_NAME in cache on the entry edge, the
>   rest should be just added immediately before current stmt if needed
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2010-11-04  Jakub Jelinek  <jakub@redhat.com>
>
>        PR tree-optimization/46099
>        * tree-parloops.c (take_address_of): Add GSI argument.  Return NULL
>        if it is NULL and uid wasn't found in the hash table.  Just fold the
>        result if it is NULL otherwise.  Insert other potentially needed
>        stmts right before current stmt instead of on the entry edge.
>        (struct elv_data): Add gsi and reset fields.
>        (eliminate_local_variables_1): Adjust caller.  If take_address_of
>        failed for debug stmt, set dta->reset and return.
>        (eliminate_local_variables_stmt): Change STMT argument for GSI,
>        pass GSI through to the callback, handle resetting of debug stmts.
>        (eliminate_local_variables): Adjust caller.  Process debug stmts
>        in second phase.
>
>        * gcc.dg/autopar/pr46099.c: New test.
>
> --- gcc/tree-parloops.c.jj      2010-11-01 09:07:22.000000000 +0100
> +++ gcc/tree-parloops.c 2010-11-04 15:59:10.000000000 +0100
> @@ -314,10 +314,12 @@ loop_has_blocks_with_irreducible_flag (s
>  /* Assigns the address of OBJ in TYPE to an ssa name, and returns this name.
>    The assignment statement is placed on edge ENTRY.  DECL_ADDRESS maps decls
>    to their addresses that can be reused.  The address of OBJ is known to
> -   be invariant in the whole function.  */
> +   be invariant in the whole function.  Other needed statements are placed
> +   right before GSI.  */
>
>  static tree
> -take_address_of (tree obj, tree type, edge entry, htab_t decl_address)
> +take_address_of (tree obj, tree type, edge entry, htab_t decl_address,
> +                gimple_stmt_iterator *gsi)
>  {
>   int uid;
>   void **dslot;
> @@ -346,6 +348,8 @@ take_address_of (tree obj, tree type, ed
>   dslot = htab_find_slot_with_hash (decl_address, &ielt, uid, INSERT);
>   if (!*dslot)
>     {
> +      if (gsi == NULL)
> +       return NULL;
>       addr = TREE_OPERAND (*var_p, 0);
>       bvar = create_tmp_var (TREE_TYPE (addr),
>                             get_name (TREE_OPERAND
> @@ -366,17 +370,20 @@ take_address_of (tree obj, tree type, ed
>
>   /* Express the address in terms of the canonical SSA name.  */
>   TREE_OPERAND (*var_p, 0) = name;
> +  if (gsi == NULL)
> +    return build_fold_addr_expr_with_type (obj, type);
> +
>   name = force_gimple_operand (build_addr (obj, current_function_decl),
>                               &stmts, true, NULL_TREE);
>   if (!gimple_seq_empty_p (stmts))
> -    gsi_insert_seq_on_edge_immediate (entry, stmts);
> +    gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT);
>
>   if (!useless_type_conversion_p (type, TREE_TYPE (name)))
>     {
>       name = force_gimple_operand (fold_convert (type, name), &stmts, true,
>                                   NULL_TREE);
>       if (!gimple_seq_empty_p (stmts))
> -       gsi_insert_seq_on_edge_immediate (entry, stmts);
> +       gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT);
>     }
>
>   return name;
> @@ -438,7 +445,9 @@ struct elv_data
>   struct walk_stmt_info info;
>   edge entry;
>   htab_t decl_address;
> +  gimple_stmt_iterator *gsi;
>   bool changed;
> +  bool reset;
>  };
>
>  /* Eliminates references to local variables in *TP out of the single
> @@ -462,7 +471,14 @@ eliminate_local_variables_1 (tree *tp, i
>
>       type = TREE_TYPE (t);
>       addr_type = build_pointer_type (type);
> -      addr = take_address_of (t, addr_type, dta->entry, dta->decl_address);
> +      addr = take_address_of (t, addr_type, dta->entry, dta->decl_address,
> +                             dta->gsi);
> +      if (dta->gsi == NULL && addr == NULL_TREE)
> +       {
> +         dta->reset = true;
> +         return NULL_TREE;
> +       }
> +
>       *tp = build_simple_mem_ref (addr);
>
>       dta->changed = true;
> @@ -492,7 +508,13 @@ eliminate_local_variables_1 (tree *tp, i
>        return NULL_TREE;
>
>       addr_type = TREE_TYPE (t);
> -      addr = take_address_of (obj, addr_type, dta->entry, dta->decl_address);
> +      addr = take_address_of (obj, addr_type, dta->entry, dta->decl_address,
> +                             dta->gsi);
> +      if (dta->gsi == NULL && addr == NULL_TREE)
> +       {
> +         dta->reset = true;
> +         return NULL_TREE;
> +       }
>       *tp = addr;
>
>       dta->changed = true;
> @@ -505,27 +527,40 @@ eliminate_local_variables_1 (tree *tp, i
>   return NULL_TREE;
>  }
>
> -/* Moves the references to local variables in STMT out of the single
> +/* Moves the references to local variables in STMT at *GSI out of the single
>    entry single exit region starting at ENTRY.  DECL_ADDRESS contains
>    addresses of the references that had their address taken
>    already.  */
>
>  static void
> -eliminate_local_variables_stmt (edge entry, gimple stmt,
> +eliminate_local_variables_stmt (edge entry, gimple_stmt_iterator *gsi,
>                                htab_t decl_address)
>  {
>   struct elv_data dta;
> +  gimple stmt = gsi_stmt (*gsi);
>
>   memset (&dta.info, '\0', sizeof (dta.info));
>   dta.entry = entry;
>   dta.decl_address = decl_address;
>   dta.changed = false;
> +  dta.reset = false;
>
>   if (gimple_debug_bind_p (stmt))
> -    walk_tree (gimple_debug_bind_get_value_ptr (stmt),
> -              eliminate_local_variables_1, &dta.info, NULL);
> +    {
> +      dta.gsi = NULL;
> +      walk_tree (gimple_debug_bind_get_value_ptr (stmt),
> +                eliminate_local_variables_1, &dta.info, NULL);
> +      if (dta.reset)
> +       {
> +         gimple_debug_bind_reset_value (stmt);
> +         dta.changed = true;
> +       }
> +    }
>   else
> -    walk_gimple_op (stmt, eliminate_local_variables_1, &dta.info);
> +    {
> +      dta.gsi = gsi;
> +      walk_gimple_op (stmt, eliminate_local_variables_1, &dta.info);
> +    }
>
>   if (dta.changed)
>     update_stmt (stmt);
> @@ -549,6 +584,7 @@ eliminate_local_variables (edge entry, e
>   VEC (basic_block, heap) *body = VEC_alloc (basic_block, heap, 3);
>   unsigned i;
>   gimple_stmt_iterator gsi;
> +  bool has_debug_stmt = false;
>   htab_t decl_address = htab_create (10, int_tree_map_hash, int_tree_map_eq,
>                                     free);
>   basic_block entry_bb = entry->src;
> @@ -559,8 +595,17 @@ eliminate_local_variables (edge entry, e
>   FOR_EACH_VEC_ELT (basic_block, body, i, bb)
>     if (bb != entry_bb && bb != exit_bb)
>       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> -       eliminate_local_variables_stmt (entry, gsi_stmt (gsi),
> -                                       decl_address);
> +       if (gimple_debug_bind_p (gsi_stmt (gsi)))
> +         has_debug_stmt = true;
> +       else
> +         eliminate_local_variables_stmt (entry, &gsi, decl_address);
> +
> +  if (has_debug_stmt)
> +    FOR_EACH_VEC_ELT (basic_block, body, i, bb)
> +      if (bb != entry_bb && bb != exit_bb)
> +       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
> +         if (gimple_debug_bind_p (gsi_stmt (gsi)))
> +           eliminate_local_variables_stmt (entry, &gsi, decl_address);
>
>   htab_delete (decl_address);
>   VEC_free (basic_block, heap, body);
> --- gcc/testsuite/gcc.dg/autopar/pr46099.c.jj   2010-11-04 16:08:37.000000000 +0100
> +++ gcc/testsuite/gcc.dg/autopar/pr46099.c      2010-11-04 16:03:34.000000000 +0100
> @@ -0,0 +1,47 @@
> +/* PR tree-optimization/46099 */
> +/* { dg-do compile } */
> +/* { dg-options "-ftree-parallelize-loops=2 -fcompare-debug -O" } */
> +
> +static inline void
> +bar (int *i)
> +{
> +  int j = *i;
> +}
> +
> +void baz (int *, int *, int *);
> +
> +void
> +f1 (int n)
> +{
> +  int i;
> +  for (i = 0; i < n; i++)
> +    bar (&i);
> +}
> +
> +void
> +f2 (int n)
> +{
> +  int i;
> +  int a[10000], b[10000], c[10000];
> +  baz (a, b, c);
> +  for (i = 0; i < n; i++)
> +    {
> +      void *p = c;
> +      a[i] = b[i] + c[i];
> +    }
> +  baz (a, b, c);
> +}
> +
> +void
> +f3 (int n)
> +{
> +  int i;
> +  int a[10000], b[10000], c[10000];
> +  baz (a, b, c);
> +  for (i = 0; i < n; i++)
> +    {
> +      a[i] = b[i] + c[i];
> +      void *p = c;
> +    }
> +  baz (a, b, c);
> +}
>
>        Jakub
>