Fix COND_EXPR folding with CASE_LABEL_EXPRs (PR c++/83553)

Message ID 20171223083303.GS2353@tucnak
State New
Headers show
Series
  • Fix COND_EXPR folding with CASE_LABEL_EXPRs (PR c++/83553)
Related show

Commit Message

Jakub Jelinek Dec. 23, 2017, 8:33 a.m.
Hi!

The problem here is that for loops that have constant 0/false condition
the C++ FE wants to correctly emit if (0) { body; incr-expr; }
but doesn't just build3 (COND_EXPR, ...), but fold_build3, and COND_EXPR
folding with constant condition optimizes away the unused branch completely
if it doesn't contain any labels.  In this case it doesn't contain any
labels, but contains CASE_LABEL_EXPR that we should also take into account
(but can ignore it when during the walk we are inside a SWITCH_BODY).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/7.3?

2017-12-23  Jakub Jelinek  <jakub@redhat.com>

	PR c++/83553
	* fold-const.c (struct contains_label_data): New type.
	(contains_label_1): Return non-NULL even for CASE_LABEL_EXPR, unless
	inside of a SWITCH_BODY seen during the walk.
	(contains_label_p): Use walk_tree instead of
	walk_tree_without_duplicates, prepare data for contains_label_1 and
	provide own pset.

	* c-c++-common/torture/pr83553.c: New test.


	Jakub

Comments

Richard Biener Dec. 23, 2017, 8:35 a.m. | #1
On December 23, 2017 9:33:03 AM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!

>

>The problem here is that for loops that have constant 0/false condition

>the C++ FE wants to correctly emit if (0) { body; incr-expr; }

>but doesn't just build3 (COND_EXPR, ...), but fold_build3, and

>COND_EXPR

>folding with constant condition optimizes away the unused branch

>completely

>if it doesn't contain any labels.  In this case it doesn't contain any

>labels, but contains CASE_LABEL_EXPR that we should also take into

>account

>(but can ignore it when during the walk we are inside a SWITCH_BODY).

>

>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for

>trunk/7.3?


OK. 

Richard. 

>2017-12-23  Jakub Jelinek  <jakub@redhat.com>

>

>	PR c++/83553

>	* fold-const.c (struct contains_label_data): New type.

>	(contains_label_1): Return non-NULL even for CASE_LABEL_EXPR, unless

>	inside of a SWITCH_BODY seen during the walk.

>	(contains_label_p): Use walk_tree instead of

>	walk_tree_without_duplicates, prepare data for contains_label_1 and

>	provide own pset.

>

>	* c-c++-common/torture/pr83553.c: New test.

>

>--- gcc/fold-const.c.jj	2017-12-21 09:43:17.000000000 +0100

>+++ gcc/fold-const.c	2017-12-23 00:22:54.447669504 +0100

>@@ -11218,22 +11218,48 @@ fold_binary_loc (location_t loc, enum tr

>     } /* switch (code) */

> }

> 

>+/* Used by contains_label_[p1].  */

>+

>+struct contains_label_data

>+{

>+  hash_set<tree> *pset;

>+  bool inside_switch_p;

>+};

>+

>/* Callback for walk_tree, looking for LABEL_EXPR.  Return *TP if it is

>-   a LABEL_EXPR; otherwise return NULL_TREE.  Do not check the

>subtrees

>-   of GOTO_EXPR.  */

>+   a LABEL_EXPR or CASE_LABEL_EXPR not inside of another SWITCH_EXPR;

>otherwise

>+   return NULL_TREE.  Do not check the subtrees of GOTO_EXPR.  */

> 

> static tree

>-contains_label_1 (tree *tp, int *walk_subtrees, void *data

>ATTRIBUTE_UNUSED)

>+contains_label_1 (tree *tp, int *walk_subtrees, void *data)

