This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] PR17913:[4.0 Regression] ICE jumping into statement expression


Roger Sayle wrote:
The following patch fix PR17913. The patch does the following:
if the first operator of the conditional_expr is constant and
the not relevant side of it contains label, the conditional_expr
can't be reduced.

May I suggest that you change the public API of contains_tree_code to be something like: extern bool contains_tree_code_p (tree, enum tree_code);

OK. This have been fixed.


My second suggestion is that it might be appropriate to test the
expression for TREE_SIDE_EFFECTS.

Fixed.


The third suggestion is could you double check that such LABEL_DECLs
aren't "discovered" from the GOTO_DESTINATION of a GOTO_EXPR.  We don't
want to accidentally consider a block reachable if it just contains
a GOTO_STMT.  Especially given tree-ssa's representation of conditional
jumps :>

I have restructured the "conatins" function for walk_tree. The function does not search within those tree which certainly mask labels.

You'll also need to add a new test case to the testsuite, perhaps a
variant of the code attached to the bugzilla PR.  And perhaps you
could also add a testcase that confirms your fix also considers
case labels inside statement expressions as potentially reachable.

New test case have been added for checking goto and switch. By the way I had to declare some language hook for checking those tree codes which are out of the common codes (for example: SWITCH_STMT).

There are some examples:

- when both side of the conditional expression are present

1 ? 1 : ({L: ;});

1 ? 1 : ({case xxx: ;});

- when nothing have been changed (cond_expr will be reduced)

1 ? 1 : ({goto L;});

1 ? 1 : ({switch(yyy){case xxx:;...}});


The patch have been bootstraped and regtested without regressions on i686-linux.

So, how does the following patch sound?

br,
    Gabor Loki


2004-11-02 Gabor Loki <loki@inf.u-szeged.hu>


	* c-types.c (build_conditional_expr): Remove reducing cond_expr.
	* fold-const.c (fold): Expand the condition of reducing cond_expr.
	(contains_label_1,contains_label_2,contains_label_p): New functions
	for checking labels in a sub-tree.
	* langhooks.c (lhd_mask_label_p): New dummy function.
	* langhooks-def.h (lhd_mask_label_p): Define prototype, and macro.
	* langhooks.h (mask_label_p): Prototype.
	* c-objc-common.h: Macro redefine.
	* cp-objcp-common.h: Same.
	* c-common.c (c_mask_label_p): New function for checking those
	language specific tree code which can mask label.
	* c-common.h (c_mask_label_p): Prototype.
	* testsuite/gcc.c-torture/compile/pr17913.c: Test case.

Index: gcc/c-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-common.c,v
retrieving revision 1.581
diff -3 -u -p -r1.581 c-common.c
--- gcc/c-common.c	31 Oct 2004 14:31:56 -0000	1.581
+++ gcc/c-common.c	2 Nov 2004 13:41:15 -0000
@@ -3850,6 +3850,17 @@ c_staticp (tree exp)
 	  && TREE_STATIC (COMPOUND_LITERAL_EXPR_DECL (exp))
 	  ? exp : NULL);
 }
