diff mbox

[07/18] generalize build_case_label to the rest of the compiler

Message ID 1299817406-16745-8-git-send-email-froydnj@codesourcery.com
State New
Headers show

Commit Message

Nathan Froyd March 11, 2011, 4:23 a.m. UTC 
If we're going to add another TREE_OPERAND to CASE_LABEL_EXPR, then we'd
have to update a number of calls to buildN or similar to know about the
new operand.  But that's not a good idea; the new operand is a strictly
private thing and FEs in particular shouldn't have to know anything
about it.  Instead, define a build_case_label that accepts the current
three operands CASE_LABEL_EXPR requires; build_case_label can then be
updated later.  (Actually, since it can use CASE_CHAIN, it doesn't have
to be updated at all!)

This patch does lose location information on CASE_LABEL_EXPRs from the C
family of front-ends; it did not seem worth it to have a number of
places pass input_location when said information isn't even used.  I'm
happy to add the location_t argument back to CASE_LABEL_EXPRs if people
think that's worthwhile.

-Nathan

gcc/ada/
	* gcc-interface/trans.c (Case_Statement_to_gnu): Call build_case_label.

gcc/
	* except.c (sjlj_emit_dispatch_table): Call build_case_label.
	* gimplify.c (gimplify_switch_expr): Likewise.
	* omp-low.c (expand_omp_sections): Likewise.
	* tree-eh.c (lower_try_finally_switch): Likewise.
	(lower_eh_dispatch): Likewise.
	* tree.h (build_case_label): Declare.
	* tree.c (build_case_label): Define.

gcc/c-family/
	* c-common.c (c_add_case_label): Omit the loc argument to
	build_case_label.
	* c-common.h (build_case_label): Remove.
	* c-semantics.c (build_case_label): Remove.

gcc/cp/
	* decl.c (finish_case_label): Omit the loc argument to
	build_case_label.

gcc/fortran/
	* trans-decl.c (gfc_trans_entry_master_switch): Call build_case_label.
	* trans-io.c (add_case): Likewise.
	* trans-stmt.c (gfc_trans_integer_select): Likewise.
	(gfc_trans_character_select): Likewise.

gcc/java/
	* expr.c (expand_java_switch): Call build_case_label.
	(expand_java_add_case): Likewise.

Comments

Joseph Myers March 11, 2011, 1:01 p.m. UTC | #1 
On Thu, 10 Mar 2011, Nathan Froyd wrote:

> This patch does lose location information on CASE_LABEL_EXPRs from the C
> family of front-ends; it did not seem worth it to have a number of
> places pass input_location when said information isn't even used.  I'm
> happy to add the location_t argument back to CASE_LABEL_EXPRs if people
> think that's worthwhile.

Since implicit use of input_location is deprecated and should be being 
phased out (making more locations explicit - including explicit 
input_location until the places using it have a better location 
available), I think you should keep the location argument.

The C front-end changes in this patch series (including c-family changes) 
are otherwise OK.
Richard Biener March 11, 2011, 1:10 p.m. UTC | #2 
On Fri, Mar 11, 2011 at 2:01 PM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> On Thu, 10 Mar 2011, Nathan Froyd wrote:
>
>> This patch does lose location information on CASE_LABEL_EXPRs from the C
>> family of front-ends; it did not seem worth it to have a number of
>> places pass input_location when said information isn't even used.  I'm
>> happy to add the location_t argument back to CASE_LABEL_EXPRs if people
>> think that's worthwhile.
>
> Since implicit use of input_location is deprecated and should be being
> phased out (making more locations explicit - including explicit
> input_location until the places using it have a better location
> available), I think you should keep the location argument.
>
> The C front-end changes in this patch series (including c-family changes)
> are otherwise OK.

Yep, please pass UNKNOWN_LOCATION at callers that didn't set a
location (and add a location argument to build_case_label).

Thanks,
Richard.

> --
> Joseph S. Myers
> joseph@codesourcery.com
>
Tom Tromey March 11, 2011, 2:55 p.m. UTC | #3 
>>>>> "Nathan" == Nathan Froyd <froydnj@codesourcery.com> writes:

