From: Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>
To: Ajit Agarwal <aagarwa1@linux.ibm.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>,
Richard Biener <richard.guenther@gmail.com>,
Segher Boessenkool <segher@kernel.crashing.org>,
Peter Bergner <bergner@linux.ibm.com>,
Jeff Law <jeffreyalaw@gmail.com>
Subject: Re: PING^1 [PATCH v7] tree-ssa-sink: Improve code sinking pass
Date: Tue, 18 Jul 2023 16:38:55 +0530 [thread overview]
Message-ID: <CAAgBjMmoT0R3NzoJuUajEWEzZ1mhLQ+8JKp8saLMMLG041Gn6Q@mail.gmail.com> (raw)
In-Reply-To: <ab576007-4f7d-59a9-5f2e-c4902572c616@linux.ibm.com>
On Tue, 18 Jul 2023 at 13:26, Ajit Agarwal via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
> Ping!
>
> please review.
>
> Thanks & Regards
> Ajit
>
>
> This patch improves code sinking pass to sink statements before call to reduce
> register pressure.
> Review comments are incorporated.
>
> For example :
>
> void bar();
> int j;
> void foo(int a, int b, int c, int d, int e, int f)
> {
> int l;
> l = a + b + c + d +e + f;
> if (a != 5)
> {
> bar();
> j = l;
> }
> }
>
> Code Sinking does the following:
>
> void bar();
> int j;
> void foo(int a, int b, int c, int d, int e, int f)
> {
> int l;
>
> if (a != 5)
> {
> l = a + b + c + d +e + f;
> bar();
> j = l;
> }
> }
>
> Bootstrapped regtested on powerpc64-linux-gnu.
>
> Thanks & Regards
> Ajit
>
>
> tree-ssa-sink: Improve code sinking pass
>
> Currently, code sinking will sink code after function calls. This increases
> register pressure for callee-saved registers. The following patch improves
> code sinking by placing the sunk code before calls in the use block or in
> the immediate dominator of the use blocks.
>
> 2023-06-01 Ajit Kumar Agarwal <aagarwa1@linux.ibm.com>
>
> gcc/ChangeLog:
>
> PR tree-optimization/81953
> * tree-ssa-sink.cc (statement_sink_location): Move statements before
> calls.
> (def_use_same_block): New function.
> (select_best_block): Add heuristics to select the best blocks in the
> immediate post dominator.
>
> gcc/testsuite/ChangeLog:
>
> PR tree-optimization/81953
> * gcc.dg/tree-ssa/ssa-sink-20.c: New testcase.
> * gcc.dg/tree-ssa/ssa-sink-21.c: New testcase.
> ---
> gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c | 15 ++++
> gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c | 19 +++++
> gcc/tree-ssa-sink.cc | 79 ++++++++++++++-------
> 3 files changed, 87 insertions(+), 26 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c
> new file mode 100644
> index 00000000000..d3b79ca5803
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-20.c
> @@ -0,0 +1,15 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-sink-stats" } */
> +void bar();
> +int j;
> +void foo(int a, int b, int c, int d, int e, int f)
> +{
> + int l;
> + l = a + b + c + d +e + f;
> + if (a != 5)
> + {
> + bar();
> + j = l;
> + }
> +}
> +/* { dg-final { scan-tree-dump {l_12\s+=\s+_4\s+\+\s+f_11\(D\);\n\s+bar\s+\(\)} sink1 } } */
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c
> new file mode 100644
> index 00000000000..84e7938c54f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-sink-21.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-sink-stats" } */
> +void bar();
> +int j, x;
> +void foo(int a, int b, int c, int d, int e, int f)
> +{
> + int l;
> + l = a + b + c + d +e + f;
> + if (a != 5)
> + {
> + bar();
> + if (b != 3)
> + x = 3;
> + else
> + x = 5;
> + j = l;
> + }
> +}
> +/* { dg-final { scan-tree-dump {l_13\s+=\s+_4\s+\+\s+f_12\(D\);\n\s+bar\s+\(\)} sink1 } } */
> diff --git a/gcc/tree-ssa-sink.cc b/gcc/tree-ssa-sink.cc
> index b1ba7a2ad6c..113c89d0967 100644
> --- a/gcc/tree-ssa-sink.cc
> +++ b/gcc/tree-ssa-sink.cc
> @@ -171,9 +171,28 @@ nearest_common_dominator_of_uses (def_operand_p def_p, bool *debug_stmts)
> return commondom;
> }
>
> +/* Return TRUE if immediate defs of STMT and STMT are in same
> + * block, FALSE otherwise. */
> +
> +static bool
> +def_use_same_block (gimple *stmt)
> +{
> + def_operand_p def;
> + ssa_op_iter iter;
> +
> + FOR_EACH_SSA_DEF_OPERAND (def, stmt, iter, SSA_OP_DEF)
> + {
> + gimple *def_stmt = SSA_NAME_DEF_STMT (DEF_FROM_PTR (def));
> + if ((gimple_bb (def_stmt) == gimple_bb (stmt)))
> + return true;
Hi Ajit,
Just wondering, won't this always return true since you're iterating over defs,
and def_stmt == stmt ? Sorry, if I misunderstood.
Thanks,
Prathamesh
> + }
> + return false;
> +}
> +
> /* Given EARLY_BB and LATE_BB, two blocks in a path through the dominator
> tree, return the best basic block between them (inclusive) to place
> - statements.
> + statements. The best basic block should be an immediate dominator of
> + best basic block if the use stmt is after the call.
>
> We want the most control dependent block in the shallowest loop nest.
>
> @@ -190,11 +209,22 @@ nearest_common_dominator_of_uses (def_operand_p def_p, bool *debug_stmts)
> static basic_block
> select_best_block (basic_block early_bb,
> basic_block late_bb,
> - gimple *stmt)
> + gimple *stmt,
> + gimple *use)
> {
> basic_block best_bb = late_bb;
> basic_block temp_bb = late_bb;
> int threshold;
> + /* Get the sinking threshold. If the statement to be moved has memory
> + operands, then increase the threshold by 7% as those are even more
> + profitable to avoid, clamping at 100%. */
> + threshold = param_sink_frequency_threshold;
> + if (gimple_vuse (stmt) || gimple_vdef (stmt))
> + {
> + threshold += 7;
> + if (threshold > 100)
> + threshold = 100;
> + }
>
> while (temp_bb != early_bb)
> {
> @@ -203,34 +233,33 @@ select_best_block (basic_block early_bb,
> if (bb_loop_depth (temp_bb) < bb_loop_depth (best_bb))
> best_bb = temp_bb;
>
> + /* Placing a statement before a setjmp-like function would be invalid
> + (it cannot be reevaluated when execution follows an abnormal edge).
> + If we selected a block with abnormal predecessors, just punt. */
> + if (bb_has_abnormal_pred (temp_bb))
> + return early_bb;
> +
> + /* if we have temp_bb post dominated by use block and def
> + and use are not in same block then immediate dominator
> + would be our best block. */
> + if (use
> + && bb_loop_depth(temp_bb) == bb_loop_depth (early_bb)
> + && !(temp_bb->count * 100 >= early_bb->count * threshold)
> + && dominated_by_p (CDI_POST_DOMINATORS, temp_bb, gimple_bb (use))
> + && !def_use_same_block (use))
> + best_bb = temp_bb;
> +
> /* Walk up the dominator tree, hopefully we'll find a shallower
> loop nest. */
> temp_bb = get_immediate_dominator (CDI_DOMINATORS, temp_bb);
> }
>
> - /* Placing a statement before a setjmp-like function would be invalid
> - (it cannot be reevaluated when execution follows an abnormal edge).
> - If we selected a block with abnormal predecessors, just punt. */
> - if (bb_has_abnormal_pred (best_bb))
> - return early_bb;
> -
> /* If we found a shallower loop nest, then we always consider that
> a win. This will always give us the most control dependent block
> within that loop nest. */
> if (bb_loop_depth (best_bb) < bb_loop_depth (early_bb))
> return best_bb;
>
> - /* Get the sinking threshold. If the statement to be moved has memory
> - operands, then increase the threshold by 7% as those are even more
> - profitable to avoid, clamping at 100%. */
> - threshold = param_sink_frequency_threshold;
> - if (gimple_vuse (stmt) || gimple_vdef (stmt))
> - {
> - threshold += 7;
> - if (threshold > 100)
> - threshold = 100;
> - }
> -
> /* If BEST_BB is at the same nesting level, then require it to have
> significantly lower execution frequency to avoid gratuitous movement. */
> if (bb_loop_depth (best_bb) == bb_loop_depth (early_bb)
> @@ -439,7 +468,7 @@ statement_sink_location (gimple *stmt, basic_block frombb,
> if (!dominated_by_p (CDI_DOMINATORS, commondom, frombb))
> return false;
>
> - commondom = select_best_block (frombb, commondom, stmt);
> + commondom = select_best_block (frombb, commondom, stmt, NULL);
>
> if (commondom == frombb)
> return false;
> @@ -456,19 +485,17 @@ statement_sink_location (gimple *stmt, basic_block frombb,
> continue;
> break;
> }
> +
> use = USE_STMT (one_use);
>
> if (gimple_code (use) != GIMPLE_PHI)
> {
> - sinkbb = select_best_block (frombb, gimple_bb (use), stmt);
> + sinkbb = select_best_block (frombb, gimple_bb (use), stmt, use);
>
> if (sinkbb == frombb)
> return false;
>
> - if (sinkbb == gimple_bb (use))
> - *togsi = gsi_for_stmt (use);
> - else
> - *togsi = gsi_after_labels (sinkbb);
> + *togsi = gsi_after_labels (sinkbb);
>
> return true;
> }
> @@ -480,7 +507,7 @@ statement_sink_location (gimple *stmt, basic_block frombb,
> if (!sinkbb)
> return false;
>
> - sinkbb = select_best_block (frombb, sinkbb, stmt);
> + sinkbb = select_best_block (frombb, sinkbb, stmt, NULL);
> if (!sinkbb || sinkbb == frombb)
> return false;
>
> --
> 2.39.3
>
next prev parent reply other threads:[~2023-07-18 11:09 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-27 8:13 Ajit Agarwal
2023-07-18 7:56 ` PING^1 " Ajit Agarwal
2023-07-18 11:08 ` Prathamesh Kulkarni [this message]
2023-07-18 11:16 ` Ajit Agarwal
2023-07-18 11:44 ` Richard Biener
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=CAAgBjMmoT0R3NzoJuUajEWEzZ1mhLQ+8JKp8saLMMLG041Gn6Q@mail.gmail.com \
--to=prathamesh.kulkarni@linaro.org \
--cc=aagarwa1@linux.ibm.com \
--cc=bergner@linux.ibm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jeffreyalaw@gmail.com \
--cc=richard.guenther@gmail.com \
--cc=segher@kernel.crashing.org \
/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).