[PATCH] Simplified switch conversion in simple cases

[PATCH] Simplified switch conversion in simple cases

[Martin Jambor]

Hi,

this patch implements what Richi asked for in

http://gcc.gnu.org/ml/gcc-patches/2009-04/msg01204.html

When there is only one value  in the switch conversion array, than the
pass does not need  to generate a static array and a  load from it but
can load the variable with that constant straight away.

I also took  this opportunity to clean up the  rather wild marking for
renaming  in  the  patch and  simply  replaced  them  with a  call  to
update_stmt at  appropriate places.   Since Richi also  discouraged me
from   using   make_rename-temp,  I   now   create  temporaries   with
create_tmp_var and create ssa names myself since it is easy.

Bootstrapped and regression tested on linux-x86_64.  OK for trunk?

Thanks,

Martin


2009-04-20  Martin Jambor  <mjambor@suse.cz>

	* tree-switch-conversion.c (build_constructors): Split a long line.
	(constructor_contains_same_values_p): New function.
	(build_one_array): Create assigns of constants if possible, do not call
	mark_sym_for_renaming, call update_stmt.
	(build_arrays): Call make_ssa_name (create_tmp_var ()) instead of
	make_rename_temp.  Do not call mark_symbols_for_renaming, call
	update_stmt.
	(gen_def_assigns): Do not call mark_symbols_for_renaming or
	find_new_referenced_vars, call update_stmt.
	(gen_inbound_check): Use create_tmp_var and create ssa names manually
	instead of calling make_rename_temp.  Do not call
	find_new_referenced_vars or mark_symbols_for_renaming, call
	update_stmt.


Index: gcc/testsuite/gcc.dg/tree-ssa/cswtch-2.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/cswtch-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/cswtch-2.c	(revision 0)
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-switchconv" } */
+
+typedef enum { a = 5, b = 6, c = 7, d = 8, e = 9 } X;
+
+int h1 (X x)
+{
+  switch (x) {
+  case a:
+  case b:
+  case c:
+  case d:
+  case e:
+    return 1;
+  default:
+    return 0;
+    }
+}
+
+/* { dg-final { scan-tree-dump-times "CSWTCH" 0 "switchconv" } } */
+/* { dg-final { cleanup-tree-dump "switchconv" } } */
Index: gcc/tree-switch-conversion.c
===================================================================
--- gcc/tree-switch-conversion.c	(revision 146365)
+++ gcc/tree-switch-conversion.c	(working copy)
@@ -453,12 +453,35 @@ build_constructors (gimple swtch)
 	      elt->value = val;
 
 	      pos = int_const_binop (PLUS_EXPR, pos, integer_one_node, 0);
-	    } while (!tree_int_cst_lt (high, pos) && tree_int_cst_lt (low, pos));
+	    } while (!tree_int_cst_lt (high, pos)
+		     && tree_int_cst_lt (low, pos));
 	  j++;
 	}
     }
 }
 
+/* If all values in the constructor vector are the same, return the value.
+   Otherwise return NULL_TREE.  Not supposed to be called for empty
+   vectors.  */
+
+static tree
+constructor_contains_same_values_p (VEC (constructor_elt, gc) *vec)
+{
+  int i, len = VEC_length (constructor_elt, vec);
+  tree prev = NULL_TREE;
+
+  for (i = 0; i < len; i++)
+    {
+      constructor_elt *elt = VEC_index (constructor_elt, vec, i);
+      
+      if (!prev)
+	prev = elt->value;
+      else if (!operand_equal_p (elt->value, prev, OEP_ONLY_CONST))
+	return NULL_TREE;
+    }
+  return prev;
+}
+
 /* Create an appropriate array type and declaration and assemble a static array
    variable.  Also create a load statement that initializes the variable in
    question with a value from the static array.  SWTCH is the switch statement
@@ -466,47 +489,53 @@ build_constructors (gimple swtch)
    and target SSA names for this particular array.  ARR_INDEX_TYPE is the type
    of the index of the new array, PHI is the phi node of the final BB that
    corresponds to the value that will be loaded from the created array.  TIDX
-   is a temporary variable holding the index for loads from the new array.  */
+   is an ssa name of a temporary variable holding the index for loads from the
+   new array.  */
 
 static void
 build_one_array (gimple swtch, int num, tree arr_index_type, gimple phi,
 		 tree tidx)
 {
-  tree array_type, ctor, decl, value_type, name, fetch;
+  tree name, cst;
   gimple load;
-  gimple_stmt_iterator gsi;
+  gimple_stmt_iterator gsi = gsi_for_stmt (swtch);
 
   gcc_assert (info.default_values[num]);
-  value_type = TREE_TYPE (info.default_values[num]);
-  array_type = build_array_type (value_type, arr_index_type);
-
-  ctor = build_constructor (array_type, info.constructors[num]);
-  TREE_CONSTANT (ctor) = true;
-
-  decl = build_decl (VAR_DECL, NULL_TREE, array_type);
-  TREE_STATIC (decl) = 1;
-  DECL_INITIAL (decl) = ctor;
-
-  DECL_NAME (decl) = create_tmp_var_name ("CSWTCH");
-  DECL_ARTIFICIAL (decl) = 1;
-  TREE_CONSTANT (decl) = 1;
-  add_referenced_var (decl);
-  varpool_mark_needed_node (varpool_node (decl));
-  varpool_finalize_decl (decl);
-  mark_sym_for_renaming (decl);
 
   name = make_ssa_name (SSA_NAME_VAR (PHI_RESULT (phi)), NULL);
   info.target_inbound_names[num] = name;
 
-  fetch = build4 (ARRAY_REF, value_type, decl, tidx, NULL_TREE,
-		  NULL_TREE);
-  load = gimple_build_assign (name, fetch);
-  SSA_NAME_DEF_STMT (name) = load;
+  cst = constructor_contains_same_values_p (info.constructors[num]);
+  if (cst)
+    load = gimple_build_assign (name, cst);
+  else
+    {
+      tree array_type, ctor, decl, value_type, fetch;
 
-  gsi = gsi_for_stmt (swtch);
-  gsi_insert_before (&gsi, load, GSI_SAME_STMT);
-  mark_symbols_for_renaming (load);
+      value_type = TREE_TYPE (info.default_values[num]);
+      array_type = build_array_type (value_type, arr_index_type);
+      ctor = build_constructor (array_type, info.constructors[num]);
+      TREE_CONSTANT (ctor) = true;
+
+      decl = build_decl (VAR_DECL, NULL_TREE, array_type);
+      TREE_STATIC (decl) = 1;
+      DECL_INITIAL (decl) = ctor;
+
+      DECL_NAME (decl) = create_tmp_var_name ("CSWTCH");
+      DECL_ARTIFICIAL (decl) = 1;
+      TREE_CONSTANT (decl) = 1;
+      add_referenced_var (decl);
+      varpool_mark_needed_node (varpool_node (decl));
+      varpool_finalize_decl (decl);
+
+      fetch = build4 (ARRAY_REF, value_type, decl, tidx, NULL_TREE,
+		      NULL_TREE);
+      load = gimple_build_assign (name, fetch);
+    }
 
+  SSA_NAME_DEF_STMT (name) = load;
+  gsi_insert_before (&gsi, load, GSI_SAME_STMT);
+  update_stmt (load);
   info.arr_ref_last = load;
 }
 