Nathan> gcc/java/
Nathan> 	* expr.c (expand_java_switch): Call build_case_label.
Nathan> 	(expand_java_add_case): Likewise.

The java parts are ok.

FWIW, I tend to think that if a core change like this one is accepted,
then updates to the front ends should be considered obvious, barring
some unusual circumstance.

Your update to this patch to pass a location, per the other reviews, is
pre-approved.

Tom
diff mbox

Patch

diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
index e438960..90220f2 100644
--- a/gcc/ada/gcc-interface/trans.c
+++ b/gcc/ada/gcc-interface/trans.c
@@ -2047,9 +2047,8 @@  Case_Statement_to_gnu (Node_Id gnat_node)
 	  if ((!gnu_low || TREE_CODE (gnu_low) == INTEGER_CST)
 	      && (!gnu_high || TREE_CODE (gnu_high) == INTEGER_CST))
 	    {
-	      add_stmt_with_node (build3
-				  (CASE_LABEL_EXPR, void_type_node,
-				   gnu_low, gnu_high,
+	      add_stmt_with_node (build_case_label
+				  (gnu_low, gnu_high,
 				   create_artificial_label (input_location)),
 				  gnat_choice);
 	      choices_added_p = true;
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 7207335..da859ec 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5298,7 +5298,7 @@  c_add_case_label (location_t loc, splay_tree cases, tree cond, tree orig_type,
     }
 
   /* Add a CASE_LABEL to the statement-tree.  */
-  case_label = add_stmt (build_case_label (loc, low_value, high_value, label));
+  case_label = add_stmt (build_case_label (low_value, high_value, label));
   /* Register this case label in the splay tree.  */
   splay_tree_insert (cases,
 		     (splay_tree_key) low_value,
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 7e4f0a1..382d535 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -829,7 +829,6 @@  extern void warn_for_omitted_condop (location_t, tree);
 
 extern tree do_case (location_t, tree, tree);
 extern tree build_stmt (location_t, enum tree_code, ...);
-extern tree build_case_label (location_t, tree, tree, tree);
 extern tree build_real_imag_expr (location_t, enum tree_code, tree);
 
 /* These functions must be defined by each front-end which implements
diff --git a/gcc/c-family/c-semantics.c b/gcc/c-family/c-semantics.c
index 0f9f51e..a5bd9ba 100644
--- a/gcc/c-family/c-semantics.c
+++ b/gcc/c-family/c-semantics.c
@@ -131,15 +131,6 @@  build_stmt (location_t loc, enum tree_code code, ...)
   return ret;
 }
 
-/* Create a CASE_LABEL_EXPR tree node and return it.  */
-
-tree
-build_case_label (location_t loc,
-		  tree low_value, tree high_value, tree label_decl)
-{
-  return build_stmt (loc, CASE_LABEL_EXPR, low_value, high_value, label_decl);
-}
-
 /* Build a REALPART_EXPR or IMAGPART_EXPR, according to CODE, from ARG.  */
 
 tree
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 14d530a..6ff6974 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -2933,7 +2933,7 @@  finish_case_label (location_t loc, tree low_value, tree high_value)
       /* For templates, just add the case label; we'll do semantic
 	 analysis at instantiation-time.  */
       label = build_decl (loc, LABEL_DECL, NULL_TREE, NULL_TREE);
-      return add_stmt (build_case_label (loc, low_value, high_value, label));
+      return add_stmt (build_case_label (low_value, high_value, label));
     }
 
   /* Find the condition on which this switch statement depends.  */
diff --git a/gcc/except.c b/gcc/except.c
index 251a550..def8a53 100644
--- a/gcc/except.c
+++ b/gcc/except.c
@@ -1288,9 +1288,8 @@  sjlj_emit_dispatch_table (rtx dispatch_label, int num_dispatch)
 	    tree t_label, case_elt;
 
 	    t_label = create_artificial_label (UNKNOWN_LOCATION);
-	    case_elt = build3 (CASE_LABEL_EXPR, void_type_node,
-			       build_int_cst (NULL, disp_index),
-			       NULL, t_label);
+	    case_elt = build_case_label (build_int_cst (NULL, disp_index),
+					 NULL, t_label);
 	    gimple_switch_set_label (switch_stmt, disp_index, case_elt);
 
 	    label = label_rtx (t_label);
diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c
index 08207e0..c6698c5 100644
--- a/gcc/fortran/trans-decl.c
+++ b/gcc/fortran/trans-decl.c
@@ -4298,7 +4298,7 @@  gfc_trans_entry_master_switch (gfc_entry_list * el)
       /* Add the case label.  */
       label = gfc_build_label_decl (NULL_TREE);
       val = build_int_cst (gfc_array_index_type, el->id);
-      tmp = build3_v (CASE_LABEL_EXPR, val, NULL_TREE, label);
+      tmp = build_case_label (val, NULL_TREE, label);
       gfc_add_expr_to_block (&block, tmp);
 
       /* And jump to the actual entry point.  */
diff --git a/gcc/fortran/trans-io.c b/gcc/fortran/trans-io.c
index f6a783f..023e0a8 100644
--- a/gcc/fortran/trans-io.c
+++ b/gcc/fortran/trans-io.c
@@ -838,7 +838,7 @@  add_case (int label_value, gfc_st_label * label, stmtblock_t * body)
   tmp = gfc_build_label_decl (NULL_TREE);
 
   /* And the case itself.  */
-  tmp = build3_v (CASE_LABEL_EXPR, value, NULL_TREE, tmp);
+  tmp = build_case_label (value, NULL_TREE, tmp);
   gfc_add_expr_to_block (body, tmp);
 
   /* Jump to the label.  */
diff --git a/gcc/fortran/trans-stmt.c b/gcc/fortran/trans-stmt.c
index e120285..41c2d0e 100644
--- a/gcc/fortran/trans-stmt.c
+++ b/gcc/fortran/trans-stmt.c
@@ -1625,8 +1625,7 @@  gfc_trans_integer_select (gfc_code * code)
 
 	  /* Add this case label.
              Add parameter 'label', make it match GCC backend.  */
-	  tmp = fold_build3_loc (input_location, CASE_LABEL_EXPR,
-				 void_type_node, low, high, label);
+	  tmp = build_case_label (low, high, label);
 	  gfc_add_expr_to_block (&body, tmp);
 	}
 
@@ -1903,8 +1902,7 @@  gfc_trans_character_select (gfc_code *code)
 
 		  /* Add this case label.
 		     Add parameter 'label', make it match GCC backend.  */
-		  tmp = fold_build3_loc (input_location, CASE_LABEL_EXPR,
-					 void_type_node, low, high, label);
+		  tmp = build_case_label (low, high, label);
 		  gfc_add_expr_to_block (&body, tmp);
 		}
 
@@ -1983,11 +1981,9 @@  gfc_trans_character_select (gfc_code *code)
       for (d = c->ext.block.case_list; d; d = d->next)
         {
 	  label = gfc_build_label_decl (NULL_TREE);
-	  tmp = fold_build3_loc (input_location, CASE_LABEL_EXPR,
-				 void_type_node,
-				 (d->low == NULL && d->high == NULL)
-				 ? NULL : build_int_cst (NULL_TREE, d->n),
-				 NULL, label);
+	  tmp = build_case_label ((d->low == NULL && d->high == NULL)
+				  ? NULL : build_int_cst (NULL_TREE, d->n),
+				  NULL, label);
           gfc_add_expr_to_block (&body, tmp);
         }
 
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 350d793..89e647d 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1582,10 +1582,11 @@  gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p)
 			break;
 		    }
 		  if (i == len)
