Re: [PATCH] Simplified switch conversion in simple cases

[Richard Guenther <rguenther at suse dot de>]

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