@@ -526,16 +555,17 @@ build_arrays (gimple swtch)
   gsi = gsi_for_stmt (swtch);
 
   arr_index_type = build_index_type (info.range_size);
-  tidx = make_rename_temp (arr_index_type, "csti");
+  tidx = make_ssa_name (create_tmp_var (arr_index_type, "csti"), NULL);
   sub = fold_build2 (MINUS_EXPR, TREE_TYPE (info.index_expr), info.index_expr,
 		     fold_convert (TREE_TYPE (info.index_expr),
 				   info.range_min));
   sub = force_gimple_operand_gsi (&gsi, fold_convert (arr_index_type, sub),
 				  false, NULL, true, GSI_SAME_STMT);
   stmt = gimple_build_assign (tidx, sub);
+  SSA_NAME_DEF_STMT (tidx) = stmt;
 
   gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
-  mark_symbols_for_renaming (stmt);
+  update_stmt (stmt);
   info.arr_ref_first = stmt;
 
   for (gsi = gsi_start_phis (info.final_bb), i = 0;
@@ -561,8 +591,7 @@ gen_def_assigns (gimple_stmt_iterator *g
       assign = gimple_build_assign (name, info.default_values[i]);
       SSA_NAME_DEF_STMT (name) = assign;
       gsi_insert_before (gsi, assign, GSI_SAME_STMT);
-      find_new_referenced_vars (assign);
-      mark_symbols_for_renaming (assign);
+      update_stmt (assign);
     }
   return assign;
 }
@@ -640,7 +669,7 @@ gen_inbound_check (gimple swtch)
   gimple label1, label2, label3;
 
   tree utype;
-  tree tmp_u;
+  tree tmp_u_1, tmp_u_2, tmp_u_var;
   tree cast;
   gimple cast_assign, minus_assign;
   tree ulb, minus;
@@ -664,30 +693,29 @@ gen_inbound_check (gimple swtch)
 
   /* (end of) block 0 */
   gsi = gsi_for_stmt (info.arr_ref_first);
-  tmp_u = make_rename_temp (utype, "csui");
+  tmp_u_var = create_tmp_var (utype, "csui");
+  tmp_u_1 = make_ssa_name (tmp_u_var, NULL);
 
   cast = fold_convert (utype, info.index_expr);
-  cast_assign = gimple_build_assign (tmp_u, cast);
-  find_new_referenced_vars (cast_assign);
+  cast_assign = gimple_build_assign (tmp_u_1, cast);
+  SSA_NAME_DEF_STMT (tmp_u_1) = cast_assign;
   gsi_insert_before (&gsi, cast_assign, GSI_SAME_STMT);
-  mark_symbols_for_renaming (cast_assign);
+  update_stmt (cast_assign);
 
   ulb = fold_convert (utype, info.range_min);
-  minus = fold_build2 (MINUS_EXPR, utype, tmp_u, ulb);
+  minus = fold_build2 (MINUS_EXPR, utype, tmp_u_1, ulb);
   minus = force_gimple_operand_gsi (&gsi, minus, false, NULL, true,
 				    GSI_SAME_STMT);
-  minus_assign = gimple_build_assign (tmp_u, minus);
-  find_new_referenced_vars (minus_assign);
+  tmp_u_2 = make_ssa_name (tmp_u_var, NULL);
+  minus_assign = gimple_build_assign (tmp_u_2, minus);
+  SSA_NAME_DEF_STMT (tmp_u_2) = minus_assign;
   gsi_insert_before (&gsi, minus_assign, GSI_SAME_STMT);
-  mark_symbols_for_renaming (minus_assign);
+  update_stmt (minus_assign);
 
   bound = fold_convert (utype, info.range_size);
-
-  cond_stmt = gimple_build_cond (LE_EXPR, tmp_u, bound, NULL_TREE, NULL_TREE);
-
-  find_new_referenced_vars (cond_stmt);
+  cond_stmt = gimple_build_cond (LE_EXPR, tmp_u_2, bound, NULL_TREE, NULL_TREE);
   gsi_insert_before (&gsi, cond_stmt, GSI_SAME_STMT);
-  mark_symbols_for_renaming (cond_stmt);
+  update_stmt (cond_stmt);
 
   /* block 2 */
   gsi = gsi_for_stmt (info.arr_ref_first);

Re: [PATCH] Simplified switch conversion in simple cases

[Richard Guenther]