-		    default_case = build3 (CASE_LABEL_EXPR, void_type_node,
-					   NULL_TREE, NULL_TREE,
-					   CASE_LABEL (VEC_index (tree,
-								  labels, 0)));
+		    {
+		      tree label = CASE_LABEL (VEC_index (tree, labels, 0));
+		      default_case = build_case_label (NULL_TREE, NULL_TREE,
+						       label);
+		    }
 		}
 	    }
 
@@ -1594,9 +1595,8 @@  gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p)
 	      gimple new_default;
 
 	      default_case
-		= build3 (CASE_LABEL_EXPR, void_type_node,
-			  NULL_TREE, NULL_TREE,
-			  create_artificial_label (UNKNOWN_LOCATION));
+		= build_case_label (NULL_TREE, NULL_TREE,
+				    create_artificial_label (UNKNOWN_LOCATION));
 	      new_default = gimple_build_label (CASE_LABEL (default_case));
 	      gimplify_seq_add_stmt (&switch_body_seq, new_default);
 	    }
diff --git a/gcc/java/expr.c b/gcc/java/expr.c
index 53feab5..3be1cff 100644
--- a/gcc/java/expr.c
+++ b/gcc/java/expr.c
@@ -1874,8 +1874,8 @@  expand_java_switch (tree selector, int default_pc)
 			NULL_TREE, NULL_TREE);
   java_add_stmt (switch_expr);
 
