This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix PR34677, enhance load-PRE


On Mon, Mar 10, 2008 at 11:46 AM, Richard Guenther <rguenther@suse.de> wrote:
>
>  Currently fake loads are only inserted for stores through plain
>  INDIRECT_REFs.  The following patch makes that (and the load-PRE
>  opportunities exposed by it) also work for all component references.
>  Instead of enhancing poolify_tree and friends I got rid of it
>  (the reference tree itself is shared as we replace it by an SSA_NAME
>  or remove the fake stmt again, so the GC overhead is a modify stmt
>  only which isn't too bad -- in the end this particular pooling looks
>  like more maintainance overhead than worth it).
>
>  Bootstrapped and tested on x86_64-unknown-linux-gnu.  Ok for trunk?

Fine by me.
We originally poolified trees because there were cases we would insert
10k+ fake stmts.
GC overhead was pretty high, but pool usage was very minimal.

It also kept PRE's GC usage down :)
>
>  Thanks,
>  Richard.
>
>  2008-03-10  Richard Guenther  <rguenther@suse.de>
>
>         PR tree-optimization/34677
>         * tree-ssa-pre.c (modify_expr_node_pool): Remove.
>         (poolify_tree): Likewise.
>         (modify_expr_template): Likewise.
>         (poolify_modify_stmt): Likewise.
>         (insert_fake_stores): Handle all component-ref style stores
>         in addition to INDIRECT_REF.  Also handle complex types.
>         Do not poolify the inserted load.
>         (realify_fake_stores): Do not rebuild the tree but only
>         make it a SSA_NAME copy.
>         (init_pre): Remove initialzation of modify_expr_template.
>         Do not allocate modify_expr_node_pool.
>         (fini_pre): Do not free modify_expr_node_pool.
>
>  Index: gcc/tree-ssa-pre.c
>  ===================================================================
>  *** gcc/tree-ssa-pre.c.orig     2008-03-10 15:30:31.000000000 +0100
>  --- gcc/tree-ssa-pre.c  2008-03-10 15:36:32.000000000 +0100
>  *************** static alloc_pool binary_node_pool;
>  *** 394,400 ****
>   static alloc_pool unary_node_pool;
>   static alloc_pool reference_node_pool;
>   static alloc_pool comparison_node_pool;
>  - static alloc_pool modify_expr_node_pool;
>   static bitmap_obstack grand_bitmap_obstack;
>
>   /* We can't use allocation pools to hold temporary CALL_EXPR objects, since
>  --- 394,399 ----
>  *************** create_value_expr_from (tree expr, basic
>  *** 3046,3109 ****
>     return vexpr;
>   }
>
>  - /* Return a copy of NODE that is stored in the temporary alloc_pool's.
>  -    This is made recursively true, so that the operands are stored in
>  -    the pool as well.  */
>  -
>  - static tree
>  - poolify_tree (tree node)
>  - {
>  -   switch  (TREE_CODE (node))
>  -     {
>  -     case INDIRECT_REF:
>  -       {
>  -       tree temp = (tree) pool_alloc (reference_node_pool);
>  -       memcpy (temp, node, tree_size (node));
>  -       TREE_OPERAND (temp, 0) = poolify_tree (TREE_OPERAND (temp, 0));
>  -       return temp;
>  -       }
>  -       break;
>  -     case GIMPLE_MODIFY_STMT:
>  -       {
>  -       tree temp = (tree) pool_alloc (modify_expr_node_pool);
>  -       memcpy (temp, node, tree_size (node));
>  -       GIMPLE_STMT_OPERAND (temp, 0) =
>  -         poolify_tree (GIMPLE_STMT_OPERAND (temp, 0));
>  -       GIMPLE_STMT_OPERAND (temp, 1) =
>  -         poolify_tree (GIMPLE_STMT_OPERAND (temp, 1));
>  -       return temp;
>  -       }
>  -       break;
>  -     case SSA_NAME:
>  -     case INTEGER_CST:
>  -     case STRING_CST:
>  -     case REAL_CST:
>  -     case FIXED_CST:
>  -     case PARM_DECL:
>  -     case VAR_DECL:
>  -     case RESULT_DECL:
>  -       return node;
>  -     default:
>  -       gcc_unreachable ();
>  -     }
>  - }
>  -
>  - static tree modify_expr_template;
>  -
>  - /* Allocate a GIMPLE_MODIFY_STMT with TYPE, and operands OP1, OP2 in the
>  -    alloc pools and return it.  */
>  - static tree
>  - poolify_modify_stmt (tree op1, tree op2)
>  - {
>  -   if (modify_expr_template == NULL)
>  -     modify_expr_template = build_gimple_modify_stmt (op1, op2);
>  -
>  -   GIMPLE_STMT_OPERAND (modify_expr_template, 0) = op1;
>  -   GIMPLE_STMT_OPERAND (modify_expr_template, 1) = op2;
>  -
>  -   return poolify_tree (modify_expr_template);
>  - }
>  -
>
>   /* For each real store operation of the form
>      *a = <value> that we see, create a corresponding fake store of the
>  --- 3045,3050 ----
>  *************** insert_fake_stores (void)
>  *** 3134,3149 ****
>              virtual uses occur in abnormal phis.  */
>
>           if (TREE_CODE (stmt) == GIMPLE_MODIFY_STMT
>  !             && TREE_CODE (GIMPLE_STMT_OPERAND (stmt, 0)) == INDIRECT_REF
>  !             && !AGGREGATE_TYPE_P (TREE_TYPE (GIMPLE_STMT_OPERAND (stmt, 0)))
>  !             && TREE_CODE (TREE_TYPE (GIMPLE_STMT_OPERAND
>  !                                       (stmt, 0))) != COMPLEX_TYPE)
>             {
>               ssa_op_iter iter;
>               def_operand_p defp;
>               tree lhs = GIMPLE_STMT_OPERAND (stmt, 0);
>               tree rhs = GIMPLE_STMT_OPERAND (stmt, 1);
>  !             tree new_tree;
>               bool notokay = false;
>
>               FOR_EACH_SSA_DEF_OPERAND (defp, stmt, iter, SSA_OP_VIRTUAL_DEFS)
>  --- 3075,3089 ----
>              virtual uses occur in abnormal phis.  */
>
>           if (TREE_CODE (stmt) == GIMPLE_MODIFY_STMT
>  !             && (TREE_CODE (GIMPLE_STMT_OPERAND (stmt, 0)) == INDIRECT_REF
>  !                 || handled_component_p (GIMPLE_STMT_OPERAND (stmt, 0)))
>  !             && !AGGREGATE_TYPE_P (TREE_TYPE (GIMPLE_STMT_OPERAND (stmt, 0))))
>             {
>               ssa_op_iter iter;
>               def_operand_p defp;
>               tree lhs = GIMPLE_STMT_OPERAND (stmt, 0);
>               tree rhs = GIMPLE_STMT_OPERAND (stmt, 1);
>  !             tree new_tree, new_lhs;
>               bool notokay = false;
>
>               FOR_EACH_SSA_DEF_OPERAND (defp, stmt, iter, SSA_OP_VIRTUAL_DEFS)
>  *************** insert_fake_stores (void)
>  *** 3162,3176 ****
>               if (!storetemp || TREE_TYPE (rhs) != TREE_TYPE (storetemp))
>                 {
>                   storetemp = create_tmp_var (TREE_TYPE (rhs), "storetmp");
>  !                 if (TREE_CODE (TREE_TYPE (storetemp)) == VECTOR_TYPE)
>                     DECL_GIMPLE_REG_P (storetemp) = 1;
>                   get_var_ann (storetemp);
>                 }
>
>  !             new_tree = poolify_modify_stmt (storetemp, lhs);
>
>  -             lhs = make_ssa_name (storetemp, new_tree);
>  -             GIMPLE_STMT_OPERAND (new_tree, 0) = lhs;
>               create_ssa_artificial_load_stmt (new_tree, stmt, false);
>
>               NECESSARY (new_tree) = 0;
>  --- 3102,3117 ----
>               if (!storetemp || TREE_TYPE (rhs) != TREE_TYPE (storetemp))
>                 {
>                   storetemp = create_tmp_var (TREE_TYPE (rhs), "storetmp");
>  !                 if (TREE_CODE (TREE_TYPE (storetemp)) == VECTOR_TYPE
>  !                     || TREE_CODE (TREE_TYPE (storetemp)) == COMPLEX_TYPE)
>                     DECL_GIMPLE_REG_P (storetemp) = 1;
>                   get_var_ann (storetemp);
>                 }
>
>  !             new_tree = build_gimple_modify_stmt (NULL_TREE, lhs);
>  !             new_lhs = make_ssa_name (storetemp, new_tree);
>  !             GIMPLE_STMT_OPERAND (new_tree, 0) = new_lhs;
>
>               create_ssa_artificial_load_stmt (new_tree, stmt, false);
>
>               NECESSARY (new_tree) = 0;
>  *************** realify_fake_stores (void)
>  *** 3196,3220 ****
>       {
>         if (NECESSARY (stmt))
>         {
>  !         block_stmt_iterator bsi;
>  !         tree newstmt, tmp;
>
>           /* Mark the temp variable as referenced */
>           add_referenced_var (SSA_NAME_VAR (GIMPLE_STMT_OPERAND (stmt, 0)));
>
>  !         /* Put the new statement in GC memory, fix up the
>  !            SSA_NAME_DEF_STMT on it, and then put it in place of
>  !            the old statement before the store in the IR stream
>              as a plain ssa name copy.  */
>           bsi = bsi_for_stmt (stmt);
>           bsi_prev (&bsi);
>  !         tmp = GIMPLE_STMT_OPERAND (bsi_stmt (bsi), 1);
>  !         newstmt = build_gimple_modify_stmt (GIMPLE_STMT_OPERAND (stmt, 0),
>  !                                             tmp);
>  !         SSA_NAME_DEF_STMT (GIMPLE_STMT_OPERAND (newstmt, 0)) = newstmt;
>  !         bsi_insert_before (&bsi, newstmt, BSI_SAME_STMT);
>  !         bsi = bsi_for_stmt (stmt);
>  !         bsi_remove (&bsi, true);
>         }
>         else
>         release_defs (stmt);
>  --- 3137,3157 ----
>       {
>         if (NECESSARY (stmt))
>         {
>  !         block_stmt_iterator bsi, bsi2;
>  !         tree rhs;
>
>           /* Mark the temp variable as referenced */
>           add_referenced_var (SSA_NAME_VAR (GIMPLE_STMT_OPERAND (stmt, 0)));
>
>  !         /* Put the statement before the store in the IR stream
>              as a plain ssa name copy.  */
>           bsi = bsi_for_stmt (stmt);
>           bsi_prev (&bsi);
>  !         rhs = GIMPLE_STMT_OPERAND (bsi_stmt (bsi), 1);
>  !         GIMPLE_STMT_OPERAND (stmt, 1) = rhs;
>  !         bsi2 = bsi_for_stmt (stmt);
>  !         bsi_remove (&bsi2, true);
>  !         bsi_insert_before (&bsi, stmt, BSI_SAME_STMT);
>         }
>         else
>         release_defs (stmt);
>  *************** init_pre (bool do_fre)
>  *** 3818,3828 ****
>                                            tree_code_size (ARRAY_REF), 30);
>     comparison_node_pool = create_alloc_pool ("Comparison tree nodes",
>                                             tree_code_size (EQ_EXPR), 30);
>  -   modify_expr_node_pool = create_alloc_pool ("GIMPLE_MODIFY_STMT nodes",
>  -                                            tree_code_size (GIMPLE_MODIFY_STMT),
>  -                                            30);
>     obstack_init (&temp_call_expr_obstack);
>  -   modify_expr_template = NULL;
>
>     FOR_ALL_BB (bb)
>       {
>  --- 3755,3761 ----
>  *************** fini_pre (void)
>  *** 3854,3860 ****
>     free_alloc_pool (reference_node_pool);
>     free_alloc_pool (unary_node_pool);
>     free_alloc_pool (comparison_node_pool);
>  -   free_alloc_pool (modify_expr_node_pool);
>     htab_delete (phi_translate_table);
>     remove_fake_exit_edges ();
>
>  --- 3787,3792 ----
>  Index: gcc/testsuite/gcc.dg/tree-ssa/loadpre23.c
>  ===================================================================
>  *** /dev/null   1970-01-01 00:00:00.000000000 +0000
>  --- gcc/testsuite/gcc.dg/tree-ssa/loadpre23.c   2008-03-10 15:31:42.000000000 +0100
>  ***************
>  *** 0 ****
>  --- 1,25 ----
>  + /* { dg-do compile } */
>  + /* { dg-options "-O2 -fdump-tree-pre-stats" } */
>  +
>  + struct {
>  +   int a;
>  +   int large[100];
>  + } x;
>  +
>  + int foo(int argc)
>  + {
>  +   int b;
>  +   int c;
>  +   int i;
>  +   int d, e;
>  +
>  +   for (i = 0; i < argc; i++)
>  +     {
>  +       e = x.a;
>  +       x.a = 9;
>  +     }
>  +   return d + e;
>  + }
>  +
>  + /* { dg-final { scan-tree-dump-times "Eliminated: 1" 1 "pre"  } } */
>  + /* { dg-final { cleanup-tree-dump "pre" } } */
>  Index: gcc/testsuite/gcc.dg/tree-ssa/loadpre24.c
>  ===================================================================
>  *** /dev/null   1970-01-01 00:00:00.000000000 +0000
>  --- gcc/testsuite/gcc.dg/tree-ssa/loadpre24.c   2008-03-10 15:37:16.000000000 +0100
>  ***************
>  *** 0 ****
>  --- 1,24 ----
>  + /* { dg-do compile } */
>  + /* { dg-options "-O2 -fdump-tree-pre-stats" } */
>  +
>  + int a;
>  +
>  + int foo(int argc)
>  + {
>  +   int b;
>  +   int c;
>  +   int i;
>  +   int d, e;
>  +
>  +   for (i = 0; i < argc; i++)
>  +     {
>  +       e = a;
>  +       a = 9;
>  +     }
>  +   return d + e;
>  + }
>  +
>  + /* PRE of globals doesn't work.  */
>  +
>  + /* { dg-final { scan-tree-dump-times "Eliminated: 1" 1 "pre" { xfail *-*-* } } } */
>  + /* { dg-final { cleanup-tree-dump "pre" } } */
>  Index: gcc/testsuite/gcc.dg/tree-ssa/loadpre25.c
>  ===================================================================
>  *** /dev/null   1970-01-01 00:00:00.000000000 +0000
>  --- gcc/testsuite/gcc.dg/tree-ssa/loadpre25.c   2008-03-10 15:36:50.000000000 +0100
>  ***************
>  *** 0 ****
>  --- 1,20 ----
>  + /* { dg-do compile } */
>  + /* { dg-options "-O2 -fdump-tree-pre-stats" } */
>  + struct X { int i; };
>  + int foo(struct X *a, int argc)
>  + {
>  +   int b;
>  +   int c;
>  +   int i;
>  +   int d, e;
>  +
>  +   for (i = 0; i < argc; i++)
>  +     {
>  +       e = a->i;
>  +       a->i = 9;
>  +     }
>  +   return d + e;
>  + }
>  +
>  + /* { dg-final { scan-tree-dump-times "Eliminated: 1" 1 "pre"  } } */
>  + /* { dg-final { cleanup-tree-dump "pre" } } */
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]