[PATCH] gengtype: remove "tree_exp" special attribute

[PATCH] gengtype: remove "tree_exp" special attribute

[Patrick Palka]

The function comment for adjust_field_tree_exp says this special case
is for handling trees whose operands may contain pointers to RTL instead
of to trees.  But ever since r0-59671, which fixed/removed the last two
tree codes for which this was possible (GOTO_SUBROUTINE_EXPR and
WITH_CLEANUP_EXPR), this special attribute is largely a no-op.

This patch removes it and instead just annotates struct tree_exp
with the "length" attribute directly.  Not sure it makes a difference,
but I use %h instead of %0 in the "length" attribute to be consistent
with other structures' "length" attributes within tree-core.h.

This changes the code generated for TS_EXP handling in gt-cp-tree.h from:

  case TS_EXP:
    gt_ggc_m_9tree_node ((*x).generic.exp.typed.type);
    switch ((int) (TREE_CODE ((tree) &(*x))))
      {
      default:
	{
	  size_t i3;
	  size_t l3 = (size_t)(TREE_OPERAND_LENGTH ((tree) &(*x)));
	  for (i3 = 0; i3 != l3; i3++) {
	    gt_ggc_m_9tree_node ((*x).generic.exp.operands[i3]);
	  }
	}
	break;
      }
    break;

to:

  case TS_EXP:
    {
      size_t l3 = (size_t)(TREE_OPERAND_LENGTH ((tree)&((*x).generic.exp)));
      gt_ggc_m_9tree_node ((*x).generic.exp.typed.type);
      {
	size_t i3;
	for (i3 = 0; i3 != l3; i3++) {
	  gt_ggc_m_9tree_node ((*x).generic.exp.operands[i3]);
	}
      }
    }

which seems equivalent and simpler.

Boostrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk?

gcc/ChangeLog:

	* gengtype.c (adjust_field_tree_exp): Remove.
	(adjust_field_type): Don't handle the "tree_exp" special attribute.
	* tree-core.h (struct tree_exp): Replace special and desc
	attributes with length.
---
 gcc/gengtype.c  | 35 +----------------------------------
 gcc/tree-core.h |  4 +---
 2 files changed, 2 insertions(+), 37 deletions(-)

diff --git a/gcc/gengtype.c b/gcc/gengtype.c
index db218a7bce7..2dc443175fa 100644
--- a/gcc/gengtype.c
+++ b/gcc/gengtype.c
@@ -511,7 +511,6 @@ pair_p typedefs = NULL;
 type_p structures = NULL;
 pair_p variables = NULL;
 
-static type_p adjust_field_tree_exp (type_p t, options_p opt);
 static type_p adjust_field_rtx_def (type_p t, options_p opt);
 
 /* Define S as a typedef to T at POS.  */
@@ -1384,36 +1383,6 @@ adjust_field_rtx_def (type_p t, options_p ARG_UNUSED (opt))
 			nodot, NULL);
 }
 