-  x = build3 (CASE_LABEL_EXPR, void_type_node, NULL_TREE, NULL_TREE,
-	      create_artificial_label (input_location));
+  x = build_case_label (NULL_TREE, NULL_TREE,
+			create_artificial_label (input_location));
   append_to_statement_list (x, &SWITCH_BODY (switch_expr));
 
   x = build1 (GOTO_EXPR, void_type_node, lookup_label (default_pc));
@@ -1891,8 +1891,8 @@  expand_java_add_case (tree switch_expr, int match, int target_pc)
 
   value = build_int_cst (TREE_TYPE (switch_expr), match);
   
-  x = build3 (CASE_LABEL_EXPR, void_type_node, value, NULL_TREE,
-	      create_artificial_label (input_location));
+  x = build_case_label (value, NULL_TREE,
+			create_artificial_label (input_location));
   append_to_statement_list (x, &SWITCH_BODY (switch_expr));
 
   x = build1 (GOTO_EXPR, void_type_node, lookup_label (target_pc));
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index c3f2178..13af7fc 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -4773,8 +4773,7 @@  expand_omp_sections (struct omp_region *region)
   i = 0;
   if (exit_reachable)
     {
-      t = build3 (CASE_LABEL_EXPR, void_type_node,
-		  build_int_cst (unsigned_type_node, 0), NULL, l2);
+      t = build_case_label (build_int_cst (unsigned_type_node, 0), NULL, l2);
       VEC_quick_push (tree, label_vec, t);
       i++;
     }
@@ -4799,7 +4798,7 @@  expand_omp_sections (struct omp_region *region)
 
       t = gimple_block_label (s_entry_bb);
       u = build_int_cst (unsigned_type_node, casei);
-      u = build3 (CASE_LABEL_EXPR, void_type_node, u, NULL, t);
+      u = build_case_label (u, NULL, t);
       VEC_quick_push (tree, label_vec, u);
 
       si = gsi_last_bb (s_entry_bb);
@@ -4820,7 +4819,7 @@  expand_omp_sections (struct omp_region *region)
 
   /* Error handling code goes in DEFAULT_BB.  */
   t = gimple_block_label (default_bb);
-  u = build3 (CASE_LABEL_EXPR, void_type_node, NULL, NULL, t);
+  u = build_case_label (NULL, NULL, t);
   make_edge (l0_bb, default_bb, 0);
 
   stmt = gimple_build_switch_vec (vmain, u, label_vec);
diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
index f189b9b..a09e6a6 100644
--- a/gcc/tree-eh.c
+++ b/gcc/tree-eh.c
@@ -1337,9 +1337,8 @@  lower_try_finally_switch (struct leh_state *state, struct leh_tf_state *tf)
 			       build_int_cst (NULL, fallthru_index));
       gimple_seq_add_stmt (&tf->top_p_seq, x);
 
-      last_case = build3 (CASE_LABEL_EXPR, void_type_node,
-			  build_int_cst (NULL, fallthru_index),
-			  NULL, create_artificial_label (tf_loc));
+      last_case = build_case_label (build_int_cst (NULL, fallthru_index),
+				    NULL, create_artificial_label (tf_loc));
       VEC_quick_push (tree, case_label_vec, last_case);
       last_case_index++;
 
