if-to-switch: properly allow side effects only for first condition
Commit Message
Properly allow side effects only for a first BB in a condition chain.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
PR tree-optimization/106126
gcc/ChangeLog:
* gimple-if-to-switch.cc (struct condition_info): Save
has_side_effect.
(find_conditions): Parse all BBs.
(pass_if_to_switch::execute): Allow only side effects for first
BB.
gcc/testsuite/ChangeLog:
* gcc.dg/tree-ssa/pr106126.c: New test.
---
gcc/gimple-if-to-switch.cc | 20 +++++++++++---------
gcc/testsuite/gcc.dg/tree-ssa/pr106126.c | 12 ++++++++++++
2 files changed, 23 insertions(+), 9 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr106126.c
Comments
On Thu, Jun 30, 2022 at 4:29 PM Martin Liška <mliska@suse.cz> wrote:
>
> Properly allow side effects only for a first BB in a condition chain.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
OK
> Thanks,
> Martin
>
> PR tree-optimization/106126
>
> gcc/ChangeLog:
>
> * gimple-if-to-switch.cc (struct condition_info): Save
> has_side_effect.
> (find_conditions): Parse all BBs.
> (pass_if_to_switch::execute): Allow only side effects for first
> BB.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/pr106126.c: New test.
> ---
> gcc/gimple-if-to-switch.cc | 20 +++++++++++---------
> gcc/testsuite/gcc.dg/tree-ssa/pr106126.c | 12 ++++++++++++
> 2 files changed, 23 insertions(+), 9 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr106126.c
>
> diff --git a/gcc/gimple-if-to-switch.cc b/gcc/gimple-if-to-switch.cc
> index 70daae2003c..4441206c481 100644
> --- a/gcc/gimple-if-to-switch.cc
> +++ b/gcc/gimple-if-to-switch.cc
> @@ -61,9 +61,11 @@ struct condition_info
> {
> typedef auto_vec<std::pair<gphi *, tree>> mapping_vec;
>
> - condition_info (gcond *cond): m_cond (cond), m_bb (gimple_bb (cond)),
> - m_forwarder_bb (NULL), m_ranges (), m_true_edge (NULL), m_false_edge (NULL),
> - m_true_edge_phi_mapping (), m_false_edge_phi_mapping ()
> + condition_info (gcond *cond, bool has_side_effect): m_cond (cond),
> + m_bb (gimple_bb (cond)), m_forwarder_bb (NULL), m_ranges (),
> + m_true_edge (NULL), m_false_edge (NULL),
> + m_true_edge_phi_mapping (), m_false_edge_phi_mapping (),
> + m_has_side_effect (has_side_effect)
> {
> m_ranges.create (0);
> }
> @@ -80,6 +82,7 @@ struct condition_info
> edge m_false_edge;
> mapping_vec m_true_edge_phi_mapping;
> mapping_vec m_false_edge_phi_mapping;
> + bool m_has_side_effect;
> };
>
> /* Recond PHI mapping for an original edge E and save these into vector VEC. */
> @@ -389,16 +392,11 @@ find_conditions (basic_block bb,
> if (cond == NULL)
> return;
>
> - /* An empty conditions_in_bbs indicates we are processing the first
> - basic-block then no need check side effect. */
> - if (!conditions_in_bbs->is_empty () && !no_side_effect_bb (bb))
> - return;
> -
> tree lhs = gimple_cond_lhs (cond);
> tree rhs = gimple_cond_rhs (cond);
> tree_code code = gimple_cond_code (cond);
>
> - condition_info *info = new condition_info (cond);
> + condition_info *info = new condition_info (cond, !no_side_effect_bb (bb));
>
> gassign *def;
> if (code == NE_EXPR
> @@ -536,6 +534,10 @@ pass_if_to_switch::execute (function *fun)
> if ((*info2)->m_false_edge != e)
> break;
>
> + /* Only the first BB in a chain can have a side effect. */
> + if (info->m_has_side_effect)
> + break;
> +
> chain->m_entries.safe_push (*info2);
> bitmap_set_bit (seen_bbs, e->src->index);
> info = *info2;
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr106126.c b/gcc/testsuite/gcc.dg/tree-ssa/pr106126.c
> new file mode 100644
> index 00000000000..2f0fd44164b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr106126.c
> @@ -0,0 +1,12 @@
> +/* PR tree-optimization/106126 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +char *var_1;
> +void pool_conda_matchspec() {
> + for (; var_1 && *var_1 &&
> + *var_1 != '<' && *var_1 != '>' &&
> + *var_1 != '!' && *var_1 != '~';)
> + if (*var_1 >= 'A' && *var_1 <= 'Z')
> + *var_1 += 'A';
> +}
> --
> 2.36.1
>
@@ -61,9 +61,11 @@ struct condition_info
{
typedef auto_vec<std::pair<gphi *, tree>> mapping_vec;
- condition_info (gcond *cond): m_cond (cond), m_bb (gimple_bb (cond)),
- m_forwarder_bb (NULL), m_ranges (), m_true_edge (NULL), m_false_edge (NULL),
- m_true_edge_phi_mapping (), m_false_edge_phi_mapping ()
+ condition_info (gcond *cond, bool has_side_effect): m_cond (cond),
+ m_bb (gimple_bb (cond)), m_forwarder_bb (NULL), m_ranges (),
+ m_true_edge (NULL), m_false_edge (NULL),
+ m_true_edge_phi_mapping (), m_false_edge_phi_mapping (),
+ m_has_side_effect (has_side_effect)
{
m_ranges.create (0);
}
@@ -80,6 +82,7 @@ struct condition_info
edge m_false_edge;
mapping_vec m_true_edge_phi_mapping;
mapping_vec m_false_edge_phi_mapping;
+ bool m_has_side_effect;
};
/* Recond PHI mapping for an original edge E and save these into vector VEC. */
@@ -389,16 +392,11 @@ find_conditions (basic_block bb,
if (cond == NULL)
return;
- /* An empty conditions_in_bbs indicates we are processing the first
- basic-block then no need check side effect. */
- if (!conditions_in_bbs->is_empty () && !no_side_effect_bb (bb))
- return;
-
tree lhs = gimple_cond_lhs (cond);
tree rhs = gimple_cond_rhs (cond);
tree_code code = gimple_cond_code (cond);
- condition_info *info = new condition_info (cond);
+ condition_info *info = new condition_info (cond, !no_side_effect_bb (bb));
gassign *def;
if (code == NE_EXPR
@@ -536,6 +534,10 @@ pass_if_to_switch::execute (function *fun)
if ((*info2)->m_false_edge != e)
break;
+ /* Only the first BB in a chain can have a side effect. */
+ if (info->m_has_side_effect)
+ break;
+
chain->m_entries.safe_push (*info2);
bitmap_set_bit (seen_bbs, e->src->index);
info = *info2;
new file mode 100644
@@ -0,0 +1,12 @@
+/* PR tree-optimization/106126 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+char *var_1;
+void pool_conda_matchspec() {
+ for (; var_1 && *var_1 &&
+ *var_1 != '<' && *var_1 != '>' &&
+ *var_1 != '!' && *var_1 != '~';)
+ if (*var_1 >= 'A' && *var_1 <= 'Z')
+ *var_1 += 'A';
+}