On Mon, 20 Apr 2009, Martin Jambor wrote:

> Hi,
> 
> this patch implements what Richi asked for in
> 
> http://gcc.gnu.org/ml/gcc-patches/2009-04/msg01204.html
> 
> When there is only one value  in the switch conversion array, than the
> pass does not need  to generate a static array and a  load from it but
> can load the variable with that constant straight away.
> 
> I also took  this opportunity to clean up the  rather wild marking for
> renaming  in  the  patch and  simply  replaced  them  with a  call  to
> update_stmt at  appropriate places.   Since Richi also  discouraged me
> from   using   make_rename-temp,  I   now   create  temporaries   with
> create_tmp_var and create ssa names myself since it is easy.
> 
> Bootstrapped and regression tested on linux-x86_64.  OK for trunk?

Ok.

Many thanks!
Richard.

> Thanks,
> 
> Martin
> 
> 
> 2009-04-20  Martin Jambor  <mjambor@suse.cz>
> 
> 	* tree-switch-conversion.c (build_constructors): Split a long line.
> 	(constructor_contains_same_values_p): New function.
> 	(build_one_array): Create assigns of constants if possible, do not call
> 	mark_sym_for_renaming, call update_stmt.
> 	(build_arrays): Call make_ssa_name (create_tmp_var ()) instead of
> 	make_rename_temp.  Do not call mark_symbols_for_renaming, call
> 	update_stmt.
> 	(gen_def_assigns): Do not call mark_symbols_for_renaming or
> 	find_new_referenced_vars, call update_stmt.
> 	(gen_inbound_check): Use create_tmp_var and create ssa names manually
> 	instead of calling make_rename_temp.  Do not call
> 	find_new_referenced_vars or mark_symbols_for_renaming, call
> 	update_stmt.
> 
> 
> Index: gcc/testsuite/gcc.dg/tree-ssa/cswtch-2.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/cswtch-2.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/tree-ssa/cswtch-2.c	(revision 0)
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-switchconv" } */
> +
> +typedef enum { a = 5, b = 6, c = 7, d = 8, e = 9 } X;
> +
> +int h1 (X x)
> +{
> +  switch (x) {
> +  case a:
> +  case b:
> +  case c:
> +  case d:
> +  case e:
> +    return 1;
> +  default:
> +    return 0;
> +    }
> +}
> +
> +/* { dg-final { scan-tree-dump-times "CSWTCH" 0 "switchconv" } } */
> +/* { dg-final { cleanup-tree-dump "switchconv" } } */
> Index: gcc/tree-switch-conversion.c
> ===================================================================
> --- gcc/tree-switch-conversion.c	(revision 146365)
> +++ gcc/tree-switch-conversion.c	(working copy)
> @@ -453,12 +453,35 @@ build_constructors (gimple swtch)
>  	      elt->value = val;
>  
>  	      pos = int_const_binop (PLUS_EXPR, pos, integer_one_node, 0);
> -	    } while (!tree_int_cst_lt (high, pos) && tree_int_cst_lt (low, pos));
> +	    } while (!tree_int_cst_lt (high, pos)
> +		     && tree_int_cst_lt (low, pos));
>  	  j++;
>  	}
>      }
>  }
>  
> +/* If all values in the constructor vector are the same, return the value.
> +   Otherwise return NULL_TREE.  Not supposed to be called for empty
> +   vectors.  */
> +
> +static tree
> +constructor_contains_same_values_p (VEC (constructor_elt, gc) *vec)
> +{
> +  int i, len = VEC_length (constructor_elt, vec);
> +  tree prev = NULL_TREE;
> +
> +  for (i = 0; i < len; i++)
> +    {
> +      constructor_elt *elt = VEC_index (constructor_elt, vec, i);
> +      
> +      if (!prev)
> +	prev = elt->value;
> +      else if (!operand_equal_p (elt->value, prev, OEP_ONLY_CONST))
> +	return NULL_TREE;
> +    }
> +  return prev;
> +}
> +
>  /* Create an appropriate array type and declaration and assemble a static array
>     variable.  Also create a load statement that initializes the variable in
>     question with a value from the static array.  SWTCH is the switch statement
> @@ -466,47 +489,53 @@ build_constructors (gimple swtch)
>     and target SSA names for this particular array.  ARR_INDEX_TYPE is the type
>     of the index of the new array, PHI is the phi node of the final BB that
>     corresponds to the value that will be loaded from the created array.  TIDX
> -   is a temporary variable holding the index for loads from the new array.  */
> +   is an ssa name of a temporary variable holding the index for loads from the
> +   new array.  */
>  
>  static void
>  build_one_array (gimple swtch, int num, tree arr_index_type, gimple phi,
>  		 tree tidx)
>  {
> -  tree array_type, ctor, decl, value_type, name, fetch;
> +  tree name, cst;
>    gimple load;
> -  gimple_stmt_iterator gsi;
> +  gimple_stmt_iterator gsi = gsi_for_stmt (swtch);
>  
>    gcc_assert (info.default_values[num]);
> -  value_type = TREE_TYPE (info.default_values[num]);
> -  array_type = build_array_type (value_type, arr_index_type);
> -
> -  ctor = build_constructor (array_type, info.constructors[num]);
> -  TREE_CONSTANT (ctor) = true;
> -
> -  decl = build_decl (VAR_DECL, NULL_TREE, array_type);
> -  TREE_STATIC (decl) = 1;
> -  DECL_INITIAL (decl) = ctor;
> -
> -  DECL_NAME (decl) = create_tmp_var_name ("CSWTCH");
> -  DECL_ARTIFICIAL (decl) = 1;
> -  TREE_CONSTANT (decl) = 1;
> -  add_referenced_var (decl);
> -  varpool_mark_needed_node (varpool_node (decl));
> -  varpool_finalize_decl (decl);
> -  mark_sym_for_renaming (decl);
>  
>    name = make_ssa_name (SSA_NAME_VAR (PHI_RESULT (phi)), NULL);
>    info.target_inbound_names[num] = name;
>  
> -  fetch = build4 (ARRAY_REF, value_type, decl, tidx, NULL_TREE,
> -		  NULL_TREE);
> -  load = gimple_build_assign (name, fetch);
> -  SSA_NAME_DEF_STMT (name) = load;
> +  cst = constructor_contains_same_values_p (info.constructors[num]);
> +  if (cst)
> +    load = gimple_build_assign (name, cst);
> +  else
> +    {
> +      tree array_type, ctor, decl, value_type, fetch;
>  
> -  gsi = gsi_for_stmt (swtch);
> -  gsi_insert_before (&gsi, load, GSI_SAME_STMT);
> -  mark_symbols_for_renaming (load);
> +      value_type = TREE_TYPE (info.default_values[num]);
> +      array_type = build_array_type (value_type, arr_index_type);
> +      ctor = build_constructor (array_type, info.constructors[num]);
> +      TREE_CONSTANT (ctor) = true;
> +
> +      decl = build_decl (VAR_DECL, NULL_TREE, array_type);
> +      TREE_STATIC (decl) = 1;
> +      DECL_INITIAL (decl) = ctor;
> +
> +      DECL_NAME (decl) = create_tmp_var_name ("CSWTCH");
> +      DECL_ARTIFICIAL (decl) = 1;
> +      TREE_CONSTANT (decl) = 1;
> +      add_referenced_var (decl);
> +      varpool_mark_needed_node (varpool_node (decl));
> +      varpool_finalize_decl (decl);
> +
> +      fetch = build4 (ARRAY_REF, value_type, decl, tidx, NULL_TREE,
> +		      NULL_TREE);
> +      load = gimple_build_assign (name, fetch);
> +    }
>  
> +  SSA_NAME_DEF_STMT (name) = load;
> +  gsi_insert_before (&gsi, load, GSI_SAME_STMT);
> +  update_stmt (load);
>    info.arr_ref_last = load;
>  }
>  
> @@ -526,16 +555,17 @@ build_arrays (gimple swtch)
>    gsi = gsi_for_stmt (swtch);
>  
>    arr_index_type = build_index_type (info.range_size);
> -  tidx = make_rename_temp (arr_index_type, "csti");
> +  tidx = make_ssa_name (create_tmp_var (arr_index_type, "csti"), NULL);
>    sub = fold_build2 (MINUS_EXPR, TREE_TYPE (info.index_expr), info.index_expr,
>  		     fold_convert (TREE_TYPE (info.index_expr),
>  				   info.range_min));
>    sub = force_gimple_operand_gsi (&gsi, fold_convert (arr_index_type, sub),
>  				  false, NULL, true, GSI_SAME_STMT);
>    stmt = gimple_build_assign (tidx, sub);
> +  SSA_NAME_DEF_STMT (tidx) = stmt;
>  
>    gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
> -  mark_symbols_for_renaming (stmt);
> +  update_stmt (stmt);
>    info.arr_ref_first = stmt;
>  
>    for (gsi = gsi_start_phis (info.final_bb), i = 0;
> @@ -561,8 +591,7 @@ gen_def_assigns (gimple_stmt_iterator *g
>        assign = gimple_build_assign (name, info.default_values[i]);
>        SSA_NAME_DEF_STMT (name) = assign;
>        gsi_insert_before (gsi, assign, GSI_SAME_STMT);
> -      find_new_referenced_vars (assign);
> -      mark_symbols_for_renaming (assign);
> +      update_stmt (assign);
>      }
>    return assign;
>  }
> @@ -640,7 +669,7 @@ gen_inbound_check (gimple swtch)
>    gimple label1, label2, label3;
>  
>    tree utype;
> -  tree tmp_u;
> +  tree tmp_u_1, tmp_u_2, tmp_u_var;
>    tree cast;
>    gimple cast_assign, minus_assign;
>    tree ulb, minus;
> @@ -664,30 +693,29 @@ gen_inbound_check (gimple swtch)
>  
>    /* (end of) block 0 */
>    gsi = gsi_for_stmt (info.arr_ref_first);
> -  tmp_u = make_rename_temp (utype, "csui");
> +  tmp_u_var = create_tmp_var (utype, "csui");
> +  tmp_u_1 = make_ssa_name (tmp_u_var, NULL);
>  
>    cast = fold_convert (utype, info.index_expr);
> -  cast_assign = gimple_build_assign (tmp_u, cast);
> -  find_new_referenced_vars (cast_assign);
> +  cast_assign = gimple_build_assign (tmp_u_1, cast);
> +  SSA_NAME_DEF_STMT (tmp_u_1) = cast_assign;
>    gsi_insert_before (&gsi, cast_assign, GSI_SAME_STMT);
> -  mark_symbols_for_renaming (cast_assign);
> +  update_stmt (cast_assign);
>  
>    ulb = fold_convert (utype, info.range_min);
> -  minus = fold_build2 (MINUS_EXPR, utype, tmp_u, ulb);
> +  minus = fold_build2 (MINUS_EXPR, utype, tmp_u_1, ulb);
>    minus = force_gimple_operand_gsi (&gsi, minus, false, NULL, true,
>  				    GSI_SAME_STMT);
> -  minus_assign = gimple_build_assign (tmp_u, minus);
> -  find_new_referenced_vars (minus_assign);
> +  tmp_u_2 = make_ssa_name (tmp_u_var, NULL);
> +  minus_assign = gimple_build_assign (tmp_u_2, minus);
> +  SSA_NAME_DEF_STMT (tmp_u_2) = minus_assign;
>    gsi_insert_before (&gsi, minus_assign, GSI_SAME_STMT);
> -  mark_symbols_for_renaming (minus_assign);
> +  update_stmt (minus_assign);
>  
>    bound = fold_convert (utype, info.range_size);
> -
> -  cond_stmt = gimple_build_cond (LE_EXPR, tmp_u, bound, NULL_TREE, NULL_TREE);
> -
> -  find_new_referenced_vars (cond_stmt);
> +  cond_stmt = gimple_build_cond (LE_EXPR, tmp_u_2, bound, NULL_TREE, NULL_TREE);
>    gsi_insert_before (&gsi, cond_stmt, GSI_SAME_STMT);
> -  mark_symbols_for_renaming (cond_stmt);
> +  update_stmt (cond_stmt);
>  
>    /* block 2 */
>    gsi = gsi_for_stmt (info.arr_ref_first);
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex

