public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] if-to-switch: properly allow side effects only for first condition
@ 2022-06-30 14:29 Martin Liška
  2022-07-01  6:39 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Martin Liška @ 2022-06-30 14:29 UTC (permalink / raw)
  To: gcc-patches

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

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] if-to-switch: properly allow side effects only for first condition
  2022-06-30 14:29 [PATCH] if-to-switch: properly allow side effects only for first condition Martin Liška
@ 2022-07-01  6:39 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2022-07-01  6:39 UTC (permalink / raw)
  To: Martin Liška; +Cc: GCC Patches, xionghuluo(罗雄虎)

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
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-07-01  6:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-30 14:29 [PATCH] if-to-switch: properly allow side effects only for first condition Martin Liška
2022-07-01  6:39 ` Richard Biener

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).