[PATCH PR/82546] tree node size

[PATCH PR/82546] tree node size

[Nathan Sidwell]

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

[Although I filed this as a middle-end bug, it's really a core infra 
bug, not sure who the best reviewer is]

In working on tree streaming in the modules branch, I discovered poor 
tree node size and hierarchy bits.

Here's a fix for the first part of that.  tree.c (tree_code_size) 
returns sizeof (tree_type_non_common) for any tcc_type node. That's 
wasteful, given we have tree_type_common-> 
tree_type_with_lang_specific-> tree_type_non_common available as 
choices.  It's also obscuring defects in (at least) the c++ FE where we 
use tree_type_non_common fields, but claim the node doesn't contain that 
structure.   Ew.

This patch makes tree_code_size ask the lang hook for the size of 
non-core type nodes.  It also fixes the c++ and objc FEs to return a 
size for the nodes it cares about.

I don't (yet) know whether all the core types are tree_type_non_common, 
nor whether the FE's can return smaller sizes than the do with this 
patch.  But at least the control flow is now correct.  during developing 
this patch I added an assert that the lang hook was returning a size at 
least as big as tree_type_non_common, so they couldn't be more broken 
than before the patch.

I intend to continue cleaning this up of course.  It's not clear to me 
whether we should cache these node sizes in an array, and the way it 
goes about checking nodes with nested switches is understandable, but 
possible not the fastest solution. However let's at least get the sizing 
right first.

ok?

nathan
-- 
Nathan Sidwell