Re: [PATCH] Simplified switch conversion in simple cases

[Martin Jambor]

On Tue, Apr 21, 2009 at 10:50:45AM +0200, Richard Guenther wrote:
> On Mon, 20 Apr 2009, Martin Jambor wrote:
> 
> > Hi,
> > 
> > this patch implements what Richi asked for in
> > 
> > http://gcc.gnu.org/ml/gcc-patches/2009-04/msg01204.html
> > 
> > When there is only one value  in the switch conversion array, than the
> > pass does not need  to generate a static array and a  load from it but
> > can load the variable with that constant straight away.
> > 
> > I also took  this opportunity to clean up the  rather wild marking for
> > renaming  in  the  patch and  simply  replaced  them  with a  call  to
> > update_stmt at  appropriate places.   Since Richi also  discouraged me
> > from   using   make_rename-temp,  I   now   create  temporaries   with
> > create_tmp_var and create ssa names myself since it is easy.
> > 
> > Bootstrapped and regression tested on linux-x86_64.  OK for trunk?
> 
> Ok.

Committed as revision 146517.

Thanks,

Martin

Re: [PATCH] Simplified switch conversion in simple cases

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 2467 bytes --]

> 2009-04-20  Martin Jambor  <mjambor@suse.cz>
>
> 	* tree-switch-conversion.c (build_constructors): Split a long line.
> 	(constructor_contains_same_values_p): New function.
> 	(build_one_array): Create assigns of constants if possible, do not call
> 	mark_sym_for_renaming, call update_stmt.
> 	(build_arrays): Call make_ssa_name (create_tmp_var ()) instead of
> 	make_rename_temp.  Do not call mark_symbols_for_renaming, call
> 	update_stmt.
> 	(gen_def_assigns): Do not call mark_symbols_for_renaming or
> 	find_new_referenced_vars, call update_stmt.
> 	(gen_inbound_check): Use create_tmp_var and create ssa names manually
> 	instead of calling make_rename_temp.  Do not call
> 	find_new_referenced_vars or mark_symbols_for_renaming, call
> 	update_stmt.