> {

>+  contains_label_data *d = (contains_label_data *) data;

>   switch (TREE_CODE (*tp))

>     {

>     case LABEL_EXPR:

>       return *tp;

> 

>+    case CASE_LABEL_EXPR:

>+      if (!d->inside_switch_p)

>+	return *tp;

>+      return NULL_TREE;

>+

>+    case SWITCH_EXPR:

>+      if (!d->inside_switch_p)

>+	{

>+	  if (walk_tree (&SWITCH_COND (*tp), contains_label_1, data,

>d->pset))

>+	    return *tp;

>+	  d->inside_switch_p = true;

>+	  if (walk_tree (&SWITCH_BODY (*tp), contains_label_1, data,

>d->pset))

>+	    return *tp;

>+	  d->inside_switch_p = false;

>+	  *walk_subtrees = 0;

>+	}

>+      return NULL_TREE;

>+

>     case GOTO_EXPR:

>       *walk_subtrees = 0;

>-

>-      /* fall through */

>+      return NULL_TREE;

> 

>     default:

>       return NULL_TREE;

>@@ -11246,8 +11272,9 @@ contains_label_1 (tree *tp, int *walk_su

> static bool

> contains_label_p (tree st)

> {

>-  return

>-   (walk_tree_without_duplicates (&st, contains_label_1 , NULL) !=

>NULL_TREE);

>+  hash_set<tree> pset;

>+  contains_label_data data = { &pset, false };

>+  return walk_tree (&st, contains_label_1, &data, &pset) != NULL_TREE;

> }

> 

> /* Fold a ternary expression of code CODE and type TYPE with operands

>--- gcc/testsuite/c-c++-common/torture/pr83553.c.jj	2017-12-23

>00:30:20.548180122 +0100

>+++ gcc/testsuite/c-c++-common/torture/pr83553.c	2017-12-23

>00:29:11.000000000 +0100

>@@ -0,0 +1,29 @@

>+/* PR c++/83553 */

>+/* { dg-do run } */

>+

>+int a[3];

>+

>+int

>+foo (int n)

>+{

>+  switch (n)

>+    {

>+    case 0:

>+      for (n = 7, a[0]++; 0; a[2] = a[1] + 1)

>+	{

>+    case 2:

>+	  a[1] = a[0] + 1;

>+	}

>+    }

>+  return n;

>+}

>+

>+int

>+main ()

>+{

>+  if (foo (0) != 7 || a[0] != 1 || a[1] || a[2])

>+    __builtin_abort ();

>+  if (foo (2) != 2 || a[0] != 1 || a[1] != 2 || a[2] != 3)

>+    __builtin_abort ();

>+  return 0;

>+}

>

>	Jakub

Patch

--- gcc/fold-const.c.jj	2017-12-21 09:43:17.000000000 +0100
+++ gcc/fold-const.c	2017-12-23 00:22:54.447669504 +0100
@@ -11218,22 +11218,48 @@  fold_binary_loc (location_t loc, enum tr
     } /* switch (code) */
 }
 
+/* Used by contains_label_[p1].  */
+
+struct contains_label_data
+{
+  hash_set<tree> *pset;
+  bool inside_switch_p;
+};
+
 /* Callback for walk_tree, looking for LABEL_EXPR.  Return *TP if it is
-   a LABEL_EXPR; otherwise return NULL_TREE.  Do not check the subtrees
-   of GOTO_EXPR.  */
+   a LABEL_EXPR or CASE_LABEL_EXPR not inside of another SWITCH_EXPR; otherwise
+   return NULL_TREE.  Do not check the subtrees of GOTO_EXPR.  */
 
 static tree
-contains_label_1 (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
+contains_label_1 (tree *tp, int *walk_subtrees, void *data)
 {
+  contains_label_data *d = (contains_label_data *) data;
   switch (TREE_CODE (*tp))
     {
     case LABEL_EXPR:
       return *tp;
 
+    case CASE_LABEL_EXPR:
+      if (!d->inside_switch_p)
+	return *tp;
+      return NULL_TREE;
+
+    case SWITCH_EXPR:
+      if (!d->inside_switch_p)
+	{
+	  if (walk_tree (&SWITCH_COND (*tp), contains_label_1, data, d->pset))
+	    return *tp;
+	  d->inside_switch_p = true;
+	  if (walk_tree (&SWITCH_BODY (*tp), contains_label_1, data, d->pset))
+	    return *tp;
+	  d->inside_switch_p = false;
+	  *walk_subtrees = 0;
+	}
+      return NULL_TREE;
+
     case GOTO_EXPR:
       *walk_subtrees = 0;
-
-      /* fall through */
+      return NULL_TREE;
 
     default:
       return NULL_TREE;
@@ -11246,8 +11272,9 @@  contains_label_1 (tree *tp, int *walk_su
 static bool
 contains_label_p (tree st)
 {
-  return
-   (walk_tree_without_duplicates (&st, contains_label_1 , NULL) != NULL_TREE);
+  hash_set<tree> pset;
+  contains_label_data data = { &pset, false };
+  return walk_tree (&st, contains_label_1, &data, &pset) != NULL_TREE;
 }
 
 /* Fold a ternary expression of code CODE and type TYPE with operands
--- gcc/testsuite/c-c++-common/torture/pr83553.c.jj	2017-12-23 00:30:20.548180122 +0100
+++ gcc/testsuite/c-c++-common/torture/pr83553.c	2017-12-23 00:29:11.000000000 +0100
@@ -0,0 +1,29 @@ 
+/* PR c++/83553 */
+/* { dg-do run } */
+
+int a[3];
+
+int
+foo (int n)
+{
+  switch (n)
+    {
+    case 0:
+      for (n = 7, a[0]++; 0; a[2] = a[1] + 1)
+	{
+    case 2:
+	  a[1] = a[0] + 1;
+	}
+    }
+  return n;
+}
+
+int
+main ()
+{
+  if (foo (0) != 7 || a[0] != 1 || a[1] || a[2])
+    __builtin_abort ();
+  if (foo (2) != 2 || a[0] != 1 || a[1] != 2 || a[2] != 3)
+    __builtin_abort ();
+  return 0;
+}