On Thu, 10 Sep 2026, Aldy Hernandez wrote:
> We recompute the per-path stats at every node of the search, which is
> rather expensive it turns out. There was a note hidden in the
> original code saying we should do this incrementally, and this commit
> does so and gives us a 7.8% improvement in the threader.
>
> Tested on ppc64le Linux.
>
> Over a corpus of .ii files from a stage1 bootstrap the assembly is
> identical. Also, I instrumented the compiler before and after this
> patch, and verified that the path and stats are identical, so
> really... a NFC.
>
> gcc/ChangeLog:
>
> * tree-ssa-threadbackward.cc (back_threader_profitability): Add
> push_bb, pop_bb, m_loop, unwind_state and m_unwind; add n_insns
> to path_stats.
> (back_threader_profitability::back_threader_profitability):
> Initialize m_loop and m_stats.
> (back_threader_profitability::account_bb): Return the number of
> insns in the block.
> (back_threader_profitability::push_bb): New.
> (back_threader_profitability::pop_bb): New.
> (back_threader_profitability::possibly_profitable_path_p): Use
> the accumulated stats.
> (back_threader::find_paths_to_names): Call push_bb/pop_bb.
> ---
> gcc/tree-ssa-threadbackward.cc | 110 ++++++++++++++++++++-------------
> 1 file changed, 68 insertions(+), 42 deletions(-)
>
> diff --git a/gcc/tree-ssa-threadbackward.cc b/gcc/tree-ssa-threadbackward.cc
> index 1ae32312968..03a7e1db50c 100644
> --- a/gcc/tree-ssa-threadbackward.cc
> +++ b/gcc/tree-ssa-threadbackward.cc
> @@ -62,24 +62,40 @@ class back_threader_profitability
> {
> public:
> back_threader_profitability (bool speed_p, gimple *stmt);
> + void push_bb (const vec<basic_block> &);
> + void pop_bb ();
> bool possibly_profitable_path_p (const vec<basic_block> &, bool *);
> bool profitable_path_p (const vec<basic_block> &,
> edge taken, bool *irreducible_loop);
> private:
> - void account_bb (basic_block, bool);
> + int account_bb (basic_block, bool);
> const bool m_speed_p;
> int m_exit_jump_benefit;
> bool m_threaded_multiway_branch;
> - // The following are computed by possibly_profitable_path_p
> + // The loop the path starts in, i.e. m_path[0]->loop_father.
> + class loop *m_loop;
> + // The following are accumulated by push_bb as the path grows and
> + // restored by pop_bb as it shrinks.
> struct path_stats
> {
> + int n_insns;
> bool threaded_through_latch;
> bool multiway_branch_in_path;
> bool contains_hot_bb;
> bool unprofitable_bb;
> };
> path_stats m_stats;
> + // Number of insns on the path, less the branch we get to remove.
> int m_n_insns;
> + // One entry per push. These are the stats as they stood before that push,
> + // and the insns of the block. Basically the entry pushed for m_path[i]
> + // accounts m_path[i - 1].
> + struct unwind_state
> + {
> + path_stats stats;
> + int bb_insns;
> + };
the m_n_insns / path_stats.n_insins and unwind_state bb_insns thing
is a bit confusing? Why is path_stats.n_insns not the overall
number of insns on the path and simply pushed/popped like all
the rest?
> + auto_vec<unwind_state, 20> m_unwind;
> };
>
> back_threader_profitability::back_threader_profitability (bool speed_p,
> @@ -92,15 +108,19 @@ back_threader_profitability::back_threader_profitability
> (bool speed_p,
> // particular it estimates further DCE from eliminating the exit
> // control stmt.
> m_exit_jump_benefit = estimate_num_insns (last, &eni_size_weights);
> + m_loop = NULL;
> + m_stats = path_stats ();
> }
>
> /* Account for BB in the cumulative stats for the path being threaded.
> CHECK_MULTIWAY is true for all blocks except the block whose branch
> - we are going to eliminate. */
> + we are going to eliminate. Return the number of insns in BB. */
>
> -void
> +int
> back_threader_profitability::account_bb (basic_block bb, bool check_multiway)
> {
> + int n_insns = 0;
> +
> if (!m_stats.contains_hot_bb && m_speed_p)
> m_stats.contains_hot_bb |= optimize_bb_for_speed_p (bb);
>
> @@ -117,12 +137,12 @@ back_threader_profitability::account_bb (basic_block
> bb, bool check_multiway)
> || gimple_call_builtin_p (stmt, BUILT_IN_CONSTANT_P))
> {
> m_stats.unprofitable_bb = true;
> - return;
> + return n_insns;
> }
> /* Do not count empty statements and labels. */
> if (gimple_code (stmt) != GIMPLE_NOP
> && !is_gimple_debug (stmt))
> - m_n_insns += estimate_num_insns (stmt, &eni_size_weights);
> + n_insns += estimate_num_insns (stmt, &eni_size_weights);
> }
>
> /* We do not look at the block with the threaded branch in this loop.
> @@ -136,6 +156,42 @@ back_threader_profitability::account_bb (basic_block bb,
> bool check_multiway)
> || gimple_code (last) == GIMPLE_GOTO))
> m_stats.multiway_branch_in_path = true;
> }
> +
> + return n_insns;
> +}
> +
> +/* Update the stats after a block has been appended to PATH. */
> +
> +void
> +back_threader_profitability::push_bb (const vec<basic_block> &path)
> +{
> + unwind_state state = { m_stats, /*bb_insns=*/0 };
> +
> + unsigned n = path.length ();
> + if (n == 1)
> + m_loop = path[0]->loop_father;
> + else
> + {
> + /* Appending a block makes the previous entry block part of the copied
> + path, so that's where to account for now. */
> + unsigned copied = n - 2;
> + bool check_multiway = copied > 0;
> + state.bb_insns = account_bb (path[copied], check_multiway);
> + m_stats.n_insns += state.bb_insns;
> + }
> + m_unwind.safe_push (state);
> +
> + /* Note if we thread through the latch, we will want to include the
> + last entry in the array when determining if we thread through the
> + loop latch. */
> + if (m_loop->latch == path[n - 1])
> + m_stats.threaded_through_latch = true;
> +}
> +
> +void
> +back_threader_profitability::pop_bb ()
> +{
> + m_stats = m_unwind.pop ().stats;
> }
>
> // Back threader flags.
> @@ -440,6 +496,7 @@ back_threader::find_paths_to_names (basic_block bb,
> bitmap interesting,
> return;
>
> m_path.safe_push (bb);
> + profit.push_bb (m_path);
>
> // Try to resolve the path without looking back. Avoid resolving paths
> // we know are large but are not (yet) recognized as Finite State Machine.
> @@ -576,6 +633,7 @@ back_threader::find_paths_to_names (basic_block bb,
> bitmap interesting,
> param_max_jump_thread_paths);
>
> // Reset things to their original state.
> + profit.pop_bb ();
> m_path.pop ();
> m_visited_bbs.remove (bb);
> }
> @@ -685,42 +743,10 @@ back_threader_profitability::possibly_profitable_path_p
> if (m_path.length () <= 1)
> return false;
>
> - loop_p loop = m_path[0]->loop_father;
> -
> - // We recompute the following, when we rewrite possibly_profitable_path_p
> - // to work incrementally on added BBs we have to unwind them on
> backtracking
> - m_n_insns = 0;
> - m_stats = path_stats ();
> + loop_p loop = m_loop;
>
> - /* Count the number of instructions on the path: as these instructions
> - will have to be duplicated, we will not record the path if there
> - are too many instructions on the path. Also check that all the
> - blocks in the path belong to a single loop. */
> - for (unsigned j = 0; j < m_path.length (); j++)
> - {
> - basic_block bb = m_path[j];
> -
> - /* Remember, blocks in the path are stored in opposite order in
> - the PATH array. The last entry in the array represents the
> - block with an outgoing edge that we will redirect to the jump
> - threading path. Thus we don't care how many statements are
> - in that block because it will not be copied or whether or not
> - it ends in a multiway branch. */
> - if (j < m_path.length () - 1)
> - {
> - /* The block in PATH[0] is special, it's the block were we're
> - going to be able to eliminate its branch. */
> - account_bb (bb, /*check_multiway=*/j > 0);
> - if (m_stats.unprofitable_bb)
> - return false;
> - }
> -
> - /* Note if we thread through the latch, we will want to include
> - the last entry in the array when determining if we thread
> - through the loop latch. */
> - if (loop->latch == bb)
> - m_stats.threaded_through_latch = true;
> - }
> + if (m_stats.unprofitable_bb)
> + return false;
>
> if (dump_file && (dump_flags & TDF_DETAILS))
> {
> @@ -736,7 +762,7 @@ back_threader_profitability::possibly_profitable_path_p
> /* We are going to remove the control statement at the end of the
> last block in the threading path. So don't count it against our
> statement count. */
> - m_n_insns -= m_exit_jump_benefit;
> + m_n_insns = m_stats.n_insns - m_exit_jump_benefit;
>
> if (dump_file && (dump_flags & TDF_DETAILS))
> fprintf (dump_file, "\n Control statement insns: %i\n"
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)