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]

[tuples] Fix gimplification of MODIFY_EXPR


These are a few miscellaneous fixes found while trying to compile libgcc.

- We were not consistently checking what is allowed on the RHS of an
assignment.  I reformulated is_gimple_formal_tmp_rhs in terms of a new
predicate that returns whether a tree expression is a gimple binary,
unary or single object RHS.

- The code for attaching statements to sequences was allowing sequences
that were "cut in the middle", added new asserts to make sure that
sequences are cut off at the ends.

Tested with gimple.exp.  Committed.
2007-07-27  Diego Novillo  <dnovillo@google.com>

	* tree-gimple.c (get_gimple_rhs_class): New.
	(is_gimple_formal_tmp_rhs): Call it.
	* tree-gimple.h (enum gimple_rhs_class): New.
	* gimple-iterator.h (gsi_next): Assert that there is nothing
	beyond the end of the sequence.
	(gsi_prev): Assert that there is nothing before the start of
	the sequence.
	* gimplify.c (gimplify_switch_expr): Tidy creation of default label.
	(gimplify_expr): Fix concatenation of internal sequences to PRE_P.
	* gimple.c (get_num_ops_for): Remove.  Update users.
	(build_gimple_assign): Call get_gimple_rhs_class to determine
	how many operands to allocate.
	(gimple_add): Assert that GS does not have previous or next
	statements.
	(gimple_seq_append): Move from gimple.h.

Index: tree-gimple.c
===================================================================
--- tree-gimple.c	(revision 126977)
+++ tree-gimple.c	(working copy)
@@ -38,19 +38,26 @@ Boston, MA 02110-1301, USA.  */
 
 /* Validation of GIMPLE expressions.  */
 
-/* Return true if T is a GIMPLE RHS for an assignment to a temporary.  */
+/* Determine if expression T is one of the valid expressions that can
+   be used on the RHS of GIMPLE assignments.  */
 