[-- Attachment #2: ns.diff --]
[-- Type: text/x-patch, Size: 8810 bytes --]

2017-10-13  Nathan Sidwell  <nathan@acm.org>

	PR middle-end/82546
	gcc/
	* tree.c (tree_code_size): Reformat.  Punt to lang hook for unknown
	TYPE nodes.
	gcc/cp/
	* cp-objcp-common.c (cp_tree_size): Reformat.  Adjust returns size
	of TYPE nodes.
	gcc/objc/
	* objc-act.c (objc_common_tree_size): Return size of TYPE nodes.

Index: cp/cp-objcp-common.c
===================================================================
--- cp/cp-objcp-common.c	(revision 253733)
+++ cp/cp-objcp-common.c	(working copy)
@@ -61,43 +61,34 @@ cxx_warn_unused_global_decl (const_tree
 size_t
 cp_tree_size (enum tree_code code)
 {
+  gcc_checking_assert (code >= NUM_TREE_CODES);
   switch (code)
     {
-    case PTRMEM_CST:		return sizeof (struct ptrmem_cst);
-    case BASELINK:		return sizeof (struct tree_baselink);
+    case PTRMEM_CST:		return sizeof (ptrmem_cst);
+    case BASELINK:		return sizeof (tree_baselink);
     case TEMPLATE_PARM_INDEX:	return sizeof (template_parm_index);
-    case DEFAULT_ARG:		return sizeof (struct tree_default_arg);
-    case DEFERRED_NOEXCEPT:	return sizeof (struct tree_deferred_noexcept);
-    case OVERLOAD:		return sizeof (struct tree_overload);
-    case STATIC_ASSERT:         return sizeof (struct tree_static_assert);
+    case DEFAULT_ARG:		return sizeof (tree_default_arg);
+    case DEFERRED_NOEXCEPT:	return sizeof (tree_deferred_noexcept);
+    case OVERLOAD:		return sizeof (tree_overload);
+    case STATIC_ASSERT:         return sizeof (tree_static_assert);
     case TYPE_ARGUMENT_PACK:
-    case TYPE_PACK_EXPANSION:
-      return sizeof (struct tree_common);
-
+    case TYPE_PACK_EXPANSION:	return sizeof (tree_type_non_common);
     case NONTYPE_ARGUMENT_PACK:
-    case EXPR_PACK_EXPANSION:
-      return sizeof (struct tree_exp);
-
-    case ARGUMENT_PACK_SELECT:
-      return sizeof (struct tree_argument_pack_select);
-
-    case TRAIT_EXPR:
-      return sizeof (struct tree_trait_expr);
-
-    case LAMBDA_EXPR:           return sizeof (struct tree_lambda_expr);
-
-    case TEMPLATE_INFO:         return sizeof (struct tree_template_info);
-
-    case CONSTRAINT_INFO:       return sizeof (struct tree_constraint_info);
-
-    case USERDEF_LITERAL:	return sizeof (struct tree_userdef_literal);
-
-    case TEMPLATE_DECL:		return sizeof (struct tree_template_decl);
-
+    case EXPR_PACK_EXPANSION:	return sizeof (tree_exp);
+    case ARGUMENT_PACK_SELECT:	return sizeof (tree_argument_pack_select);
+    case TRAIT_EXPR:		return sizeof (tree_trait_expr);
+    case LAMBDA_EXPR:           return sizeof (tree_lambda_expr);
+    case TEMPLATE_INFO:         return sizeof (tree_template_info);
+    case CONSTRAINT_INFO:       return sizeof (tree_constraint_info);
+    case USERDEF_LITERAL:	return sizeof (tree_userdef_literal);
+    case TEMPLATE_DECL:		return sizeof (tree_template_decl);
     default:
-      if (TREE_CODE_CLASS (code) == tcc_declaration)
-	return sizeof (struct tree_decl_non_common);
-      gcc_unreachable ();
+      switch (TREE_CODE_CLASS (code))
+	{
+	case tcc_declaration:	return sizeof (tree_decl_non_common);
+	case tcc_type:		return sizeof (tree_type_non_common);
+	default: gcc_unreachable ();
+	}
     }
   /* NOTREACHED */
 }
Index: objc/objc-act.c
===================================================================
--- objc/objc-act.c	(revision 253733)
+++ objc/objc-act.c	(working copy)
@@ -10118,11 +10118,14 @@ objc_common_tree_size (enum tree_code co
     case CLASS_METHOD_DECL:
     case INSTANCE_METHOD_DECL:
     case KEYWORD_DECL:
-    case PROPERTY_DECL:
-      return sizeof (struct tree_decl_non_common);
+    case PROPERTY_DECL:			return sizeof (tree_decl_non_common);
+    case CLASS_INTERFACE_TYPE:
+    case CLASS_IMPLEMENTATION_TYPE:
+    case CATEGORY_INTERFACE_TYPE:
+    case CATEGORY_IMPLEMENTATION_TYPE:
+    case PROTOCOL_INTERFACE_TYPE:	return sizeof (tree_type_non_common);
     default:
       gcc_unreachable ();
-  
     }
 }
 
Index: tree.c
===================================================================
--- tree.c	(revision 253733)
+++ tree.c	(working copy)
@@ -763,40 +763,53 @@ tree_code_size (enum tree_code code)
   switch (TREE_CODE_CLASS (code))
     {
     case tcc_declaration:  /* A decl node */
-      {
-	switch (code)
-	  {
-	  case FIELD_DECL:
-	    return sizeof (struct tree_field_decl);
-	  case PARM_DECL:
-	    return sizeof (struct tree_parm_decl);
-	  case VAR_DECL:
-	    return sizeof (struct tree_var_decl);
-	  case LABEL_DECL:
-	    return sizeof (struct tree_label_decl);
-	  case RESULT_DECL:
-	    return sizeof (struct tree_result_decl);
-	  case CONST_DECL:
-	    return sizeof (struct tree_const_decl);
-	  case TYPE_DECL:
-	    return sizeof (struct tree_type_decl);
-	  case FUNCTION_DECL:
-	    return sizeof (struct tree_function_decl);
-	  case DEBUG_EXPR_DECL:
-	    return sizeof (struct tree_decl_with_rtl);
-	  case TRANSLATION_UNIT_DECL:
-	    return sizeof (struct tree_translation_unit_decl);
-	  case NAMESPACE_DECL:
-	  case IMPORTED_DECL:
-	  case NAMELIST_DECL:
-	    return sizeof (struct tree_decl_non_common);
-	  default:
-	    return lang_hooks.tree_size (code);
-	  }
-      }
+      switch (code)
+	{
+	case FIELD_DECL:	return sizeof (tree_field_decl);
+	case PARM_DECL:		return sizeof (tree_parm_decl);
+	case VAR_DECL:		return sizeof (tree_var_decl);
+	case LABEL_DECL:	return sizeof (tree_label_decl);
+	case RESULT_DECL:	return sizeof (tree_result_decl);
+	case CONST_DECL:	return sizeof (tree_const_decl);
+	case TYPE_DECL:		return sizeof (tree_type_decl);
+	case FUNCTION_DECL:	return sizeof (tree_function_decl);
+	case DEBUG_EXPR_DECL:	return sizeof (tree_decl_with_rtl);
+	case TRANSLATION_UNIT_DECL: return sizeof (tree_translation_unit_decl);
+	case NAMESPACE_DECL:
+	case IMPORTED_DECL:
+	case NAMELIST_DECL:	return sizeof (tree_decl_non_common);
+	default:
+	  gcc_checking_assert (code >= NUM_TREE_CODES);
+	  return lang_hooks.tree_size (code);
+	}
 
     case tcc_type:  /* a type node */
-      return sizeof (struct tree_type_non_common);
+      switch (code)
+	{
+	case OFFSET_TYPE:
+	case ENUMERAL_TYPE:
+	case BOOLEAN_TYPE:
+	case INTEGER_TYPE:
+	case REAL_TYPE:
+	case POINTER_TYPE:
+	case REFERENCE_TYPE:
+	case NULLPTR_TYPE:
+	case FIXED_POINT_TYPE:
+	case COMPLEX_TYPE:
+	case VECTOR_TYPE:
+	case ARRAY_TYPE:
+	case RECORD_TYPE:
+	case UNION_TYPE:
+	case QUAL_UNION_TYPE:
+	case VOID_TYPE:
+	case POINTER_BOUNDS_TYPE:
+	case FUNCTION_TYPE:
+	case METHOD_TYPE:
+	case LANG_TYPE:		return sizeof (tree_type_non_common);
+	default:
+	  gcc_checking_assert (code >= NUM_TREE_CODES);
+	  return lang_hooks.tree_size (code);
+	}
 
     case tcc_reference:   /* a reference */
     case tcc_expression:  /* an expression */
@@ -810,14 +823,15 @@ tree_code_size (enum tree_code code)
     case tcc_constant:  /* a constant */
       switch (code)
 	{
-	case VOID_CST:		return sizeof (struct tree_typed);
+	case VOID_CST:		return sizeof (tree_typed);
 	case INTEGER_CST:	gcc_unreachable ();
-	case REAL_CST:		return sizeof (struct tree_real_cst);
-	case FIXED_CST:		return sizeof (struct tree_fixed_cst);
-	case COMPLEX_CST:	return sizeof (struct tree_complex);
-	case VECTOR_CST:	return sizeof (struct tree_vector);
+	case REAL_CST:		return sizeof (tree_real_cst);
+	case FIXED_CST:		return sizeof (tree_fixed_cst);
+	case COMPLEX_CST:	return sizeof (tree_complex);
+	case VECTOR_CST:	return sizeof (tree_vector);
 	case STRING_CST:	gcc_unreachable ();
 	default:
+	  gcc_checking_assert (code >= NUM_TREE_CODES);
 	  return lang_hooks.tree_size (code);
 	}
 
@@ -825,23 +839,24 @@ tree_code_size (enum tree_code code)
       switch (code)
 	{
 	case IDENTIFIER_NODE:	return lang_hooks.identifier_size;
-	case TREE_LIST:		return sizeof (struct tree_list);
+	case TREE_LIST:		return sizeof (tree_list);
 
 	case ERROR_MARK:
-	case PLACEHOLDER_EXPR:	return sizeof (struct tree_common);
+	case PLACEHOLDER_EXPR:	return sizeof (tree_common);
 
-	case TREE_VEC:
+	case TREE_VEC:		gcc_unreachable ();
 	case OMP_CLAUSE:	gcc_unreachable ();
 
-	case SSA_NAME:		return sizeof (struct tree_ssa_name);
+	case SSA_NAME:		return sizeof (tree_ssa_name);
 
-	case STATEMENT_LIST:	return sizeof (struct tree_statement_list);
+	case STATEMENT_LIST:	return sizeof (tree_statement_list);
 	case BLOCK:		return sizeof (struct tree_block);
-	case CONSTRUCTOR:	return sizeof (struct tree_constructor);
-	case OPTIMIZATION_NODE: return sizeof (struct tree_optimization_option);
-	case TARGET_OPTION_NODE: return sizeof (struct tree_target_option);
+	case CONSTRUCTOR:	return sizeof (tree_constructor);
+	case OPTIMIZATION_NODE: return sizeof (tree_optimization_option);
+	case TARGET_OPTION_NODE: return sizeof (tree_target_option);
 
 	default:
+	  gcc_checking_assert (code >= NUM_TREE_CODES);
 	  return lang_hooks.tree_size (code);
 	}
 

Re: [PATCH PR/82546] tree node size

[Richard Biener]

On October 13, 2017 8:29:40 PM GMT+02:00, Nathan Sidwell <nathan@acm.org> wrote:
>[Although I filed this as a middle-end bug, it's really a core infra 
>bug, not sure who the best reviewer is]
>
>In working on tree streaming in the modules branch, I discovered poor 
>tree node size and hierarchy bits.
>
>Here's a fix for the first part of that.  tree.c (tree_code_size) 
>returns sizeof (tree_type_non_common) for any tcc_type node. That's 
>wasteful, given we have tree_type_common-> 
>tree_type_with_lang_specific-> tree_type_non_common available as 
>choices.  It's also obscuring defects in (at least) the c++ FE where we
>
>use tree_type_non_common fields, but claim the node doesn't contain
>that 
>structure.   Ew.
>
>This patch makes tree_code_size ask the lang hook for the size of 
>non-core type nodes.  It also fixes the c++ and objc FEs to return a 
>size for the nodes it cares about.
>
>I don't (yet) know whether all the core types are tree_type_non_common,
>
>nor whether the FE's can return smaller sizes than the do with this 
>patch.  But at least the control flow is now correct.  during
>developing 
>this patch I added an assert that the lang hook was returning a size at
>
>least as big as tree_type_non_common, so they couldn't be more broken 
>than before the patch.
>
>I intend to continue cleaning this up of course.  It's not clear to me 
>whether we should cache these node sizes in an array, and the way it 
>goes about checking nodes with nested switches is understandable, but 
>possible not the fastest solution. However let's at least get the
>sizing 
>right first.

We were conservative exactly to avoid the langhook here. I think there's similar 'bug' on the decl side. 

Richard. 

>ok?
>
>nathan

Re: [PATCH PR/82546] tree node size

[Nathan Sidwell]

On 10/16/2017 02:49 AM, Richard Biener wrote:
> On October 13, 2017 8:29:40 PM GMT+02:00, Nathan Sidwell <nathan@acm.org> wrote:

>> I intend to continue cleaning this up of course.  It's not clear to me
>> whether we should cache these node sizes in an array, and the way it
>> goes about checking nodes with nested switches is understandable, but
>> possible not the fastest solution. However let's at least get the
>> sizing
>> right first.
> 
> We were conservative exactly to avoid the langhook here. I think there's similar 'bug' on the decl side.

The other code types (decls, exprs, etc) call the langhook.  tcc_type 
seems the exception (now?).

nathan

-- 
Nathan Sidwell

Re: [PATCH PR/82546] tree node size

[Richard Biener]

On Mon, 16 Oct 2017, Nathan Sidwell wrote:

> On 10/16/2017 02:49 AM, Richard Biener wrote:
> > On October 13, 2017 8:29:40 PM GMT+02:00, Nathan Sidwell <nathan@acm.org>
> > wrote:
> 
> > > I intend to continue cleaning this up of course.  It's not clear to me
> > > whether we should cache these node sizes in an array, and the way it
> > > goes about checking nodes with nested switches is understandable, but
> > > possible not the fastest solution. However let's at least get the
> > > sizing
> > > right first.
> > 
> > We were conservative exactly to avoid the langhook here. I think there's
> > similar 'bug' on the decl side.
> 
> The other code types (decls, exprs, etc) call the langhook.  tcc_type seems
> the exception (now?).

Sorry for not looking at the patch before replying.  The patch looks ok
but shouldn't LANG_TYPE be also handled by the FE?  LANG_TYPE itself
is an odd beast if I may say that - it's only used by the C++ and Ada FEs
and the Ada FE does only

/* Make a dummy type corresponding to GNAT_TYPE.  */

tree
make_dummy_type (Entity_Id gnat_type)
{
...
  /* Create a debug type so that debug info consumers only see an 
unspecified
     type.  */
  if (Needs_Debug_Info (gnat_type))
    {
      debug_type = make_node (LANG_TYPE);
      SET_TYPE_DEBUG_TYPE (gnu_type, debug_type);

      TYPE_NAME (debug_type) = TYPE_NAME (gnu_type);
      TYPE_ARTIFICIAL (debug_type) = TYPE_ARTIFICIAL (gnu_type);
    }


Thus the patch is ok.

Thanks,
Richard.

Re: [PATCH PR/82546] tree node size

[Nathan Sidwell]

On 10/17/2017 05:26 AM, Richard Biener wrote:

> Sorry for not looking at the patch before replying.  The patch looks ok
> but shouldn't LANG_TYPE be also handled by the FE?  LANG_TYPE itself
> is an odd beast if I may say that - it's only used by the C++ and Ada FEs
> and the Ada FE does only

I agree.  I think LANG_TYPE may be from when there were no FE-specific 
nodes.  It should probably be killed and resurrected as appropriate FE 
nodes.

Olivier, as an ADA person, is that something that could be done?

> Thus the patch is ok.

Thanks,

As a heads up, we currently have:

struct type_common;
struct type_with_lang_specific : type_common;
struct type_non_common : type_with_lang_specific;

And many (most?, all?) FE type nodes derive from type_non_common (even 
if, as I discovered, they don't know it).  It seems the hierarchy would 
be better as:

struct type_common;
struct type_non_common : type_common; // FE type derive here
struct type_with_lang_specific : type_non_common;

After all, why would a FE-specific type need a lang-specific pointer?  I 
don't think there are types that just have the land-pointer and don't 
have non_common.

That's the direction I'm heading in with this clean up.

nathan

-- 
Nathan Sidwell

Re: [PATCH PR/82546] tree node size

[Richard Biener]

On Tue, 17 Oct 2017, Nathan Sidwell wrote:

> On 10/17/2017 05:26 AM, Richard Biener wrote:
> 
> > Sorry for not looking at the patch before replying.  The patch looks ok
> > but shouldn't LANG_TYPE be also handled by the FE?  LANG_TYPE itself
> > is an odd beast if I may say that - it's only used by the C++ and Ada FEs
> > and the Ada FE does only
> 
> I agree.  I think LANG_TYPE may be from when there were no FE-specific nodes.
> It should probably be killed and resurrected as appropriate FE nodes.
> 
> Olivier, as an ADA person, is that something that could be done?
> 
> > Thus the patch is ok.
> 
> Thanks,
> 
> As a heads up, we currently have:
> 
> struct type_common;
> struct type_with_lang_specific : type_common;
> struct type_non_common : type_with_lang_specific;
> 
> And many (most?, all?) FE type nodes derive from type_non_common (even if, as
> I discovered, they don't know it).  It seems the hierarchy would be better as:
> 
> struct type_common;
> struct type_non_common : type_common; // FE type derive here
> struct type_with_lang_specific : type_non_common;
> 
> After all, why would a FE-specific type need a lang-specific pointer?  I don't
> think there are types that just have the land-pointer and don't have
> non_common.
> 
> That's the direction I'm heading in with this clean up.

Not sure.  For decls the lang_specific pointer is even in decl_common!

It's probably that there are basically no types w/o a FE using the
lang-specific pointer but there are some types using
type_common fields only (but need lang-specifics).

So maybe do like with decls and remove type_with_lang_specific, instead
folding it into type_common?

Richard.

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Unbreak Ada bootstrap (was Re: [PATCH PR/82546] tree node size)

[Jakub Jelinek]

Hi!

On Fri, Oct 13, 2017 at 02:29:40PM -0400, Nathan Sidwell wrote:
> [Although I filed this as a middle-end bug, it's really a core infra bug,
> not sure who the best reviewer is]

> 2017-10-13  Nathan Sidwell  <nathan@acm.org>
> 
> 	PR middle-end/82546
> 	gcc/
> 	* tree.c (tree_code_size): Reformat.  Punt to lang hook for unknown
> 	TYPE nodes.

This change broke Ada bootstrap, because the FE doesn't have any tree_size
langhook, but has one language specific tcc_type tree -
UNCONSTRAINED_ARRAY_TYPE.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2017-10-17  Jakub Jelinek  <jakub@redhat.com>

	* langhooks.h (struct lang_hooks): Document that tree_size langhook
	may be also called on tcc_type nodes.
	* langhooks.c (lhd_tree_size): Likewise.

	* gcc-interface/misc.c (gnat_tree_size): New function.
	(LANG_HOOKS_TREE_SIZE): Redefine.

--- gcc/langhooks.h.jj	2017-09-12 17:20:17.000000000 +0200
+++ gcc/langhooks.h	2017-10-17 19:49:29.277324006 +0200
@@ -307,10 +307,10 @@ struct lang_hooks
   /* Remove any parts of the tree that are used only by the FE. */
   void (*free_lang_data) (tree);
 
-  /* Determines the size of any language-specific tcc_constant or
-     tcc_exceptional nodes.  Since it is called from make_node, the
-     only information available is the tree code.  Expected to die
-     on unrecognized codes.  */
+  /* Determines the size of any language-specific tcc_constant,
+     tcc_exceptional or tcc_type nodes.  Since it is called from
+     make_node, the only information available is the tree code.
+     Expected to die on unrecognized codes.  */
   size_t (*tree_size) (enum tree_code);
 
   /* Return the language mask used for converting argv into a sequence
--- gcc/langhooks.c.jj	2017-05-21 15:46:13.000000000 +0200
+++ gcc/langhooks.c	2017-10-17 19:47:13.973960166 +0200
@@ -266,8 +266,8 @@ lhd_gimplify_expr (tree *expr_p ATTRIBUT
 }
 
 /* lang_hooks.tree_size: Determine the size of a tree with code C,
-   which is a language-specific tree code in category tcc_constant or
-   tcc_exceptional.  The default expects never to be called.  */
+   which is a language-specific tree code in category tcc_constant,
+   tcc_exceptional or tcc_type.  The default expects never to be called.  */
 size_t
 lhd_tree_size (enum tree_code c ATTRIBUTE_UNUSED)
 {
--- gcc/ada/gcc-interface/misc.c.jj	2017-08-31 23:47:18.000000000 +0200
+++ gcc/ada/gcc-interface/misc.c	2017-10-17 19:48:39.715923329 +0200
@@ -343,6 +343,23 @@ internal_error_function (diagnostic_cont
   Compiler_Abort (sp, sp_loc, true);
 }
 
+/* lang_hooks.tree_size: Determine the size of a tree with code C,
+   which is a language-specific tree code in category tcc_constant,
+   tcc_exceptional or tcc_type.  The default expects never to be called.  */
+
+static size_t
+gnat_tree_size (enum tree_code code)
+{
+  gcc_checking_assert (code >= NUM_TREE_CODES);
+  switch (code)
+    {
+    case UNCONSTRAINED_ARRAY_TYPE:
+      return sizeof (tree_type_non_common);
+    default:
+      gcc_unreachable ();
+    }
+}
+
 /* Perform all the initialization steps that are language-specific.  */
 
 static bool
@@ -1387,6 +1404,8 @@ get_lang_specific (tree node)
 #define LANG_HOOKS_NAME			"GNU Ada"
 #undef  LANG_HOOKS_IDENTIFIER_SIZE
 #define LANG_HOOKS_IDENTIFIER_SIZE	sizeof (struct tree_identifier)
+#undef  LANG_HOOKS_TREE_SIZE
+#define LANG_HOOKS_TREE_SIZE		gnat_tree_size
 #undef  LANG_HOOKS_INIT
 #define LANG_HOOKS_INIT			gnat_init
 #undef  LANG_HOOKS_OPTION_LANG_MASK


	Jakub

Re: Unbreak Ada bootstrap (was Re: [PATCH PR/82546] tree node size)

[Richard Biener]

On October 17, 2017 9:29:46 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>On Fri, Oct 13, 2017 at 02:29:40PM -0400, Nathan Sidwell wrote:
>> [Although I filed this as a middle-end bug, it's really a core infra
>bug,
>> not sure who the best reviewer is]
>
>> 2017-10-13  Nathan Sidwell  <nathan@acm.org>
>> 
>> 	PR middle-end/82546
>> 	gcc/
>> 	* tree.c (tree_code_size): Reformat.  Punt to lang hook for unknown
>> 	TYPE nodes.
>
>This change broke Ada bootstrap, because the FE doesn't have any
>tree_size
>langhook, but has one language specific tcc_type tree -
>UNCONSTRAINED_ARRAY_TYPE.
>
>Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok
>for
>trunk?

OK. 

Richard. 

>2017-10-17  Jakub Jelinek  <jakub@redhat.com>
>
>	* langhooks.h (struct lang_hooks): Document that tree_size langhook
>	may be also called on tcc_type nodes.
>	* langhooks.c (lhd_tree_size): Likewise.
>
>	* gcc-interface/misc.c (gnat_tree_size): New function.
>	(LANG_HOOKS_TREE_SIZE): Redefine.
>
>--- gcc/langhooks.h.jj	2017-09-12 17:20:17.000000000 +0200
>+++ gcc/langhooks.h	2017-10-17 19:49:29.277324006 +0200
>@@ -307,10 +307,10 @@ struct lang_hooks
>   /* Remove any parts of the tree that are used only by the FE. */
>   void (*free_lang_data) (tree);
> 
>-  /* Determines the size of any language-specific tcc_constant or
>-     tcc_exceptional nodes.  Since it is called from make_node, the
>-     only information available is the tree code.  Expected to die
>-     on unrecognized codes.  */
>+  /* Determines the size of any language-specific tcc_constant,
>+     tcc_exceptional or tcc_type nodes.  Since it is called from
>+     make_node, the only information available is the tree code.
>+     Expected to die on unrecognized codes.  */
>   size_t (*tree_size) (enum tree_code);
> 
>   /* Return the language mask used for converting argv into a sequence
>--- gcc/langhooks.c.jj	2017-05-21 15:46:13.000000000 +0200
>+++ gcc/langhooks.c	2017-10-17 19:47:13.973960166 +0200
>@@ -266,8 +266,8 @@ lhd_gimplify_expr (tree *expr_p ATTRIBUT
> }
> 
> /* lang_hooks.tree_size: Determine the size of a tree with code C,
>-   which is a language-specific tree code in category tcc_constant or
>-   tcc_exceptional.  The default expects never to be called.  */
>+   which is a language-specific tree code in category tcc_constant,
>+   tcc_exceptional or tcc_type.  The default expects never to be
>called.  */
> size_t
> lhd_tree_size (enum tree_code c ATTRIBUTE_UNUSED)
> {
>--- gcc/ada/gcc-interface/misc.c.jj	2017-08-31 23:47:18.000000000 +0200
>+++ gcc/ada/gcc-interface/misc.c	2017-10-17 19:48:39.715923329 +0200
>@@ -343,6 +343,23 @@ internal_error_function (diagnostic_cont
>   Compiler_Abort (sp, sp_loc, true);
> }
> 
>+/* lang_hooks.tree_size: Determine the size of a tree with code C,
>+   which is a language-specific tree code in category tcc_constant,
>+   tcc_exceptional or tcc_type.  The default expects never to be
>called.  */
>+
>+static size_t
>+gnat_tree_size (enum tree_code code)
>+{
>+  gcc_checking_assert (code >= NUM_TREE_CODES);
>+  switch (code)
>+    {
>+    case UNCONSTRAINED_ARRAY_TYPE:
>+      return sizeof (tree_type_non_common);
>+    default:
>+      gcc_unreachable ();
>+    }
>+}
>+
>/* Perform all the initialization steps that are language-specific.  */
> 
> static bool
>@@ -1387,6 +1404,8 @@ get_lang_specific (tree node)
> #define LANG_HOOKS_NAME			"GNU Ada"
> #undef  LANG_HOOKS_IDENTIFIER_SIZE
> #define LANG_HOOKS_IDENTIFIER_SIZE	sizeof (struct tree_identifier)
>+#undef  LANG_HOOKS_TREE_SIZE
>+#define LANG_HOOKS_TREE_SIZE		gnat_tree_size
> #undef  LANG_HOOKS_INIT
> #define LANG_HOOKS_INIT			gnat_init
> #undef  LANG_HOOKS_OPTION_LANG_MASK
>
>
>	Jakub

Re: Unbreak Ada bootstrap (was Re: [PATCH PR/82546] tree node size)

[Eric Botcazou]

> This change broke Ada bootstrap, because the FE doesn't have any tree_size
> langhook, but has one language specific tcc_type tree -
> UNCONSTRAINED_ARRAY_TYPE.

There should be a requirement to test all languages for this kind of changes.

> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
> 
> 2017-10-17  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* langhooks.h (struct lang_hooks): Document that tree_size langhook
> 	may be also called on tcc_type nodes.
> 	* langhooks.c (lhd_tree_size): Likewise.
> 
> 	* gcc-interface/misc.c (gnat_tree_size): New function.
> 	(LANG_HOOKS_TREE_SIZE): Redefine.

OK, thanks.

-- 
Eric Botcazou

Re: [PATCH PR/82546] tree node size

[Olivier Hainque]

> On Oct 17, 2017, at 16:10 , Nathan Sidwell <nathan@acm.org> wrote:
> 
> On 10/17/2017 05:26 AM, Richard Biener wrote:
> 
>> Sorry for not looking at the patch before replying.  The patch looks ok
>> but shouldn't LANG_TYPE be also handled by the FE?  LANG_TYPE itself
>> is an odd beast if I may say that - it's only used by the C++ and Ada FEs
>> and the Ada FE does only
> 
> I agree.  I think LANG_TYPE may be from when there were no FE-specific nodes.  It should probably be killed and resurrected as appropriate FE nodes.
> 
> Olivier, as an ADA person, is that something that could be done?

I'd think so. LANG_TYPE is treated specially in several
places and Ada debug types are pretty sensitive so this would
require caution but I don't see/know-of obvious reasons why this
couldn't be done.

Eric and Pierre-Marie (cc'ed) might have more precise insights.

Olivier

Re: [PATCH PR/82546] tree node size

[Eric Botcazou]

> I'd think so. LANG_TYPE is treated specially in several
> places and Ada debug types are pretty sensitive so this would
> require caution but I don't see/know-of obvious reasons why this
> couldn't be done.

LANG_TYPE is only used in Ada to trigger the specific treatment in 
gen_type_die_with_usage for DW_TAG_unspecified_type, so very minor.

-- 
Eric Botcazou

Re: [PATCH PR/82546] tree node size

[Olivier Hainque]

> On 18 Oct 2017, at 15:59, Eric Botcazou <ebotcazou@adacore.com> wrote:
> 
>> I'd think so. LANG_TYPE is treated specially in several
>> places and Ada debug types are pretty sensitive so this would
>> require caution but I don't see/know-of obvious reasons why this
>> couldn't be done.
> 
> LANG_TYPE is only used in Ada to trigger the specific treatment in 
> gen_type_die_with_usage for DW_TAG_unspecified_type, so very minor.

OK, thanks for confirming Eric!