-/* Handle `special("tree_exp")'.  This is a special case for
-   field `operands' of struct tree_exp, which although it claims to contain
-   pointers to trees, actually sometimes contains pointers to RTL too.
-   Passed T, the old type of the field, and OPT its options.  Returns
-   a new type for the field.  */
-
-static type_p
-adjust_field_tree_exp (type_p t, options_p opt ATTRIBUTE_UNUSED)
-{
-  pair_p flds;
-  options_p nodot;
-
-  if (t->kind != TYPE_ARRAY)
-    {
-      error_at_line (&lexer_line,
-		     "special `tree_exp' must be applied to an array");
-      return &string_type;
-    }
-
-  nodot = create_string_option (NULL, "dot", "");
-
-  flds = create_field (NULL, t, "");
-  flds->opt = create_string_option (nodot, "length",
-				    "TREE_OPERAND_LENGTH ((tree) &%0)");
-  flds->opt = create_string_option (flds->opt, "default", "");
-
-  return new_structure ("tree_exp_subunion", TYPE_UNION, &lexer_line, flds,
-			nodot, NULL);
-}
-
 /* Perform any special processing on a type T, about to become the type
    of a field.  Return the appropriate type for the field.
    At present:
@@ -1447,9 +1416,7 @@ adjust_field_type (type_p t, options_p opt)
 	     && opt->kind == OPTION_STRING)
       {
 	const char *special_name = opt->info.string;
-	if (strcmp (special_name, "tree_exp") == 0)
-	  t = adjust_field_tree_exp (t, opt);
-	else if (strcmp (special_name, "rtx_def") == 0)
+	if (strcmp (special_name, "rtx_def") == 0)
 	  t = adjust_field_rtx_def (t, opt);
 	else
 	  error_at_line (&lexer_line, "unknown special `%s'", special_name);
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 91ae5237d7e..c48018999ce 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1547,9 +1547,7 @@ enum omp_clause_linear_kind
 struct GTY(()) tree_exp {
   struct tree_typed typed;
   location_t locus;
-  tree GTY ((special ("tree_exp"),
-	     desc ("TREE_CODE ((tree) &%0)")))
-    operands[1];
+  tree GTY ((length ("TREE_OPERAND_LENGTH ((tree)&%h)"))) operands[1];
 };
 
 /* Immediate use linking structure.  This structure is used for maintaining
-- 
2.34.1.75.gabe6bb3905

Re: [PATCH] gengtype: remove "tree_exp" special attribute

[Jeff Law]

On 12/10/2021 8:41 AM, Patrick Palka via Gcc-patches wrote:
> The function comment for adjust_field_tree_exp says this special case
> is for handling trees whose operands may contain pointers to RTL instead
> of to trees.  But ever since r0-59671, which fixed/removed the last two
> tree codes for which this was possible (GOTO_SUBROUTINE_EXPR and
> WITH_CLEANUP_EXPR), this special attribute is largely a no-op.
>
> This patch removes it and instead just annotates struct tree_exp
> with the "length" attribute directly.  Not sure it makes a difference,
> but I use %h instead of %0 in the "length" attribute to be consistent
> with other structures' "length" attributes within tree-core.h.
>
> This changes the code generated for TS_EXP handling in gt-cp-tree.h from:
>
>    case TS_EXP:
>      gt_ggc_m_9tree_node ((*x).generic.exp.typed.type);
>      switch ((int) (TREE_CODE ((tree) &(*x))))
>        {
>        default:
> 	{
> 	  size_t i3;
> 	  size_t l3 = (size_t)(TREE_OPERAND_LENGTH ((tree) &(*x)));
> 	  for (i3 = 0; i3 != l3; i3++) {
> 	    gt_ggc_m_9tree_node ((*x).generic.exp.operands[i3]);
> 	  }
> 	}
> 	break;
>        }
>      break;
>
> to:
>
>    case TS_EXP:
>      {
>        size_t l3 = (size_t)(TREE_OPERAND_LENGTH ((tree)&((*x).generic.exp)));
>        gt_ggc_m_9tree_node ((*x).generic.exp.typed.type);
>        {
> 	size_t i3;
> 	for (i3 = 0; i3 != l3; i3++) {
> 	  gt_ggc_m_9tree_node ((*x).generic.exp.operands[i3]);
> 	}
>        }
>      }
>
> which seems equivalent and simpler.
>
> Boostrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk?
>
> gcc/ChangeLog:
>
> 	* gengtype.c (adjust_field_tree_exp): Remove.
> 	(adjust_field_type): Don't handle the "tree_exp" special attribute.
> 	* tree-core.h (struct tree_exp): Replace special and desc
> 	attributes with length.
Should this wait until stage1 reopens?  It doesn't seem like a bugfix.

jeff

Re: [PATCH] gengtype: remove "tree_exp" special attribute

[Patrick Palka]

On Fri, Dec 10, 2021 at 3:57 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 12/10/2021 8:41 AM, Patrick Palka via Gcc-patches wrote:
> > The function comment for adjust_field_tree_exp says this special case
> > is for handling trees whose operands may contain pointers to RTL instead
> > of to trees.  But ever since r0-59671, which fixed/removed the last two
> > tree codes for which this was possible (GOTO_SUBROUTINE_EXPR and
> > WITH_CLEANUP_EXPR), this special attribute is largely a no-op.
> >
> > This patch removes it and instead just annotates struct tree_exp
> > with the "length" attribute directly.  Not sure it makes a difference,
> > but I use %h instead of %0 in the "length" attribute to be consistent
> > with other structures' "length" attributes within tree-core.h.
> >
> > This changes the code generated for TS_EXP handling in gt-cp-tree.h from:
> >
> >    case TS_EXP:
> >      gt_ggc_m_9tree_node ((*x).generic.exp.typed.type);
> >      switch ((int) (TREE_CODE ((tree) &(*x))))
> >        {
> >        default:
> >       {
> >         size_t i3;
> >         size_t l3 = (size_t)(TREE_OPERAND_LENGTH ((tree) &(*x)));
> >         for (i3 = 0; i3 != l3; i3++) {
> >           gt_ggc_m_9tree_node ((*x).generic.exp.operands[i3]);
> >         }
> >       }
> >       break;
> >        }
> >      break;
> >
> > to:
> >
> >    case TS_EXP:
> >      {
> >        size_t l3 = (size_t)(TREE_OPERAND_LENGTH ((tree)&((*x).generic.exp)));
> >        gt_ggc_m_9tree_node ((*x).generic.exp.typed.type);
> >        {
> >       size_t i3;
> >       for (i3 = 0; i3 != l3; i3++) {
> >         gt_ggc_m_9tree_node ((*x).generic.exp.operands[i3]);
> >       }
> >        }
> >      }
> >
> > which seems equivalent and simpler.
> >
> > Boostrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk?
> >
> > gcc/ChangeLog:
> >
> >       * gengtype.c (adjust_field_tree_exp): Remove.
> >       (adjust_field_type): Don't handle the "tree_exp" special attribute.
> >       * tree-core.h (struct tree_exp): Replace special and desc
> >       attributes with length.
> Should this wait until stage1 reopens?  It doesn't seem like a bugfix.

Yes sorry, I should have clarified that this isn't a bugfix, only a
minor cleanup.  I'll try to remember to ping this when stage1 reopens.

>
> jeff
>

Re: [PATCH] gengtype: remove "tree_exp" special attribute

[Jason Merrill]

On 12/10/21 10:41, Patrick Palka wrote:
> The function comment for adjust_field_tree_exp says this special case
> is for handling trees whose operands may contain pointers to RTL instead
> of to trees.  But ever since r0-59671, which fixed/removed the last two
> tree codes for which this was possible (GOTO_SUBROUTINE_EXPR and
> WITH_CLEANUP_EXPR), this special attribute is largely a no-op.
> 
> This patch removes it and instead just annotates struct tree_exp
> with the "length" attribute directly.  Not sure it makes a difference,
> but I use %h instead of %0 in the "length" attribute to be consistent
> with other structures' "length" attributes within tree-core.h.
> 
> This changes the code generated for TS_EXP handling in gt-cp-tree.h from:
> 
>    case TS_EXP:
>      gt_ggc_m_9tree_node ((*x).generic.exp.typed.type);
>      switch ((int) (TREE_CODE ((tree) &(*x))))
>        {
>        default:
> 	{
> 	  size_t i3;
> 	  size_t l3 = (size_t)(TREE_OPERAND_LENGTH ((tree) &(*x)));
> 	  for (i3 = 0; i3 != l3; i3++) {
> 	    gt_ggc_m_9tree_node ((*x).generic.exp.operands[i3]);
> 	  }
> 	}
> 	break;
>        }
>      break;
> 
> to:
> 
>    case TS_EXP:
>      {
>        size_t l3 = (size_t)(TREE_OPERAND_LENGTH ((tree)&((*x).generic.exp)));
>        gt_ggc_m_9tree_node ((*x).generic.exp.typed.type);
>        {
> 	size_t i3;
> 	for (i3 = 0; i3 != l3; i3++) {
> 	  gt_ggc_m_9tree_node ((*x).generic.exp.operands[i3]);
> 	}
>        }
>      }
> 
> which seems equivalent and simpler.
> 
> Boostrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk?

OK.

> gcc/ChangeLog:
> 
> 	* gengtype.c (adjust_field_tree_exp): Remove.
> 	(adjust_field_type): Don't handle the "tree_exp" special attribute.
> 	* tree-core.h (struct tree_exp): Replace special and desc
> 	attributes with length.
> ---
>   gcc/gengtype.c  | 35 +----------------------------------
>   gcc/tree-core.h |  4 +---
>   2 files changed, 2 insertions(+), 37 deletions(-)
> 
> diff --git a/gcc/gengtype.c b/gcc/gengtype.c
> index db218a7bce7..2dc443175fa 100644
> --- a/gcc/gengtype.c
> +++ b/gcc/gengtype.c
> @@ -511,7 +511,6 @@ pair_p typedefs = NULL;
>   type_p structures = NULL;
>   pair_p variables = NULL;
>   
> -static type_p adjust_field_tree_exp (type_p t, options_p opt);
>   static type_p adjust_field_rtx_def (type_p t, options_p opt);
>   
>   /* Define S as a typedef to T at POS.  */
> @@ -1384,36 +1383,6 @@ adjust_field_rtx_def (type_p t, options_p ARG_UNUSED (opt))
>   			nodot, NULL);
>   }
>   
> -/* Handle `special("tree_exp")'.  This is a special case for
> -   field `operands' of struct tree_exp, which although it claims to contain
> -   pointers to trees, actually sometimes contains pointers to RTL too.
> -   Passed T, the old type of the field, and OPT its options.  Returns
> -   a new type for the field.  */
> -
> -static type_p
> -adjust_field_tree_exp (type_p t, options_p opt ATTRIBUTE_UNUSED)
> -{
> -  pair_p flds;
> -  options_p nodot;
> -
> -  if (t->kind != TYPE_ARRAY)
> -    {
> -      error_at_line (&lexer_line,
> -		     "special `tree_exp' must be applied to an array");
> -      return &string_type;
> -    }
> -
> -  nodot = create_string_option (NULL, "dot", "");
> -
> -  flds = create_field (NULL, t, "");
> -  flds->opt = create_string_option (nodot, "length",
> -				    "TREE_OPERAND_LENGTH ((tree) &%0)");
> -  flds->opt = create_string_option (flds->opt, "default", "");
> -
> -  return new_structure ("tree_exp_subunion", TYPE_UNION, &lexer_line, flds,
> -			nodot, NULL);
> -}
> -
>   /* Perform any special processing on a type T, about to become the type
>      of a field.  Return the appropriate type for the field.
>      At present:
> @@ -1447,9 +1416,7 @@ adjust_field_type (type_p t, options_p opt)
>   	     && opt->kind == OPTION_STRING)
>         {
>   	const char *special_name = opt->info.string;
> -	if (strcmp (special_name, "tree_exp") == 0)
> -	  t = adjust_field_tree_exp (t, opt);
> -	else if (strcmp (special_name, "rtx_def") == 0)
> +	if (strcmp (special_name, "rtx_def") == 0)
>   	  t = adjust_field_rtx_def (t, opt);
>   	else
>   	  error_at_line (&lexer_line, "unknown special `%s'", special_name);
> diff --git a/gcc/tree-core.h b/gcc/tree-core.h
> index 91ae5237d7e..c48018999ce 100644
> --- a/gcc/tree-core.h
> +++ b/gcc/tree-core.h
> @@ -1547,9 +1547,7 @@ enum omp_clause_linear_kind
>   struct GTY(()) tree_exp {
>     struct tree_typed typed;
>     location_t locus;
> -  tree GTY ((special ("tree_exp"),
> -	     desc ("TREE_CODE ((tree) &%0)")))
> -    operands[1];
> +  tree GTY ((length ("TREE_OPERAND_LENGTH ((tree)&%h)"))) operands[1];
>   };
>   
>   /* Immediate use linking structure.  This structure is used for maintaining

Re: [PATCH] gengtype: remove "tree_exp" special attribute

[Patrick Palka]

On Wed, Dec 15, 2021 at 5:10 PM Jason Merrill <jason@redhat.com> wrote:
>
> On 12/10/21 10:41, Patrick Palka wrote:
> > The function comment for adjust_field_tree_exp says this special case
> > is for handling trees whose operands may contain pointers to RTL instead
> > of to trees.  But ever since r0-59671, which fixed/removed the last two
> > tree codes for which this was possible (GOTO_SUBROUTINE_EXPR and
> > WITH_CLEANUP_EXPR), this special attribute is largely a no-op.
> >
> > This patch removes it and instead just annotates struct tree_exp
> > with the "length" attribute directly.  Not sure it makes a difference,
> > but I use %h instead of %0 in the "length" attribute to be consistent
> > with other structures' "length" attributes within tree-core.h.
> >
> > This changes the code generated for TS_EXP handling in gt-cp-tree.h from:
> >
> >    case TS_EXP:
> >      gt_ggc_m_9tree_node ((*x).generic.exp.typed.type);
> >      switch ((int) (TREE_CODE ((tree) &(*x))))
> >        {
> >        default:
> >       {
> >         size_t i3;
> >         size_t l3 = (size_t)(TREE_OPERAND_LENGTH ((tree) &(*x)));
> >         for (i3 = 0; i3 != l3; i3++) {
> >           gt_ggc_m_9tree_node ((*x).generic.exp.operands[i3]);
> >         }
> >       }
> >       break;
> >        }
> >      break;
> >
> > to:
> >
> >    case TS_EXP:
> >      {
> >        size_t l3 = (size_t)(TREE_OPERAND_LENGTH ((tree)&((*x).generic.exp)));
> >        gt_ggc_m_9tree_node ((*x).generic.exp.typed.type);
> >        {
> >       size_t i3;
> >       for (i3 = 0; i3 != l3; i3++) {
> >         gt_ggc_m_9tree_node ((*x).generic.exp.operands[i3]);
> >       }
> >        }
> >      }
> >
> > which seems equivalent and simpler.
> >
> > Boostrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk?
>
> OK.

Thanks, I just pushed this patch to trunk now that we're back in stage1.

>
> > gcc/ChangeLog:
> >
> >       * gengtype.c (adjust_field_tree_exp): Remove.
> >       (adjust_field_type): Don't handle the "tree_exp" special attribute.
> >       * tree-core.h (struct tree_exp): Replace special and desc
> >       attributes with length.
> > ---
> >   gcc/gengtype.c  | 35 +----------------------------------
> >   gcc/tree-core.h |  4 +---
> >   2 files changed, 2 insertions(+), 37 deletions(-)
> >
> > diff --git a/gcc/gengtype.c b/gcc/gengtype.c
> > index db218a7bce7..2dc443175fa 100644
> > --- a/gcc/gengtype.c
> > +++ b/gcc/gengtype.c
> > @@ -511,7 +511,6 @@ pair_p typedefs = NULL;
> >   type_p structures = NULL;
> >   pair_p variables = NULL;
> >
> > -static type_p adjust_field_tree_exp (type_p t, options_p opt);
> >   static type_p adjust_field_rtx_def (type_p t, options_p opt);
> >
> >   /* Define S as a typedef to T at POS.  */
> > @@ -1384,36 +1383,6 @@ adjust_field_rtx_def (type_p t, options_p ARG_UNUSED (opt))
> >                       nodot, NULL);
> >   }
> >
> > -/* Handle `special("tree_exp")'.  This is a special case for
> > -   field `operands' of struct tree_exp, which although it claims to contain
> > -   pointers to trees, actually sometimes contains pointers to RTL too.
> > -   Passed T, the old type of the field, and OPT its options.  Returns
> > -   a new type for the field.  */
> > -
> > -static type_p
> > -adjust_field_tree_exp (type_p t, options_p opt ATTRIBUTE_UNUSED)
> > -{
> > -  pair_p flds;
> > -  options_p nodot;
> > -
> > -  if (t->kind != TYPE_ARRAY)
> > -    {
> > -      error_at_line (&lexer_line,
> > -                  "special `tree_exp' must be applied to an array");
> > -      return &string_type;
> > -    }
> > -
> > -  nodot = create_string_option (NULL, "dot", "");
> > -
> > -  flds = create_field (NULL, t, "");
> > -  flds->opt = create_string_option (nodot, "length",
> > -                                 "TREE_OPERAND_LENGTH ((tree) &%0)");
> > -  flds->opt = create_string_option (flds->opt, "default", "");
> > -
> > -  return new_structure ("tree_exp_subunion", TYPE_UNION, &lexer_line, flds,
> > -                     nodot, NULL);
> > -}
> > -
> >   /* Perform any special processing on a type T, about to become the type
> >      of a field.  Return the appropriate type for the field.
> >      At present:
> > @@ -1447,9 +1416,7 @@ adjust_field_type (type_p t, options_p opt)
> >            && opt->kind == OPTION_STRING)
> >         {
> >       const char *special_name = opt->info.string;
> > -     if (strcmp (special_name, "tree_exp") == 0)
> > -       t = adjust_field_tree_exp (t, opt);
> > -     else if (strcmp (special_name, "rtx_def") == 0)
> > +     if (strcmp (special_name, "rtx_def") == 0)
> >         t = adjust_field_rtx_def (t, opt);
> >       else
> >         error_at_line (&lexer_line, "unknown special `%s'", special_name);
> > diff --git a/gcc/tree-core.h b/gcc/tree-core.h
> > index 91ae5237d7e..c48018999ce 100644
> > --- a/gcc/tree-core.h
> > +++ b/gcc/tree-core.h
> > @@ -1547,9 +1547,7 @@ enum omp_clause_linear_kind
> >   struct GTY(()) tree_exp {
> >     struct tree_typed typed;
> >     location_t locus;
> > -  tree GTY ((special ("tree_exp"),
> > -          desc ("TREE_CODE ((tree) &%0)")))
> > -    operands[1];
> > +  tree GTY ((length ("TREE_OPERAND_LENGTH ((tree)&%h)"))) operands[1];
> >   };
> >
> >   /* Immediate use linking structure.  This structure is used for maintaining
>