@@ -1362,9 +1361,8 @@  lower_try_finally_switch (struct leh_state *state, struct leh_tf_state *tf)
       x = gimple_build_goto (finally_label);
       gimple_seq_add_stmt (&eh_seq, x);
 
-      last_case = build3 (CASE_LABEL_EXPR, void_type_node,
-			  build_int_cst (NULL, eh_index),
-			  NULL, create_artificial_label (tf_loc));
+      last_case = build_case_label (build_int_cst (NULL, eh_index),
+				    NULL, create_artificial_label (tf_loc));
       VEC_quick_push (tree, case_label_vec, last_case);
       last_case_index++;
 
@@ -1415,9 +1413,8 @@  lower_try_finally_switch (struct leh_state *state, struct leh_tf_state *tf)
         {
           tree case_lab;
           void **slot;
-          case_lab = build3 (CASE_LABEL_EXPR, void_type_node,
-                             build_int_cst (NULL, switch_id),
-			     NULL, NULL);
+          case_lab = build_case_label (build_int_cst (NULL, switch_id),
+				       NULL, NULL);
           /* We store the cont_stmt in the pointer map, so that we can recover
              it in the loop below.  We don't create the new label while
              walking the goto_queue because pointers don't offer a stable
@@ -3136,8 +3133,8 @@  lower_eh_dispatch (basic_block src, gimple stmt)
 		   blocks at the end of this pass.  */
 		if (! pointer_set_contains (seen_values, TREE_VALUE (flt_node)))
 		  {
-		    tree t = build3 (CASE_LABEL_EXPR, void_type_node,
-				     TREE_VALUE (flt_node), NULL, lab);
+		    tree t = build_case_label (TREE_VALUE (flt_node),
+					       NULL, lab);
 		    VEC_safe_push (tree, heap, labels, t);
 		    pointer_set_insert (seen_values, TREE_VALUE (flt_node));
 		    have_label = true;
@@ -3184,8 +3181,7 @@  lower_eh_dispatch (basic_block src, gimple stmt)
 	    gsi_insert_before (&gsi, x, GSI_SAME_STMT);
 
 	    /* Turn the default label into a default case.  */
-	    default_label = build3 (CASE_LABEL_EXPR, void_type_node,
-				    NULL, NULL, default_label);
+	    default_label = build_case_label (NULL, NULL, default_label);
 	    sort_case_labels (labels);
 
 	    x = gimple_build_switch_vec (filter, default_label, labels);
diff --git a/gcc/tree.c b/gcc/tree.c
index da16641..d63e7d5 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -1629,6 +1629,23 @@  make_tree_binfo_stat (unsigned base_binfos MEM_STAT_DECL)
   return t;
 }
 
+/* Create a CASE_LABEL_EXPR tree node and return it.  */
+
+tree
+build_case_label (tree low_value, tree high_value, tree label_decl)
+{
+  tree t = make_node (CASE_LABEL_EXPR);
+
+  TREE_TYPE (t) = void_type_node;
+  SET_EXPR_LOCATION (t, input_location);
+
+  CASE_LOW (t) = low_value;
+  CASE_HIGH (t) = high_value;
+  CASE_LABEL (t) = label_decl;
+  CASE_CHAIN (t) = NULL_TREE;
+
+  return t;
+}
 
 /* Build a newly constructed TREE_VEC node of length LEN.  */
 
diff --git a/gcc/tree.h b/gcc/tree.h
index 58b3b9d..3e1ff2c 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -4033,6 +4033,10 @@  extern tree copy_node_stat (tree MEM_STAT_DECL);
 
 extern tree copy_list (tree);
 
+/* Make a CASE_LABEL_EXPR.  */
+
+extern tree build_case_label (tree, tree, tree);
+
 /* Make a BINFO.  */
 extern tree make_tree_binfo_stat (unsigned MEM_STAT_DECL);
 #define make_tree_binfo(t) make_tree_binfo_stat (t MEM_STAT_INFO)