From: "Daniel Berlin" <dberlin@dberlin.org>
To: "Richard Guenther" <rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Fix PR34677, enhance load-PRE
Date: Mon, 10 Mar 2008 17:09:00 -0000 [thread overview]
Message-ID: <4aca3dc20803101008o216060c8n4e877da830a9de54@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0803101643330.4133@zhemvz.fhfr.qr>
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" } } */
>
next prev parent reply other threads:[~2008-03-10 17:09 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-03-10 15:47 Richard Guenther
2008-03-10 17:09 ` Daniel Berlin [this message]
2008-03-10 19:57 ` H.J. Lu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4aca3dc20803101008o216060c8n4e877da830a9de54@mail.gmail.com \
--to=dberlin@dberlin.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).