+
+/* Hook used by contains_label_p to handle language-specific tree codes.  */
+
+bool
+c_mask_label_p (tree exp, enum tree_code code)
+{
+  if (code == CASE_LABEL_EXPR
+      && TREE_CODE(exp) == SWITCH_STMT)
+    return true;
+  return false;
+}
 
 
 /* Given a boolean expression ARG, return a tree representing an increment
Index: gcc/c-common.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-common.h,v
retrieving revision 1.269
diff -3 -u -p -r1.269 c-common.h
--- gcc/c-common.h	31 Oct 2004 06:17:49 -0000	1.269
+++ gcc/c-common.h	2 Nov 2004 13:41:15 -0000
@@ -839,6 +839,8 @@ extern rtx c_expand_expr (tree, rtx, enu
 
 extern tree c_staticp (tree);
 
+extern bool c_mask_label_p (tree, enum tree_code);
+
 extern int c_common_unsafe_for_reeval (tree);
 
 extern void init_c_lex (void);
Index: gcc/c-objc-common.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-objc-common.h,v
retrieving revision 2.1
diff -3 -u -p -r2.1 c-objc-common.h
--- gcc/c-objc-common.h	15 Aug 2004 21:47:32 -0000	2.1
+++ gcc/c-objc-common.h	2 Nov 2004 13:41:15 -0000
@@ -60,6 +60,8 @@ extern void c_initialize_diagnostics (di
 #define LANG_HOOKS_REDUCE_BIT_FIELD_OPERATIONS true
 #undef LANG_HOOKS_STATICP
 #define LANG_HOOKS_STATICP c_staticp
+#undef LANG_HOOKS_MASK_LABEL_P
+#define LANG_HOOKS_MASK_LABEL_P c_mask_label_p
 #undef LANG_HOOKS_NO_BODY_BLOCKS
 #define LANG_HOOKS_NO_BODY_BLOCKS true
 #undef LANG_HOOKS_WARN_UNUSED_GLOBAL_DECL
Index: gcc/c-typeck.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-typeck.c,v
retrieving revision 1.394
diff -3 -u -p -r1.394 c-typeck.c
--- gcc/c-typeck.c	31 Oct 2004 06:17:49 -0000	1.394
+++ gcc/c-typeck.c	2 Nov 2004 13:41:15 -0000
@@ -3023,9 +3023,6 @@ build_conditional_expr (tree ifexp, tree
   if (result_type != TREE_TYPE (op2))
     op2 = convert_and_check (result_type, op2);
 
-  if (TREE_CODE (ifexp) == INTEGER_CST)
-    return non_lvalue (integer_zerop (ifexp) ? op2 : op1);
-
   return fold (build3 (COND_EXPR, result_type, ifexp, op1, op2));
 }
 
Index: gcc/fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.469
diff -3 -u -p -r1.469 fold-const.c
--- gcc/fold-const.c	27 Oct 2004 20:27:18 -0000	1.469
+++ gcc/fold-const.c	2 Nov 2004 13:41:16 -0000
@@ -137,6 +137,9 @@ static tree fold_relational_const (enum 
 static tree fold_relational_hi_lo (enum tree_code *, const tree,
                                    tree *, tree *);
 static bool tree_expr_nonzero_p (tree);
+static tree contains_label_1 (tree*, int*, void*);
+static tree contains_label_2 (tree*, int*, void*);
+static bool contains_label_p (tree);
 
 /* We know that A1 + B1 = SUM1, using 2's complement arithmetic and ignoring
    overflow.  Suppose A, B and SUM have the same respective signs as A1, B1,
@@ -8858,12 +8861,16 @@ fold (tree expr)
 	 so all simple results must be passed through pedantic_non_lvalue.  */
       if (TREE_CODE (arg0) == INTEGER_CST)
 	{
+	  tree unused_op = TREE_OPERAND (t, (integer_zerop (arg0) ? 1 : 2));
 	  tem = TREE_OPERAND (t, (integer_zerop (arg0) ? 2 : 1));
 	  /* Only optimize constant conditions when the selected branch
 	     has the same type as the COND_EXPR.  This avoids optimizing
-	     away "c ? x : throw", where the throw has a void type.  */
-	  if (! VOID_TYPE_P (TREE_TYPE (tem))
-	      || VOID_TYPE_P (type))
+	     away "c ? x : throw", where the throw has a void type.
+	     Avoid throwing away that operand which contains label.  */
+	  if ((!TREE_SIDE_EFFECTS (unused_op)
+	       || !contains_label_p (unused_op))
+	      && (! VOID_TYPE_P (TREE_TYPE (tem))
+	          || VOID_TYPE_P (type)))
 	    return pedantic_non_lvalue (tem);
 	  return t;
 	}
@@ -10851,3 +10858,63 @@ ptr_difference_const (tree e1, tree e2, 
   *diff += (bitpos1 - bitpos2) / BITS_PER_UNIT;
   return true;
 }
+
+/* Callback for walk_tree, look for CASE_LABEL_EXPR.  */
+
+static tree
+contains_label_1 (tree *tp, 
+                      int *walk_subtrees, 
+                      void *data ATTRIBUTE_UNUSED)
+{
+  if (TREE_CODE(*tp) >= LAST_AND_UNUSED_TREE_CODE
+      && lang_hooks.mask_label_p (*tp, CASE_LABEL_EXPR))
+    {
+      *walk_subtrees=0;
+      return NULL_TREE;
+    }
+  switch (TREE_CODE(*tp))
+  {
+  case CASE_LABEL_EXPR:
+    return *tp;
+  case SWITCH_EXPR:
+    *walk_subtrees=0;
+    /* no break */
+  default:
+    return NULL_TREE;
+  }
+}
+
+/* Callback for walk_tree, look for LABEL_EXPR.  */
+
+static tree
+contains_label_2 (tree *tp, 
+                      int *walk_subtrees, 
+                      void *data ATTRIBUTE_UNUSED)
+{
+  if (TREE_CODE(*tp) >= LAST_AND_UNUSED_TREE_CODE
+      && lang_hooks.mask_label_p (*tp, LABEL_EXPR))
+    {
+      *walk_subtrees=0;
+      return NULL_TREE;
+    }
+  switch (TREE_CODE(*tp))
+  {
+  case LABEL_EXPR:
+    return *tp;
+  case GOTO_EXPR:
+    *walk_subtrees=0;
+    /* no break */
+  default:
+    return NULL_TREE;
+  }
+}
+
+/* Check wheter a sub-tree contains a label (CASE_LABEL_EXPR, LABEL_EXPR)
+   which is accessible out of the sub-tree.  */
+
+static bool
+contains_label_p (tree st)
+{
+  return (walk_tree(&st, contains_label_1 , NULL, NULL)
+          || walk_tree(&st, contains_label_2 , NULL, NULL));
+}
Index: gcc/langhooks-def.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/langhooks-def.h,v
retrieving revision 1.94
diff -3 -u -p -r1.94 langhooks-def.h
--- gcc/langhooks-def.h	14 Oct 2004 23:15:19 -0000	1.94
+++ gcc/langhooks-def.h	2 Nov 2004 13:41:16 -0000
@@ -51,6 +51,7 @@ extern tree lhd_return_null_tree (tree);
 extern tree lhd_do_nothing_iii_return_null_tree (int, int, int);
 extern int lhd_safe_from_p (rtx, tree);
 extern tree lhd_staticp (tree);
