| Message ID | 20240726043600.47858-2-quic_apinski@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] isel: Move duplicate comparisons to its own function | expand |
On Fri, Jul 26, 2024 at 6:37 AM Andrew Pinski <quic_apinski@quicinc.com> wrote: > > This is a small cleanup of the duplicating comparison code. > There is code generation difference but only for -O0 and -fno-tree-ter > (both of which will be fixed in a later patch). > The difference is instead of skipping the first use if the > comparison uses are only in cond_expr we skip the last use. > Also we go through the uses list in the opposite order now too. > > The cleanups are the following: > * Don't call has_single_use as we will do the loop anyways > * Change the order of the checks slightly, it is better > to check for cond_expr earlier > * Use cond_exprs as a stack and pop from it. > Skipping the top if the use is only from cond_expr. > > Bootstrapped and tested on x86_64-linux-gnu with no regressions. OK > gcc/ChangeLog: > > * gimple-isel.cc (duplicate_comparison): Rename to ... > (maybe_duplicate_comparison): This. Add check for use here > rather than in its caller. > (pass_gimple_isel::execute): Don't check how many uses the > comparison had and call maybe_duplicate_comparison instead of > duplicate_comparison. > > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> > --- > gcc/gimple-isel.cc | 38 ++++++++++++++++++++++++-------------- > 1 file changed, 24 insertions(+), 14 deletions(-) > > diff --git a/gcc/gimple-isel.cc b/gcc/gimple-isel.cc > index 327a78ea408..99bfc937bd5 100644 > --- a/gcc/gimple-isel.cc > +++ b/gcc/gimple-isel.cc > @@ -399,34 +399,46 @@ gimple_expand_vec_cond_expr (struct function *fun, gimple_stmt_iterator *gsi, > comparisons so RTL expansion with the help of TER > can perform better if conversion. */ > static void > -duplicate_comparison (gassign *stmt, basic_block bb) > +maybe_duplicate_comparison (gassign *stmt, basic_block bb) > { > imm_use_iterator imm_iter; > use_operand_p use_p; > auto_vec<gassign *, 4> cond_exprs; > - unsigned cnt = 0; > tree lhs = gimple_assign_lhs (stmt); > + unsigned cnt = 0; > + > FOR_EACH_IMM_USE_FAST (use_p, imm_iter, lhs) > { > if (is_gimple_debug (USE_STMT (use_p))) > continue; > cnt++; > + /* Add the use statement if it was a cond_expr. */ > if (gimple_bb (USE_STMT (use_p)) == bb > && is_gimple_assign (USE_STMT (use_p)) > - && gimple_assign_rhs1_ptr (USE_STMT (use_p)) == use_p->use > - && gimple_assign_rhs_code (USE_STMT (use_p)) == COND_EXPR) > + && gimple_assign_rhs_code (USE_STMT (use_p)) == COND_EXPR > + && gimple_assign_rhs1_ptr (USE_STMT (use_p)) == use_p->use) > cond_exprs.safe_push (as_a <gassign *> (USE_STMT (use_p))); > - } > - for (unsigned i = cond_exprs.length () == cnt ? 1 : 0; > - i < cond_exprs.length (); ++i) > + } > + > + /* If the comparison has 0 or 1 uses, no reason to do anything. */ > + if (cnt <= 1) > + return; > + > + /* If we only use the expression inside cond_exprs in that BB, we don't > + need to duplicate for one of them so pop the top. */ > + if (cond_exprs.length () == cnt) > + cond_exprs.pop(); > + > + while (!cond_exprs.is_empty()) > { > + auto old_top = cond_exprs.pop(); > gassign *copy = as_a <gassign *> (gimple_copy (stmt)); > tree new_def = duplicate_ssa_name (lhs, copy); > gimple_assign_set_lhs (copy, new_def); > - auto gsi2 = gsi_for_stmt (cond_exprs[i]); > + auto gsi2 = gsi_for_stmt (old_top); > gsi_insert_before (&gsi2, copy, GSI_SAME_STMT); > - gimple_assign_set_rhs1 (cond_exprs[i], new_def); > - update_stmt (cond_exprs[i]); > + gimple_assign_set_rhs1 (old_top, new_def); > + update_stmt (old_top); > } > } > > @@ -500,10 +512,8 @@ pass_gimple_isel::execute (struct function *fun) > continue; > > tree_code code = gimple_assign_rhs_code (stmt); > - tree lhs = gimple_assign_lhs (stmt); > - if (TREE_CODE_CLASS (code) == tcc_comparison > - && !has_single_use (lhs)) > - duplicate_comparison (stmt, bb); > + if (TREE_CODE_CLASS (code) == tcc_comparison) > + maybe_duplicate_comparison (stmt, bb); > } > } > > -- > 2.43.0 >
diff --git a/gcc/gimple-isel.cc b/gcc/gimple-isel.cc index 327a78ea408..99bfc937bd5 100644 --- a/gcc/gimple-isel.cc +++ b/gcc/gimple-isel.cc @@ -399,34 +399,46 @@ gimple_expand_vec_cond_expr (struct function *fun, gimple_stmt_iterator *gsi, comparisons so RTL expansion with the help of TER can perform better if conversion. */ static void -duplicate_comparison (gassign *stmt, basic_block bb) +maybe_duplicate_comparison (gassign *stmt, basic_block bb) { imm_use_iterator imm_iter; use_operand_p use_p; auto_vec<gassign *, 4> cond_exprs; - unsigned cnt = 0; tree lhs = gimple_assign_lhs (stmt); + unsigned cnt = 0; + FOR_EACH_IMM_USE_FAST (use_p, imm_iter, lhs) { if (is_gimple_debug (USE_STMT (use_p))) continue; cnt++; + /* Add the use statement if it was a cond_expr. */ if (gimple_bb (USE_STMT (use_p)) == bb && is_gimple_assign (USE_STMT (use_p)) - && gimple_assign_rhs1_ptr (USE_STMT (use_p)) == use_p->use - && gimple_assign_rhs_code (USE_STMT (use_p)) == COND_EXPR) + && gimple_assign_rhs_code (USE_STMT (use_p)) == COND_EXPR + && gimple_assign_rhs1_ptr (USE_STMT (use_p)) == use_p->use) cond_exprs.safe_push (as_a <gassign *> (USE_STMT (use_p))); - } - for (unsigned i = cond_exprs.length () == cnt ? 1 : 0; - i < cond_exprs.length (); ++i) + } + + /* If the comparison has 0 or 1 uses, no reason to do anything. */ + if (cnt <= 1) + return; + + /* If we only use the expression inside cond_exprs in that BB, we don't + need to duplicate for one of them so pop the top. */ + if (cond_exprs.length () == cnt) + cond_exprs.pop(); + + while (!cond_exprs.is_empty()) { + auto old_top = cond_exprs.pop(); gassign *copy = as_a <gassign *> (gimple_copy (stmt)); tree new_def = duplicate_ssa_name (lhs, copy); gimple_assign_set_lhs (copy, new_def); - auto gsi2 = gsi_for_stmt (cond_exprs[i]); + auto gsi2 = gsi_for_stmt (old_top); gsi_insert_before (&gsi2, copy, GSI_SAME_STMT); - gimple_assign_set_rhs1 (cond_exprs[i], new_def); - update_stmt (cond_exprs[i]); + gimple_assign_set_rhs1 (old_top, new_def); + update_stmt (old_top); } } @@ -500,10 +512,8 @@ pass_gimple_isel::execute (struct function *fun) continue; tree_code code = gimple_assign_rhs_code (stmt); - tree lhs = gimple_assign_lhs (stmt); - if (TREE_CODE_CLASS (code) == tcc_comparison - && !has_single_use (lhs)) - duplicate_comparison (stmt, bb); + if (TREE_CODE_CLASS (code) == tcc_comparison) + maybe_duplicate_comparison (stmt, bb); } }
This is a small cleanup of the duplicating comparison code. There is code generation difference but only for -O0 and -fno-tree-ter (both of which will be fixed in a later patch). The difference is instead of skipping the first use if the comparison uses are only in cond_expr we skip the last use. Also we go through the uses list in the opposite order now too. The cleanups are the following: * Don't call has_single_use as we will do the loop anyways * Change the order of the checks slightly, it is better to check for cond_expr earlier * Use cond_exprs as a stack and pop from it. Skipping the top if the use is only from cond_expr. Bootstrapped and tested on x86_64-linux-gnu with no regressions. gcc/ChangeLog: * gimple-isel.cc (duplicate_comparison): Rename to ... (maybe_duplicate_comparison): This. Add check for use here rather than in its caller. (pass_gimple_isel::execute): Don't check how many uses the comparison had and call maybe_duplicate_comparison instead of duplicate_comparison. Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com> --- gcc/gimple-isel.cc | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-)