diff mbox

[5/6] Port various places from union access to subclass access.

Message ID 1383236801-13234-6-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm Oct. 31, 2013, 4:26 p.m. UTC 
* gimple-streamer-in.c (input_gimple_stmt): Port from union
	access to use of as_a.
	* gimple.c (gimple_build_asm_1): Likewise.
	(gimple_build_try): Likewise.  Also, return a specific subclass
	rather than just gimple.
	(gimple_build_resx): Port from union access to use of as_a.
	(gimple_build_eh_dispatch): Likewise.
	(gimple_build_omp_for): Likewise.  Also, convert allocation of iter
	now that gengtype no longer provides a typed allocator function.
	(gimple_copy): Likewise.
	* gimple.h (gimple_build_try): Return a specific subclass rather
	than just gimple.
	* gimplify.c (gimplify_cleanup_point_expr): Replace union access
	with subclass access by making use of new return type of
	gimple_build_try.
	* tree-phinodes.c: (allocate_phi_node): Return a
	"gimple_statement_phi *" rather than just a gimple.
	(resize_phi_node): Likewise.
	(make_phi_node): Replace union access with subclass access by
	making use of new return type of allocate_phi_node.
	(reserve_phi_args_for_new_edge): Replace union access with as_a.
	(remove_phi_arg_num): Accept a "gimple_statement_phi *" rather
	than just a gimple.
	(remove_phi_args): Update for change to remove_phi_arg_num.
---
 gcc/gimple-streamer-in.c | 11 ++++-----
 gcc/gimple.c             | 58 ++++++++++++++++++++++++++++++------------------
 gcc/gimple.h             |  3 ++-
 gcc/gimplify.c           |  4 ++--
 gcc/tree-phinodes.c      | 37 ++++++++++++++++--------------
 5 files changed, 66 insertions(+), 47 deletions(-)

Comments