+extern bool lhd_mask_label_p (tree, enum tree_code);
 extern void lhd_print_tree_nothing (FILE *, tree, int);
 extern const char *lhd_decl_printable_name (tree, int);
 extern int lhd_types_compatible_p (tree, tree);
@@ -107,6 +108,7 @@ extern int lhd_gimplify_expr (tree *, tr
 #define LANG_HOOKS_SAFE_FROM_P		lhd_safe_from_p
 #define LANG_HOOKS_FINISH_INCOMPLETE_DECL lhd_do_nothing_t
 #define LANG_HOOKS_STATICP		lhd_staticp
+#define LANG_HOOKS_MASK_LABEL_P         lhd_mask_label_p
 #define LANG_HOOKS_DUP_LANG_SPECIFIC_DECL lhd_do_nothing_t
 #define LANG_HOOKS_SET_DECL_ASSEMBLER_NAME lhd_set_decl_assembler_name
 #define LANG_HOOKS_CAN_USE_BIT_FIELDS_P lhd_can_use_bit_fields_p
@@ -272,6 +274,7 @@ extern tree lhd_make_node (enum tree_cod
   LANG_HOOKS_FINISH_INCOMPLETE_DECL, \
   LANG_HOOKS_MARK_ADDRESSABLE, \
   LANG_HOOKS_STATICP, \
+  LANG_HOOKS_MASK_LABEL_P, \
   LANG_HOOKS_DUP_LANG_SPECIFIC_DECL, \
   LANG_HOOKS_SET_DECL_ASSEMBLER_NAME, \
   LANG_HOOKS_CAN_USE_BIT_FIELDS_P, \
Index: gcc/langhooks.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/langhooks.c,v
retrieving revision 1.79
diff -3 -u -p -r1.79 langhooks.c
--- gcc/langhooks.c	14 Oct 2004 23:15:19 -0000	1.79
+++ gcc/langhooks.c	2 Nov 2004 13:41:16 -0000
@@ -132,6 +132,14 @@ lhd_staticp (tree ARG_UNUSED (exp))
   return NULL;
 }
 
+/* Called from contains_label_p.  */
+
+bool 
+lhd_mask_label_p (tree ARG_UNUSED (exp), enum tree_code ARG_UNUSED (code))
+{
+  return false;
+}
+
 /* Called from check_global_declarations.  */
 
 bool
Index: gcc/langhooks.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/langhooks.h,v
retrieving revision 1.102
diff -3 -u -p -r1.102 langhooks.h
--- gcc/langhooks.h	14 Oct 2004 23:15:20 -0000	1.102
+++ gcc/langhooks.h	2 Nov 2004 13:41:16 -0000
@@ -311,6 +311,10 @@ struct lang_hooks
   /* Hook called by staticp for language-specific tree codes.  */
   tree (*staticp) (tree);
 
+  /* Nonzero if the language-specific tree node can contain specific 
+     tree code. Hook called by contains_label_p.  */
+  bool (*mask_label_p) (tree, enum tree_code);
+
   /* Replace the DECL_LANG_SPECIFIC data, which may be NULL, of the
      DECL_NODE with a newly GC-allocated copy.  */
   void (*dup_lang_specific_decl) (tree);
Index: gcc/cp/cp-objcp-common.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/cp-objcp-common.h,v
retrieving revision 1.4
diff -3 -u -p -r1.4 cp-objcp-common.h
--- gcc/cp/cp-objcp-common.h	22 Sep 2004 06:11:20 -0000	1.4
+++ gcc/cp/cp-objcp-common.h	2 Nov 2004 13:41:16 -0000
@@ -94,6 +94,8 @@ extern tree objcp_tsubst_copy_and_build 
 #define LANG_HOOKS_WRITE_GLOBALS lhd_do_nothing
 #undef LANG_HOOKS_COMDAT_GROUP
 #define LANG_HOOKS_COMDAT_GROUP cxx_comdat_group
+#undef LANG_HOOKS_MASK_LABEL_P
+#define LANG_HOOKS_MASK_LABEL_P c_mask_label_p
 
 #undef LANG_HOOKS_FUNCTION_INIT
 #define LANG_HOOKS_FUNCTION_INIT cxx_push_function_context
Index: gcc/testsuite/gcc.c-torture/compile/pr17913.c
===================================================================
--- /dev/null   2004-09-23 10:14:19.000000000 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr17913.c   2004-10-29 09:36:30.000000000 +0200
@@ -0,0 +1,13 @@
+void
+f(int a)
+{
+  goto l0;
+  1 ? a=1 : ({l0: a++;});
+  switch(a)
+  {
+  case 0:
+    a=6;
+  case 1:
+    1 ? a-- : ({case 2: a++;});
+  }
+}




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]