This breaks Ada on x86:

/home/eric/build/gcc/native32/./gcc/xgcc -B/home/eric/build/gcc/native32/./gcc/ -B/home/eric/install/gcc/i586-suse-linux/bin/ -B/home/eric/install/gcc/i586-suse-linux/lib/ -isystem /home/eric/install/gcc/i586-suse-linux/include -isystem /home/eric/install/gcc/i586-suse-linux/sys-include -c -g -O2   -W -Wall -gnatpg   
g-socket.adb -o g-socket.o
+===========================GNAT BUG DETECTED==============================+
| 4.5.0 20090421 (experimental) [trunk revision 146532] (i586-suse-linux-gnu) 
GCC error:|
| in make_decl_rtl, at varasm.c:1304                                       |
| Error detected around g-socket.adb:1746                                  |
| Please submit a bug report; see http://gcc.gnu.org/bugs.html.            |

Reduced testcase attached, compile with:

gcc/xgcc -Bgcc -S g-socket.adb -Igcc/ada/rts -O


(gdb) frame 2
#2  0x08cfa656 in make_decl_rtl (decl=0xf7e050b8)
    at /home/eric/svn/gcc/gcc/varasm.c:1300
1300      gcc_assert (TREE_CODE (decl) != VAR_DECL
(gdb) p debug_tree(decl)
 <var_decl 0xf7e050b8 csui.11
    type <integer_type 0xf7d31bd0 unsigned int sizes-gimplified public visited 
unsigned SI
        size <integer_cst 0xf7d1c604 constant 32>
        unit size <integer_cst 0xf7d1c3f0 constant 4>
        align 32 symtab 0 alias set -1 canonical type 0xf7d31bd0 precision 32 
min <integer_cst 0xf7d1cd90 0> max <integer_cst 0xf7d1cb98 4294967295>>
    used unsigned ignored SI file g-socket.adb line 52 col 1 size <integer_cst 
0xf7d1c604 32> unit size <integer_cst 0xf7d1c3f0 4>
    align 32 context <function_decl 0xf7ddd800 
gnat__sockets__resolve_exception> abstract_origin <var_decl 0xf7df89b4 
csui.11>>

-- 
Eric Botcazou

[-- Attachment #2: g-socket.adb --]
[-- Type: text/x-adasrc, Size: 1964 bytes --]

with System.OS_Constants;

package body GNAT.Sockets is

   package SOSC renames System.OS_Constants;

   function Resolve_Error
     (Error_Value : Integer;
      From_Errno  : Boolean) return Error_Type
   is
      use SOSC;
   begin
      if not From_Errno then
         case Error_Value is
            when SOSC.HOST_NOT_FOUND => return Unknown_Host;
            when SOSC.TRY_AGAIN      => return Host_Name_Lookup_Failure;
            when SOSC.NO_RECOVERY    => return Non_Recoverable_Error;
            when SOSC.NO_DATA        => return Unknown_Server_Error;
            when others              => return Cannot_Resolve_Error;
         end case;
      end if;

      case Error_Value is
         when EACCES          => return Permission_Denied;
         when EADDRINUSE      => return Address_Already_In_Use;
         when EADDRNOTAVAIL   => return Cannot_Assign_Requested_Address;
         when EAFNOSUPPORT    => return
                                 Address_Family_Not_Supported_By_Protocol;
         when EALREADY        => return Operation_Already_In_Progress;
         when EBADF           => return Bad_File_Descriptor;
         when ECONNABORTED    => return Software_Caused_Connection_Abort;
         when ECONNREFUSED    => return Connection_Refused;
         when ECONNRESET      => return Connection_Reset_By_Peer;
         when EDESTADDRREQ    => return Destination_Address_Required;
         when EFAULT          => return Bad_Address;
         when EHOSTDOWN       => return Host_Is_Down;
         when EHOSTUNREACH    => return No_Route_To_Host;
         when others          => return Cannot_Resolve_Error;
      end case;
   end;

   function Resolve_Exception
     (Occurrence : Exception_Occurrence) return Error_Type
   is
      Msg   : constant String := Exception_Message (Occurrence);
      Val   : Integer;
   begin
      Val := Integer'Value (Msg (1 .. 4));
      return Resolve_Error (Val, False);
   end;

end GNAT.Sockets;

[-- Attachment #3: g-socket.ads --]
[-- Type: text/x-adasrc, Size: 1617 bytes --]

with Ada.Exceptions; use Ada.Exceptions;

package GNAT.Sockets is

   type Error_Type is
     (Success,
      Permission_Denied,
      Address_Already_In_Use,
      Cannot_Assign_Requested_Address,
      Address_Family_Not_Supported_By_Protocol,
      Operation_Already_In_Progress,
      Bad_File_Descriptor,
      Software_Caused_Connection_Abort,
      Connection_Refused,
      Connection_Reset_By_Peer,
      Destination_Address_Required,
      Bad_Address,
      Host_Is_Down,
      No_Route_To_Host,
      Operation_Now_In_Progress,
      Interrupted_System_Call,
      Invalid_Argument,
      Input_Output_Error,
      Transport_Endpoint_Already_Connected,
      Too_Many_Symbolic_Links,
      Too_Many_Open_Files,
      Message_Too_Long,
      File_Name_Too_Long,
      Network_Is_Down,
      Network_Dropped_Connection_Because_Of_Reset,
      Network_Is_Unreachable,
      No_Buffer_Space_Available,
      Protocol_Not_Available,
      Transport_Endpoint_Not_Connected,
      Socket_Operation_On_Non_Socket,
      Operation_Not_Supported,
      Protocol_Family_Not_Supported,
      Protocol_Not_Supported,
      Protocol_Wrong_Type_For_Socket,
      Cannot_Send_After_Transport_Endpoint_Shutdown,
      Socket_Type_Not_Supported,
      Connection_Timed_Out,
      Too_Many_References,
      Resource_Temporarily_Unavailable,
      Broken_Pipe,
      Unknown_Host,
      Host_Name_Lookup_Failure,
      Non_Recoverable_Error,
      Unknown_Server_Error,
      Cannot_Resolve_Error);

   function Resolve_Exception
     (Occurrence : Ada.Exceptions.Exception_Occurrence) return Error_Type;

end GNAT.Sockets;

Re: [PATCH] Simplified switch conversion in simple cases

[Hans-Peter Nilsson]

On Mon, 20 Apr 2009, Martin Jambor wrote:
> 	* tree-switch-conversion.c (build_constructors): Split a long line.
> 	(constructor_contains_same_values_p): New function.
> 	(build_one_array): Create assigns of constants if possible, do not call
> 	mark_sym_for_renaming, call update_stmt.
> 	(build_arrays): Call make_ssa_name (create_tmp_var ()) instead of
> 	make_rename_temp.  Do not call mark_symbols_for_renaming, call
> 	update_stmt.
> 	(gen_def_assigns): Do not call mark_symbols_for_renaming or
> 	find_new_referenced_vars, call update_stmt.
> 	(gen_inbound_check): Use create_tmp_var and create ssa names manually
> 	instead of calling make_rename_temp.  Do not call
> 	find_new_referenced_vars or mark_symbols_for_renaming, call
> 	update_stmt.

This patch caused a regression, PR39845.

brgds, H-P

Re: [PATCH] Simplified switch conversion in simple cases

[Richard Guenther]

On Tue, 21 Apr 2009, Hans-Peter Nilsson wrote:

> On Mon, 20 Apr 2009, Martin Jambor wrote:
> > 	* tree-switch-conversion.c (build_constructors): Split a long line.
> > 	(constructor_contains_same_values_p): New function.
> > 	(build_one_array): Create assigns of constants if possible, do not call
> > 	mark_sym_for_renaming, call update_stmt.
> > 	(build_arrays): Call make_ssa_name (create_tmp_var ()) instead of
> > 	make_rename_temp.  Do not call mark_symbols_for_renaming, call
> > 	update_stmt.
> > 	(gen_def_assigns): Do not call mark_symbols_for_renaming or
> > 	find_new_referenced_vars, call update_stmt.
> > 	(gen_inbound_check): Use create_tmp_var and create ssa names manually
> > 	instead of calling make_rename_temp.  Do not call
> > 	find_new_referenced_vars or mark_symbols_for_renaming, call
> > 	update_stmt.
> 
> This patch caused a regression, PR39845.

The following fixes it.  Bootstrapped on x86_64-unknown-linux-gnu
(with assert checking), testing in progress.

I'll apply it after that succeeded.

Richard.

2009-04-22  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/39845
	* tree-switch-conversion.c (build_arrays): Add new referenced vars.
	(gen_inbound_check): Likewise.

	* gcc.c-torture/compile/pr39845.c: New testcase.

Index: gcc/tree-switch-conversion.c
===================================================================
*** gcc/tree-switch-conversion.c	(revision 146555)
--- gcc/tree-switch-conversion.c	(working copy)
*************** static void
*** 547,553 ****
  build_arrays (gimple swtch)
  {
    tree arr_index_type;
!   tree tidx, sub;
    gimple stmt;
    gimple_stmt_iterator gsi;
    int i;
--- 547,553 ----
  build_arrays (gimple swtch)
  {
    tree arr_index_type;
!   tree tidx, sub, tmp;
    gimple stmt;
    gimple_stmt_iterator gsi;
    int i;
*************** build_arrays (gimple swtch)
*** 555,561 ****
    gsi = gsi_for_stmt (swtch);
  
    arr_index_type = build_index_type (info.range_size);
!   tidx = make_ssa_name (create_tmp_var (arr_index_type, "csti"), NULL);
    sub = fold_build2 (MINUS_EXPR, TREE_TYPE (info.index_expr), info.index_expr,
  		     fold_convert (TREE_TYPE (info.index_expr),
  				   info.range_min));
--- 555,563 ----
    gsi = gsi_for_stmt (swtch);
  
    arr_index_type = build_index_type (info.range_size);
!   tmp = create_tmp_var (arr_index_type, "csti");
!   add_referenced_var (tmp);
!   tidx = make_ssa_name (tmp, NULL);
    sub = fold_build2 (MINUS_EXPR, TREE_TYPE (info.index_expr), info.index_expr,
  		     fold_convert (TREE_TYPE (info.index_expr),
  				   info.range_min));
*************** gen_inbound_check (gimple swtch)
*** 694,699 ****
--- 696,702 ----
    /* (end of) block 0 */
    gsi = gsi_for_stmt (info.arr_ref_first);
    tmp_u_var = create_tmp_var (utype, "csui");
+   add_referenced_var (tmp_u_var);
    tmp_u_1 = make_ssa_name (tmp_u_var, NULL);
  
    cast = fold_convert (utype, info.index_expr);
Index: gcc/testsuite/gcc.c-torture/compile/pr39845.c
===================================================================
*** gcc/testsuite/gcc.c-torture/compile/pr39845.c	(revision 0)
--- gcc/testsuite/gcc.c-torture/compile/pr39845.c	(revision 0)
***************
*** 0 ****
--- 1,43 ----
+ typedef union tree_node *tree;
+ enum tree_code { EXCESS_PRECISION_EXPR };
+ enum built_in_function { BUILT_IN_ACOS, BUILT_IN_FPCLASSIFY, BUILT_IN_ISFINITE };
+ struct tree_base {
+     __extension__ enum tree_code code : 16;
+     unsigned side_effects_flag : 1;
+ };
+ struct tree_exp {
+     tree     operands[1];
+ };
+ struct tree_function_decl {
+     __extension__ enum built_in_function function_code : 11;
+     unsigned static_ctor_flag : 1;
+ };
+ union tree_node {
+     struct tree_base base;
+     struct tree_function_decl function_decl;
+     struct tree_exp exp;
+ };
+ static tree
+ convert_arguments (tree fundecl)
+ {
+   tree val = (void *)0;
+   unsigned int parmnum;
+   unsigned char type_generic_remove_excess_precision = 0;
+   switch (((fundecl)->function_decl.function_code))
+     {
+       case BUILT_IN_ISFINITE:
+       case BUILT_IN_FPCLASSIFY:
+ 	  type_generic_remove_excess_precision = 1;
+     }
+   for (parmnum = 0;; ++parmnum)
+     if (((enum tree_code) (val)->base.code) == EXCESS_PRECISION_EXPR
+ 	&& !type_generic_remove_excess_precision)
+       val = ((val)->exp.operands[0]);
+   return val;
+ }
+ tree
+ build_function_call_vec (tree function)
+ {
+   return convert_arguments (function);
+ }
+ 

Re: [PATCH] Simplified switch conversion in simple cases

[Martin Jambor]

Hi,

On Tue, Apr 21, 2009 at 10:28:54PM +0200, Eric Botcazou wrote:
> > 2009-04-20  Martin Jambor  <mjambor@suse.cz>
> >
> > 	* tree-switch-conversion.c (build_constructors): Split a long line.
> > 	(constructor_contains_same_values_p): New function.
> > 	(build_one_array): Create assigns of constants if possible, do not call
> > 	mark_sym_for_renaming, call update_stmt.
> > 	(build_arrays): Call make_ssa_name (create_tmp_var ()) instead of
> > 	make_rename_temp.  Do not call mark_symbols_for_renaming, call
> > 	update_stmt.
> > 	(gen_def_assigns): Do not call mark_symbols_for_renaming or
> > 	find_new_referenced_vars, call update_stmt.
> > 	(gen_inbound_check): Use create_tmp_var and create ssa names manually
> > 	instead of calling make_rename_temp.  Do not call
> > 	find_new_referenced_vars or mark_symbols_for_renaming, call
> > 	update_stmt.
> 
> This breaks Ada on x86:

I suppose this was also  fixed by Richi's patch yesterday.  Please let
me  know if  it wans't.   Sorry  for the  annoyance, it  was a  stupid
mistake.

Martin


> 
> /home/eric/build/gcc/native32/./gcc/xgcc -B/home/eric/build/gcc/native32/./gcc/ -B/home/eric/install/gcc/i586-suse-linux/bin/ -B/home/eric/install/gcc/i586-suse-linux/lib/ -isystem /home/eric/install/gcc/i586-suse-linux/include -isystem /home/eric/install/gcc/i586-suse-linux/sys-include -c -g -O2   -W -Wall -gnatpg   
> g-socket.adb -o g-socket.o
> +===========================GNAT BUG DETECTED==============================+
> | 4.5.0 20090421 (experimental) [trunk revision 146532] (i586-suse-linux-gnu) 
> GCC error:|
> | in make_decl_rtl, at varasm.c:1304                                       |
> | Error detected around g-socket.adb:1746                                  |
> | Please submit a bug report; see http://gcc.gnu.org/bugs.html.            |
> 
> Reduced testcase attached, compile with:
> 
> gcc/xgcc -Bgcc -S g-socket.adb -Igcc/ada/rts -O
> 
> 
> (gdb) frame 2
> #2  0x08cfa656 in make_decl_rtl (decl=0xf7e050b8)
>     at /home/eric/svn/gcc/gcc/varasm.c:1300
> 1300      gcc_assert (TREE_CODE (decl) != VAR_DECL
> (gdb) p debug_tree(decl)
>  <var_decl 0xf7e050b8 csui.11
>     type <integer_type 0xf7d31bd0 unsigned int sizes-gimplified public visited 
> unsigned SI
>         size <integer_cst 0xf7d1c604 constant 32>
>         unit size <integer_cst 0xf7d1c3f0 constant 4>
>         align 32 symtab 0 alias set -1 canonical type 0xf7d31bd0 precision 32 
> min <integer_cst 0xf7d1cd90 0> max <integer_cst 0xf7d1cb98 4294967295>>
>     used unsigned ignored SI file g-socket.adb line 52 col 1 size <integer_cst 
> 0xf7d1c604 32> unit size <integer_cst 0xf7d1c3f0 4>
>     align 32 context <function_decl 0xf7ddd800 
> gnat__sockets__resolve_exception> abstract_origin <var_decl 0xf7df89b4 
> csui.11>>
> 
> -- 
> Eric Botcazou

> with System.OS_Constants;
> 
> package body GNAT.Sockets is
> 
>    package SOSC renames System.OS_Constants;
> 
>    function Resolve_Error
>      (Error_Value : Integer;
>       From_Errno  : Boolean) return Error_Type
>    is
>       use SOSC;
>    begin
>       if not From_Errno then
>          case Error_Value is
>             when SOSC.HOST_NOT_FOUND => return Unknown_Host;
>             when SOSC.TRY_AGAIN      => return Host_Name_Lookup_Failure;
>             when SOSC.NO_RECOVERY    => return Non_Recoverable_Error;
>             when SOSC.NO_DATA        => return Unknown_Server_Error;
>             when others              => return Cannot_Resolve_Error;
>          end case;
>       end if;
> 
>       case Error_Value is
>          when EACCES          => return Permission_Denied;
>          when EADDRINUSE      => return Address_Already_In_Use;
>          when EADDRNOTAVAIL   => return Cannot_Assign_Requested_Address;
>          when EAFNOSUPPORT    => return
>                                  Address_Family_Not_Supported_By_Protocol;
>          when EALREADY        => return Operation_Already_In_Progress;
>          when EBADF           => return Bad_File_Descriptor;
>          when ECONNABORTED    => return Software_Caused_Connection_Abort;
>          when ECONNREFUSED    => return Connection_Refused;
>          when ECONNRESET      => return Connection_Reset_By_Peer;
>          when EDESTADDRREQ    => return Destination_Address_Required;
>          when EFAULT          => return Bad_Address;
>          when EHOSTDOWN       => return Host_Is_Down;
>          when EHOSTUNREACH    => return No_Route_To_Host;
>          when others          => return Cannot_Resolve_Error;
>       end case;
>    end;
> 
>    function Resolve_Exception
>      (Occurrence : Exception_Occurrence) return Error_Type
>    is
>       Msg   : constant String := Exception_Message (Occurrence);
>       Val   : Integer;
>    begin
>       Val := Integer'Value (Msg (1 .. 4));
>       return Resolve_Error (Val, False);
>    end;
> 
> end GNAT.Sockets;

> with Ada.Exceptions; use Ada.Exceptions;
> 
> package GNAT.Sockets is
> 
>    type Error_Type is
>      (Success,
>       Permission_Denied,
>       Address_Already_In_Use,
>       Cannot_Assign_Requested_Address,
>       Address_Family_Not_Supported_By_Protocol,
>       Operation_Already_In_Progress,
>       Bad_File_Descriptor,
>       Software_Caused_Connection_Abort,
>       Connection_Refused,
>       Connection_Reset_By_Peer,
>       Destination_Address_Required,
>       Bad_Address,
>       Host_Is_Down,
>       No_Route_To_Host,
>       Operation_Now_In_Progress,
>       Interrupted_System_Call,
>       Invalid_Argument,
>       Input_Output_Error,
>       Transport_Endpoint_Already_Connected,
>       Too_Many_Symbolic_Links,
>       Too_Many_Open_Files,
>       Message_Too_Long,
>       File_Name_Too_Long,
>       Network_Is_Down,
>       Network_Dropped_Connection_Because_Of_Reset,
>       Network_Is_Unreachable,
>       No_Buffer_Space_Available,
>       Protocol_Not_Available,
>       Transport_Endpoint_Not_Connected,
>       Socket_Operation_On_Non_Socket,
>       Operation_Not_Supported,
>       Protocol_Family_Not_Supported,
>       Protocol_Not_Supported,
>       Protocol_Wrong_Type_For_Socket,
>       Cannot_Send_After_Transport_Endpoint_Shutdown,
>       Socket_Type_Not_Supported,
>       Connection_Timed_Out,
>       Too_Many_References,
>       Resource_Temporarily_Unavailable,
>       Broken_Pipe,
>       Unknown_Host,
>       Host_Name_Lookup_Failure,
>       Non_Recoverable_Error,
>       Unknown_Server_Error,
>       Cannot_Resolve_Error);
> 
>    function Resolve_Exception
>      (Occurrence : Ada.Exceptions.Exception_Occurrence) return Error_Type;
> 
> end GNAT.Sockets;

Re: [PATCH] Simplified switch conversion in simple cases

[Eric Botcazou]

> I suppose this was also  fixed by Richi's patch yesterday.

Yes, it was.

> Sorry  for the  annoyance, it  was a  stupid  mistake.

No problem.

-- 
Eric Botcazou