-bool
-is_gimple_formal_tmp_rhs (tree t)
+enum gimple_rhs_class
+get_gimple_rhs_class (tree t)
 {
   enum tree_code code = TREE_CODE (t);
-
-  switch (TREE_CODE_CLASS (code))
+  enum tree_code_class class = TREE_CODE_CLASS (code);
+  
+  switch (class)
     {
     case tcc_unary:
+      return GIMPLE_UNARY_RHS;
+
     case tcc_binary:
     case tcc_comparison:
-      return true;
+      return GIMPLE_BINARY_RHS;
+
+    case tcc_constant:
+      return GIMPLE_SINGLE_RHS;
 
     default:
       break;
@@ -58,28 +65,39 @@ is_gimple_formal_tmp_rhs (tree t)
 
   switch (code)
     {
-    case TRUTH_NOT_EXPR:
     case TRUTH_AND_EXPR:
     case TRUTH_OR_EXPR:
     case TRUTH_XOR_EXPR:
+      return GIMPLE_BINARY_RHS;
+
+    case TRUTH_NOT_EXPR:
     case ADDR_EXPR:
-    case CALL_EXPR:
+      return GIMPLE_UNARY_RHS;
+
+    /* FIXME tuples.  We are not allowing CALL_EXPR on the RHS
+       anymore.  Watch out for random failures.  */
     case CONSTRUCTOR:
-    case COMPLEX_EXPR:
-    case INTEGER_CST:
-    case REAL_CST:
-    case STRING_CST:
-    case COMPLEX_CST:
-    case VECTOR_CST:
     case OBJ_TYPE_REF:
     case ASSERT_EXPR:
-      return true;
+      return GIMPLE_SINGLE_RHS;
 
     default:
       break;
     }
 
-  return is_gimple_lvalue (t) || is_gimple_val (t);
+  if (is_gimple_lvalue (t) || is_gimple_val (t))
+    return GIMPLE_SINGLE_RHS;
+
+  return GIMPLE_INVALID_RHS;
+}
+
+
+/* Return true if T is a GIMPLE RHS for an assignment to a temporary.  */
+
+bool
+is_gimple_formal_tmp_rhs (tree t)
+{
+  return get_gimple_rhs_class (t) != GIMPLE_INVALID_RHS;
 }
 
 /* Returns true iff T is a valid RHS for an assignment to a renamed
Index: tree-gimple.h
===================================================================
--- tree-gimple.h	(revision 126977)
+++ tree-gimple.h	(working copy)
@@ -22,18 +22,28 @@ Boston, MA 02110-1301, USA.  */
 #ifndef _TREE_SIMPLE_H
 #define _TREE_SIMPLE_H 1
 
-
 #include "tree-iterator.h"
 #include "gimple.h"
 
+
+/* Class of GIMPLE expressions suitable for the RHS of assignments.  See
+   get_gimple_rhs_class.  */
+enum gimple_rhs_class
+{
+  GIMPLE_INVALID_RHS,	/* The expression cannot be used on the RHS.  */
+  GIMPLE_BINARY_RHS,	/* The expression is a binary operation.  */
+  GIMPLE_UNARY_RHS,	/* The expression is a unary operation.  */
+  GIMPLE_SINGLE_RHS	/* The expression is a single object (an SSA
+			   name, a _DECL, a _REF, etc.  */
+};
+
+/* In gimplify.c  */
 extern tree create_tmp_var_raw (tree, const char *);
 extern tree create_tmp_var_name (const char *);
 extern tree create_tmp_var (tree, const char *);
 extern tree get_initialized_tmp_var (tree, gimple_seq, gimple_seq);
 extern tree get_formal_tmp_var (tree, gimple_seq);
-
 extern void declare_vars (tree, gimple, bool);
-
 extern void annotate_all_with_locus (gimple_seq, location_t);
 
 /* Validation of GIMPLE expressions.  Note that these predicates only check
@@ -94,6 +104,7 @@ extern bool is_gimple_call_addr (tree);
 extern tree get_call_expr_in (tree t);
 
 extern void recalculate_side_effects (tree);
+extern enum gimple_rhs_class get_gimple_rhs_class (tree);
 
 /* FIXME we should deduce this from the predicate.  */
 typedef enum fallback_t {
Index: gimple-iterator.h
===================================================================
--- gimple-iterator.h	(revision 126977)
+++ gimple-iterator.h	(working copy)
@@ -36,7 +36,7 @@ gsi_start (gimple_seq seq)
 {
   gimple_stmt_iterator i;
 
-  i.stmt = gimple_seq_first(seq);
+  i.stmt = gimple_seq_first (seq);
   i.seq = seq;
 
   return i;
@@ -49,7 +49,7 @@ gsi_last (gimple_seq seq)
 {
   gimple_stmt_iterator i;
 
-  i.stmt = gimple_seq_last(seq);
+  i.stmt = gimple_seq_last (seq);
   i.seq = seq;
 
   return i;
@@ -68,7 +68,7 @@ gsi_end_p (gimple_stmt_iterator i)
 static inline bool
 gsi_one_before_end_p (gimple_stmt_iterator i)
 {
-  return i.stmt == gimple_seq_last(i.seq);
+  return i.stmt == gimple_seq_last (i.seq);
 }
 
 /* Return the next gimple statement in I.  */
@@ -76,6 +76,13 @@ gsi_one_before_end_p (gimple_stmt_iterat
 static inline void
 gsi_next (gimple_stmt_iterator *i)
 {
+#if defined ENABLE_GIMPLE_CHECKING
+  /* The last statement of the sequence should not have anything
+     chained after it.  */
+  gimple next = gimple_next (i->stmt);
+  if (i->stmt == gimple_seq_last (i->seq))
+    gcc_assert (next == NULL);
+#endif
   i->stmt = gimple_next (i->stmt);
 }
 
@@ -84,6 +91,13 @@ gsi_next (gimple_stmt_iterator *i)
 static inline void
 gsi_prev (gimple_stmt_iterator *i)
 {
+#if defined ENABLE_GIMPLE_CHECKING
+  /* The first statement of the sequence should not have anything
+     chained before it.  */
+  gimple prev = gimple_prev (i->stmt);
+  if (i->stmt == gimple_seq_first (i->seq))
+    gcc_assert (prev == NULL);
+#endif
   i->stmt = gimple_prev (i->stmt);
 }
 
Index: gimplify.c
===================================================================
--- gimplify.c	(revision 126977)
+++ gimplify.c	(working copy)
@@ -1458,11 +1458,9 @@ gimplify_switch_expr (tree *expr_p, gimp
 	{
 	  /* If the switch has no default label, add one, so that we jump
 	     around the switch body.  */
-	  gimple new_default = build_gimple_label (build3 (CASE_LABEL_EXPR,
-	                                               void_type_node,
-	                                               NULL_TREE,
-	                                               NULL_TREE, 
-	                                           create_artificial_label ()));
+	  tree def_lab = build3 (CASE_LABEL_EXPR, void_type_node, NULL_TREE,
+	                         NULL_TREE, create_artificial_label ());
+	  gimple new_default = build_gimple_label (def_lab);
 	  gimple_add (switch_body_seq, new_default);
 	  default_case = gimple_label_label (new_default);
 	}
@@ -6350,7 +6348,7 @@ gimplify_expr (tree *expr_p, gimple_seq 
           !gimple_seq_empty_p (&internal_post))
 	{
 	  gimple_seq_append (&internal_pre, &internal_post);
-	  gimple_seq_append (&internal_pre, pre_p);
+	  gimple_seq_append (pre_p, &internal_pre);
 	}
 
       if (!gimple_seq_empty_p (&internal_pre))
@@ -6684,7 +6682,8 @@ check_pointer_types_r (tree *tp, int *wa
 
 /* Gimplify the body of statements pointed to by BODY_P and store the
    corresponding GIMPLE sequence in SEQ_P.  FNDECL is the function
-   decl containing BODY.  */
+   decl containing BODY.  DO_PARMS is true if FNDECL's parameters
+   should be gimplified.  */
 
 void
 gimplify_body (tree *body_p, gimple_seq seq_p, tree fndecl, bool do_parms)
@@ -6698,11 +6697,6 @@ gimplify_body (tree *body_p, gimple_seq 
   gcc_assert (gimplify_ctxp == NULL);
   push_gimplify_context ();
 
-  /* Unshare most shared trees in the body and in that of any nested functions.
-     It would seem we don't have to do this for nested functions because
-     they are supposed to be output and then the outer function gimplified
-     first, but the g++ front end doesn't always do it that way.  */
-  unshare_body (body_p, fndecl);
   unvisit_body (body_p, fndecl);
 
   /* Make sure input_location isn't set to something wierd.  */
Index: gimple.c
===================================================================
--- gimple.c	(revision 126977)
+++ gimple.c	(working copy)
@@ -200,16 +200,6 @@ build_gimple_call (tree fn, size_t nargs
 }
 
 
-/* Given a CODE for the RHS of a GIMPLE_ASSIGN, return the number of operands
-   it needs.  */
-
-static inline size_t
-get_num_ops_for (enum tree_code code)
-{
-  enum tree_code_class class = TREE_CODE_CLASS (code);
-  return (class == tcc_binary || class == tcc_comparison) ? 2 : 1;
-}
-
 /* Construct a GIMPLE_ASSIGN statement.
 
    LHS of the assignment.
@@ -221,28 +211,31 @@ build_gimple_assign (tree lhs, tree rhs)
   gimple p;
   size_t num_ops;
   enum tree_code subcode = TREE_CODE (rhs);
-  enum tree_code_class class = TREE_CODE_CLASS (subcode);
+  enum gimple_rhs_class class = get_gimple_rhs_class (rhs);
 
   /* Make sure the RHS is a valid GIMPLE RHS.  */
   gcc_assert (is_gimple_formal_tmp_rhs (rhs));
 
   /* Need 1 operand for LHS and 1 or 2 for the RHS (depending on the
      code).  */
-  num_ops = get_num_ops_for (subcode) + 1;
+  if (class == GIMPLE_UNARY_RHS || class == GIMPLE_SINGLE_RHS)
+    num_ops = 2;
+  else if (class == GIMPLE_BINARY_RHS)
+    num_ops = 3;
+  else
+    gcc_unreachable ();
   
   p = build_gimple_with_ops (GIMPLE_ASSIGN, subcode, num_ops);
   gimple_assign_set_lhs (p, lhs);
 
-  if (class == tcc_binary || class == tcc_comparison)
+  if (class == GIMPLE_BINARY_RHS)
     {
       gimple_assign_set_rhs1 (p, TREE_OPERAND (rhs, 0));
       gimple_assign_set_rhs2 (p, TREE_OPERAND (rhs, 1));
     }
-  else if (class == tcc_unary)
+  else if (class == GIMPLE_UNARY_RHS)
     gimple_assign_set_rhs1 (p, TREE_OPERAND (rhs, 0));
-  else if (class == tcc_constant
-           || class == tcc_declaration
-	   || subcode == SSA_NAME)
+  else if (class == GIMPLE_SINGLE_RHS)
     gimple_assign_set_rhs1 (p, rhs);
   else
     gcc_unreachable ();
@@ -809,24 +802,44 @@ gimple_range_check_failed (const gimple 
 void
 gimple_add (gimple_seq seq, gimple gs)
 {
-  gimple last;
-
   /* Make sure this stmt is not part of another chain.  */
-  gcc_assert (gimple_prev (gs) == NULL);
-
-  for (last = gs; gimple_next (last) != NULL; last = gimple_next (last))
-    ;
+  gcc_assert (gimple_prev (gs) == NULL && gimple_next (gs) == NULL);
 
   if (gimple_seq_first (seq) == NULL)
     {
+      /* Sequence SEQ is empty.  Make GS its only member.  */
       gimple_seq_set_first (seq, gs);
-      gimple_seq_set_last (seq, last);
+      gimple_seq_set_last (seq, gs);
     }
   else
     {
+      /* Otherwise, link GS to the end of SEQ.  */
       set_gimple_prev (gs, gimple_seq_last (seq));
       set_gimple_next (gimple_seq_last (seq), gs);
-      gimple_seq_set_last (seq, last);
+      gimple_seq_set_last (seq, gs);
+    }
+}
+
+
+/* Append sequence SRC to the end of sequence DST.  */
+
+void
+gimple_seq_append (gimple_seq dst, gimple_seq src)
+{
+  if (gimple_seq_empty_p (src))
+    return;
+
+  /* Make sure SRC is not linked somewhere else.  */
+  gcc_assert (gimple_prev (src->first) == NULL
+              && gimple_next (src->last) == NULL);
+
+  if (gimple_seq_empty_p (dst))
+    gimple_seq_copy (dst, src);
+  else
+    {
+      set_gimple_next (gimple_seq_last (dst), gimple_seq_first (src));
+      set_gimple_prev (gimple_seq_first (src), gimple_seq_last (dst));
+      gimple_seq_set_last (dst, gimple_seq_last (src));
     }
 }
 
Index: gimple.h
===================================================================
--- gimple.h	(revision 126977)
+++ gimple.h	(working copy)
@@ -383,8 +383,6 @@ gimple_locus_empty_p (gimple g)
   return gimple_locus (g).file == NULL && gimple_locus (g).line == 0;
 }
 
-
-
 /* In gimple.c.  */
 extern gimple build_gimple_return (bool, tree);
 extern gimple build_gimple_assign (tree, tree);
@@ -426,6 +424,7 @@ extern void walk_seq_ops (gimple_seq, wa
                           struct pointer_set_t *);
 extern void set_gimple_body (tree, gimple_seq);
 extern gimple_seq gimple_body (tree);
+extern void gimple_seq_append (gimple_seq, gimple_seq);
 
 extern const char *const gimple_code_name[];
 
@@ -1288,15 +1287,6 @@ gimple_return_set_retval (gimple gs, tre
   gs->with_ops.op[0] = retval;
 }
 
-/* Append sequence SRC to the end of sequence DST.  */
-
-static inline void
-gimple_seq_append (gimple_seq dst, gimple_seq src)
-{
-  if (!gimple_seq_empty_p (src))
-    gimple_add (dst, gimple_seq_first (src));
-}
-
 #include "gimple-iterator.h"
 
 #endif  /* GCC_GIMPLE_IR_H */

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