Jeff Law Nov. 14, 2013, 7:34 a.m. UTC | #1 
On 10/31/13 10:26, David Malcolm wrote:
>
> diff --git a/gcc/gimple-streamer-in.c b/gcc/gimple-streamer-in.c
> index 4f31b83..2555dbe 100644
> --- a/gcc/gimple-streamer-in.c
> +++ b/gcc/gimple-streamer-in.c
> @@ -129,13 +129,14 @@ input_gimple_stmt (struct lto_input_block *ib, struct data_in *data_in,
>       case GIMPLE_ASM:
>         {
>   	/* FIXME lto.  Move most of this into a new gimple_asm_set_string().  */
> +	gimple_statement_asm *asm_stmt = as_a <gimple_statement_asm> (stmt);
>   	tree str;
> -	stmt->gimple_asm.ni = streamer_read_uhwi (ib);
> -	stmt->gimple_asm.no = streamer_read_uhwi (ib);
> -	stmt->gimple_asm.nc = streamer_read_uhwi (ib);
> -	stmt->gimple_asm.nl = streamer_read_uhwi (ib);
> +	asm_stmt->ni = streamer_read_uhwi (ib);
> +	asm_stmt->no = streamer_read_uhwi (ib);
> +	asm_stmt->nc = streamer_read_uhwi (ib);
> +	asm_stmt->nl = streamer_read_uhwi (ib);
>   	str = streamer_read_string_cst (data_in, ib);
> -	stmt->gimple_asm.string = TREE_STRING_POINTER (str);
> +	asm_stmt->string = TREE_STRING_POINTER (str);
The in/out streaming seems like another natural fit for virtual 
functions in the long term.


>         }
>         /* Fallthru  */
>
> diff --git a/gcc/gimple.c b/gcc/gimple.c
> index 9b1337a..e9ef8e0 100644
> --- a/gcc/gimple.c
> +++ b/gcc/gimple.c
> @@ -641,21 +641,22 @@ static inline gimple
>   gimple_build_asm_1 (const char *string, unsigned ninputs, unsigned noutputs,
>                       unsigned nclobbers, unsigned nlabels)
>   {
> -  gimple p;
> +  gimple_statement_asm *p;
>     int size = strlen (string);
>
>     /* ASMs with labels cannot have outputs.  This should have been
>        enforced by the front end.  */
>     gcc_assert (nlabels == 0 || noutputs == 0);
>
> -  p = gimple_build_with_ops (GIMPLE_ASM, ERROR_MARK,
> -			     ninputs + noutputs + nclobbers + nlabels);
> +  p = as_a <gimple_statement_asm> (
> +        gimple_build_with_ops (GIMPLE_ASM, ERROR_MARK,
> +			       ninputs + noutputs + nclobbers + nlabels));
>
> -  p->gimple_asm.ni = ninputs;
> -  p->gimple_asm.no = noutputs;
> -  p->gimple_asm.nc = nclobbers;
> -  p->gimple_asm.nl = nlabels;
> -  p->gimple_asm.string = ggc_alloc_string (string, size);
> +  p->ni = ninputs;
> +  p->no = noutputs;
> +  p->nc = nclobbers;
> +  p->nl = nlabels;
> +  p->string = ggc_alloc_string (string, size);
As noted in a prior message, having build methods would eliminate this 
downcasting.  Not necessary for this patch, just wanted to point it out 
to anyone reading.  I won't point it out everywhere ;-)


So given the prior disussions around as_a, I'm not going to object to 
these as_a instances.  I see them as warts/markers that we have further 
work to do in terms of fleshing out the class and possibly refactoring 
code.

Conditionally OK.  Conditional on the other related patches going in and 
keeping it updated with Andrew's churn.  If/when the set goes in, post 
the final version you actually checkin -- no re-review is needed for 
this hunk so long as any changes are the obvious fixing of fallout from 
Andrew's work.

Jeff
David Malcolm Nov. 18, 2013, 9:07 p.m. UTC | #2 
On Thu, 2013-11-14 at 00:34 -0700, Jeff Law wrote:
> On 10/31/13 10:26, David Malcolm wrote:

[...]

> > diff --git a/gcc/gimple.c b/gcc/gimple.c
> > index 9b1337a..e9ef8e0 100644
> > --- a/gcc/gimple.c
> > +++ b/gcc/gimple.c
> > @@ -641,21 +641,22 @@ static inline gimple
> >   gimple_build_asm_1 (const char *string, unsigned ninputs, unsigned noutputs,
> >                       unsigned nclobbers, unsigned nlabels)
> >   {
> > -  gimple p;
> > +  gimple_statement_asm *p;
> >     int size = strlen (string);
> >
> >     /* ASMs with labels cannot have outputs.  This should have been
> >        enforced by the front end.  */
> >     gcc_assert (nlabels == 0 || noutputs == 0);
> >
> > -  p = gimple_build_with_ops (GIMPLE_ASM, ERROR_MARK,
> > -			     ninputs + noutputs + nclobbers + nlabels);
> > +  p = as_a <gimple_statement_asm> (
> > +        gimple_build_with_ops (GIMPLE_ASM, ERROR_MARK,
> > +			       ninputs + noutputs + nclobbers + nlabels));
> >
> > -  p->gimple_asm.ni = ninputs;
> > -  p->gimple_asm.no = noutputs;
> > -  p->gimple_asm.nc = nclobbers;
> > -  p->gimple_asm.nl = nlabels;
> > -  p->gimple_asm.string = ggc_alloc_string (string, size);
> > +  p->ni = ninputs;
> > +  p->no = noutputs;
> > +  p->nc = nclobbers;
> > +  p->nl = nlabels;
> > +  p->string = ggc_alloc_string (string, size);
> As noted in a prior message, having build methods would eliminate this 
> downcasting.  Not necessary for this patch, just wanted to point it out 
> to anyone reading.  I won't point it out everywhere ;-)

This one (gimple_build_asm_1) is itself a build method.  The checking on
the downcast could be argued to be redundant, since presumably we trust
gimple_build_with_ops to give us a stmt with the code we asked for, but
I don't think it hurts.

> So given the prior disussions around as_a, I'm not going to object to 
> these as_a instances.  I see them as warts/markers that we have further 
> work to do in terms of fleshing out the class and possibly refactoring 
> code.
> 
> Conditionally OK.  Conditional on the other related patches going in and 
> keeping it updated with Andrew's churn.  If/when the set goes in, post 
> the final version you actually checkin -- no re-review is needed for 
> this hunk so long as any changes are the obvious fixing of fallout from 
> Andrew's work.

Thanks.
diff mbox

Patch

diff --git a/gcc/gimple-streamer-in.c b/gcc/gimple-streamer-in.c
index 4f31b83..2555dbe 100644
--- a/gcc/gimple-streamer-in.c
+++ b/gcc/gimple-streamer-in.c
@@ -129,13 +129,14 @@  input_gimple_stmt (struct lto_input_block *ib, struct data_in *data_in,
     case GIMPLE_ASM:
       {
 	/* FIXME lto.  Move most of this into a new gimple_asm_set_string().  */
+	gimple_statement_asm *asm_stmt = as_a <gimple_statement_asm> (stmt);
 	tree str;
-	stmt->gimple_asm.ni = streamer_read_uhwi (ib);
-	stmt->gimple_asm.no = streamer_read_uhwi (ib);
-	stmt->gimple_asm.nc = streamer_read_uhwi (ib);
-	stmt->gimple_asm.nl = streamer_read_uhwi (ib);
+	asm_stmt->ni = streamer_read_uhwi (ib);
+	asm_stmt->no = streamer_read_uhwi (ib);
+	asm_stmt->nc = streamer_read_uhwi (ib);
+	asm_stmt->nl = streamer_read_uhwi (ib);
 	str = streamer_read_string_cst (data_in, ib);
-	stmt->gimple_asm.string = TREE_STRING_POINTER (str);
+	asm_stmt->string = TREE_STRING_POINTER (str);
       }
       /* Fallthru  */
 
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 9b1337a..e9ef8e0 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -641,21 +641,22 @@  static inline gimple
 gimple_build_asm_1 (const char *string, unsigned ninputs, unsigned noutputs,
                     unsigned nclobbers, unsigned nlabels)
 {
-  gimple p;
+  gimple_statement_asm *p;
   int size = strlen (string);
 
   /* ASMs with labels cannot have outputs.  This should have been
      enforced by the front end.  */
   gcc_assert (nlabels == 0 || noutputs == 0);
 
-  p = gimple_build_with_ops (GIMPLE_ASM, ERROR_MARK,
-			     ninputs + noutputs + nclobbers + nlabels);
+  p = as_a <gimple_statement_asm> (
+        gimple_build_with_ops (GIMPLE_ASM, ERROR_MARK,
+			       ninputs + noutputs + nclobbers + nlabels));
 
-  p->gimple_asm.ni = ninputs;
-  p->gimple_asm.no = noutputs;
-  p->gimple_asm.nc = nclobbers;
-  p->gimple_asm.nl = nlabels;
-  p->gimple_asm.string = ggc_alloc_string (string, size);
+  p->ni = ninputs;
+  p->no = noutputs;
+  p->nc = nclobbers;
+  p->nl = nlabels;
+  p->string = ggc_alloc_string (string, size);
 
   if (GATHER_STATISTICS)
     gimple_alloc_sizes[(int) gimple_alloc_kind (GIMPLE_ASM)] += size;
@@ -767,14 +768,14 @@  gimple_build_eh_else (gimple_seq n_body, gimple_seq e_body)
    KIND is either GIMPLE_TRY_CATCH or GIMPLE_TRY_FINALLY depending on
    whether this is a try/catch or a try/finally respectively.  */
 
-gimple
+gimple_statement_try *
 gimple_build_try (gimple_seq eval, gimple_seq cleanup,
     		  enum gimple_try_flags kind)
 {
-  gimple p;
+  gimple_statement_try *p;
 
   gcc_assert (kind == GIMPLE_TRY_CATCH || kind == GIMPLE_TRY_FINALLY);
-  p = gimple_alloc (GIMPLE_TRY, 0);
+  p = as_a <gimple_statement_try> (gimple_alloc (GIMPLE_TRY, 0));
   gimple_set_subcode (p, kind);
   if (eval)
     gimple_try_set_eval (p, eval);
@@ -804,8 +805,10 @@  gimple_build_wce (gimple_seq cleanup)
 gimple
 gimple_build_resx (int region)
 {
-  gimple p = gimple_build_with_ops (GIMPLE_RESX, ERROR_MARK, 0);
-  p->gimple_eh_ctrl.region = region;
+  gimple_statement_eh_ctrl *p =
+    as_a <gimple_statement_eh_ctrl> (
+      gimple_build_with_ops (GIMPLE_RESX, ERROR_MARK, 0));
+  p->region = region;
   return p;
 }
 
@@ -852,8 +855,10 @@  gimple_build_switch (tree index, tree default_label, vec<tree> args)
 gimple
 gimple_build_eh_dispatch (int region)
 {
-  gimple p = gimple_build_with_ops (GIMPLE_EH_DISPATCH, ERROR_MARK, 0);
-  p->gimple_eh_ctrl.region = region;
+  gimple_statement_eh_ctrl *p =
+    as_a <gimple_statement_eh_ctrl> (
+      gimple_build_with_ops (GIMPLE_EH_DISPATCH, ERROR_MARK, 0));
+  p->region = region;
   return p;
 }
 
@@ -927,14 +932,17 @@  gimple
 gimple_build_omp_for (gimple_seq body, int kind, tree clauses, size_t collapse,
 		      gimple_seq pre_body)
 {
-  gimple p = gimple_alloc (GIMPLE_OMP_FOR, 0);
+  gimple_statement_omp_for *p =
+    as_a <gimple_statement_omp_for> (gimple_alloc (GIMPLE_OMP_FOR, 0));
   if (body)
     gimple_omp_set_body (p, body);
   gimple_omp_for_set_clauses (p, clauses);
   gimple_omp_for_set_kind (p, kind);
-  p->gimple_omp_for.collapse = collapse;
-  p->gimple_omp_for.iter
-      = ggc_alloc_cleared_vec_gimple_omp_for_iter (collapse);
+  p->collapse = collapse;
+  p->iter =  static_cast <struct gimple_omp_for_iter *> (
+   ggc_internal_cleared_vec_alloc_stat (sizeof (*p->iter),
+					collapse MEM_STAT_INFO));
+
   if (pre_body)
     gimple_omp_for_set_pre_body (p, pre_body);
 
@@ -2311,9 +2319,15 @@  gimple_copy (gimple stmt)
 	  gimple_omp_for_set_pre_body (copy, new_seq);
 	  t = unshare_expr (gimple_omp_for_clauses (stmt));
 	  gimple_omp_for_set_clauses (copy, t);
-	  copy->gimple_omp_for.iter
-	    = ggc_alloc_vec_gimple_omp_for_iter
-	    (gimple_omp_for_collapse (stmt));
+	  {
+	    gimple_statement_omp_for *omp_for_copy =
+	      as_a <gimple_statement_omp_for> (copy);
+	    omp_for_copy->iter =
+	      static_cast <struct gimple_omp_for_iter *> (
+		  ggc_internal_vec_alloc_stat (sizeof (struct gimple_omp_for_iter),
+					       gimple_omp_for_collapse (stmt)
+					       MEM_STAT_INFO));
+          }
 	  for (i = 0; i < gimple_omp_for_collapse (stmt); i++)
 	    {
 	      gimple_omp_for_set_cond (copy, i,
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 710ce04..35bfa06 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -1074,7 +1074,8 @@  gimple gimple_build_catch (tree, gimple_seq);
 gimple gimple_build_eh_filter (tree, gimple_seq);
 gimple gimple_build_eh_must_not_throw (tree);
 gimple gimple_build_eh_else (gimple_seq, gimple_seq);
-gimple gimple_build_try (gimple_seq, gimple_seq, enum gimple_try_flags);
+gimple_statement_try *gimple_build_try (gimple_seq, gimple_seq,
+					enum gimple_try_flags);
 gimple gimple_build_wce (gimple_seq);
 gimple gimple_build_resx (int);
 gimple gimple_build_eh_dispatch (int);
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 1f18466..5869441 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -5456,7 +5456,7 @@  gimplify_cleanup_point_expr (tree *expr_p, gimple_seq *pre_p)
 	    }
 	  else
 	    {
-	      gimple gtry;
+	      gimple_statement_try *gtry;
 	      gimple_seq seq;
 	      enum gimple_try_flags kind;
 
@@ -5470,7 +5470,7 @@  gimplify_cleanup_point_expr (tree *expr_p, gimple_seq *pre_p)
               /* Do not use gsi_replace here, as it may scan operands.
                  We want to do a simple structural modification only.  */
 	      gsi_set_stmt (&iter, gtry);
-	      iter = gsi_start (gtry->gimple_try.eval);
+	      iter = gsi_start (gtry->eval);
 	    }
 	}
       else
diff --git a/gcc/tree-phinodes.c b/gcc/tree-phinodes.c
index 65c636c..6e425fe 100644
--- a/gcc/tree-phinodes.c
+++ b/gcc/tree-phinodes.c
@@ -92,10 +92,10 @@  phinodes_print_statistics (void)
    happens to contain a PHI node with LEN arguments or more, return
    that one.  */
 
-static inline gimple
+static inline gimple_statement_phi *
 allocate_phi_node (size_t len)
 {
-  gimple phi;
+  gimple_statement_phi *phi;
   size_t bucket = NUM_BUCKETS - 2;
   size_t size = sizeof (struct gimple_statement_phi)
 	        + (len - 1) * sizeof (struct phi_arg_d);
@@ -110,7 +110,7 @@  allocate_phi_node (size_t len)
       && gimple_phi_capacity ((*free_phinodes[bucket])[0]) >= len)
     {
       free_phinode_count--;
-      phi = free_phinodes[bucket]->pop ();
+      phi = as_a <gimple_statement_phi> (free_phinodes[bucket]->pop ());
       if (free_phinodes[bucket]->is_empty ())
 	vec_free (free_phinodes[bucket]);
       if (GATHER_STATISTICS)
@@ -118,7 +118,8 @@  allocate_phi_node (size_t len)
     }
   else
     {
-      phi = ggc_alloc_gimple_statement_d (size);
+      phi = static_cast <gimple_statement_phi *> (
+	ggc_internal_alloc_stat (size MEM_STAT_INFO));
       if (GATHER_STATISTICS)
 	{
 	  enum gimple_alloc_kind kind = gimple_alloc_kind (GIMPLE_PHI);
@@ -170,7 +171,7 @@  ideal_phi_node_len (int len)
 static gimple
 make_phi_node (tree var, int len)
 {
-  gimple phi;
+  gimple_statement_phi *phi;
   int capacity, i;
 
   capacity = ideal_phi_node_len (len);
@@ -185,8 +186,8 @@  make_phi_node (tree var, int len)
 		   + sizeof (struct phi_arg_d) * len));
   phi->code = GIMPLE_PHI;
   gimple_init_singleton (phi);
-  phi->gimple_phi.nargs = len;
-  phi->gimple_phi.capacity = capacity;
+  phi->nargs = len;
+  phi->capacity = capacity;
   if (!var)
     ;
   else if (TREE_CODE (var) == SSA_NAME)
@@ -235,11 +236,11 @@  release_phi_node (gimple phi)
 /* Resize an existing PHI node.  The only way is up.  Return the
    possibly relocated phi.  */
 
-static gimple
-resize_phi_node (gimple phi, size_t len)
+static gimple_statement_phi *
+resize_phi_node (gimple_statement_phi *phi, size_t len)
 {
   size_t old_size, i;
-  gimple new_phi;
+  gimple_statement_phi *new_phi;
 
   gcc_assert (len > gimple_phi_capacity (phi));
 
@@ -262,7 +263,7 @@  resize_phi_node (gimple phi, size_t len)
       relink_imm_use_stmt (imm, old_imm, new_phi);
     }
 
-  new_phi->gimple_phi.capacity = len;
+  new_phi->capacity = len;
 
   for (i = gimple_phi_num_args (new_phi); i < len; i++)
     {
@@ -290,11 +291,12 @@  reserve_phi_args_for_new_edge (basic_block bb)
 
   for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
     {
-      gimple stmt = gsi_stmt (gsi);
+      gimple_statement_phi *stmt =
+	as_a <gimple_statement_phi> (gsi_stmt (gsi));
 
       if (len > gimple_phi_capacity (stmt))
 	{
-	  gimple new_phi = resize_phi_node (stmt, cap);
+	  gimple_statement_phi *new_phi = resize_phi_node (stmt, cap);
 
 	  /* The result of the PHI is defined by this PHI node.  */
 	  SSA_NAME_DEF_STMT (gimple_phi_result (new_phi)) = new_phi;
@@ -314,7 +316,7 @@  reserve_phi_args_for_new_edge (basic_block bb)
       SET_PHI_ARG_DEF (stmt, len - 1, NULL_TREE);
       gimple_phi_arg_set_location (stmt, len - 1, UNKNOWN_LOCATION);
 
-      stmt->gimple_phi.nargs++;
+      stmt->nargs++;
     }
 }
 
@@ -390,7 +392,7 @@  add_phi_arg (gimple phi, tree def, edge e, source_location locus)
    is consistent with how we remove an edge from the edge vector.  */
 
 static void
-remove_phi_arg_num (gimple phi, int i)
+remove_phi_arg_num (gimple_statement_phi *phi, int i)
 {
   int num_elem = gimple_phi_num_args (phi);
 
@@ -417,7 +419,7 @@  remove_phi_arg_num (gimple phi, int i)
   /* Shrink the vector and return.  Note that we do not have to clear
      PHI_ARG_DEF because the garbage collector will not look at those
      elements beyond the first PHI_NUM_ARGS elements of the array.  */
-  phi->gimple_phi.nargs--;
+  phi->nargs--;
 }
 
 
@@ -429,7 +431,8 @@  remove_phi_args (edge e)
   gimple_stmt_iterator gsi;
 
   for (gsi = gsi_start_phis (e->dest); !gsi_end_p (gsi); gsi_next (&gsi))
-    remove_phi_arg_num (gsi_stmt (gsi), e->dest_idx);
+    remove_phi_arg_num (as_a <gimple_statement_phi> (gsi_stmt (gsi)),
+			e->dest_idx);
 }