Re: [C++ PATCH] Kill IDENTIFIER_LABEL_VALUE

[Nathan Sidwell]

On 10/25/2017 05:36 PM, Nathan Sidwell wrote:
This patch removes 'label_value' from lang_identifier, shrinking it from 
72 to 64 bytes (on 64-bit machine).   We replace this by augmenting the 
already used per-function named_labels hash table.  This is a major win, 
because labels are extremely rare and there are many identifiers.  We 
also shring the binding structure by a pointer, as the shadowed_labels 
list goes away.


I was a little over zealous killing code, but perhaps now this is being 
a little paranoid.  It restore the UID sorting of labels when inserting 
them into the BLOCK chain.  The original comment was confusing, as it 
mentioned code generation and then debug information.  I think this just 
affects the order of debug records, but ICBW.  For any given function, 
the iteration of the hash table should be stable across versions, unless 
the hash table implementation or the IDENTIFIER_HASH_VALUE changes.  But 
may as well be safe.


I also add the N_ translation markup I forgot about yesterday when 
taking strings out of 'inform' calls.


nathan

--
Nathan Sidwell
2017-10-26  Nathan Sidwell  

	* decl.c (sort_labels): Restore function.
	(pop_labels): Sort labels
	(identify_goto): Add translation markup.

Index: decl.c
===
--- decl.c	(revision 254087)
+++ decl.c	(working copy)
@@ -372,6 +372,18 @@ check_label_used (tree label)
 }
 }
 
+/* Helper function to sort named label entries in a vector by DECL_UID.  */
+
+static int
+sort_labels (const void *a, const void *b)
+{
+  tree label1 = *(tree const *) a;
+  tree label2 = *(tree const *) b;
+
+  /* DECL_UIDs can never be equal.  */
+  return DECL_UID (label1) > DECL_UID (label2) ? -1 : +1;
+}
+
 /* At the end of a function, all labels declared within the function
go out of scope.  BLOCK is the top-level block for the
function.  */
@@ -382,6 +394,12 @@ pop_labels (tree block)
   if (!named_labels)
 return;
 
+  /* We need to add the labels to the block chain, so debug
+ information is emitted.  But, we want the order to be stable so
+ need to sort them first.  Otherwise the debug output could be
+ randomly ordered.  I guess it's mostly stable, unless the hash
+ table implementation changes.  */
+  auto_vec labels (named_labels->elements ());
   hash_table::iterator end (named_labels->end ());
   for (hash_table::iterator iter
 	 (named_labels->begin ()); iter != end; ++iter)
@@ -390,18 +408,21 @@ pop_labels (tree block)
 
   gcc_checking_assert (!ent->outer);
   if (ent->label_decl)
-	{
-	  check_label_used (ent->label_decl);
-
-	  /* Put the labels into the "variables" of the top-level block,
-	 so debugger can see them.  */
-	  DECL_CHAIN (ent->label_decl) = BLOCK_VARS (block);
-	  BLOCK_VARS (block) = ent->label_decl;
-	}
+	labels.quick_push (ent->label_decl);
   ggc_free (ent);
 }
-
   named_labels = NULL;
+  labels.qsort (sort_labels);
+
+  while (labels.length ())
+{
+  tree label = labels.pop ();
+
+  DECL_CHAIN (label) = BLOCK_VARS (block);
+  BLOCK_VARS (block) = label;
+
+  check_label_used (label);
+}
 }
 
 /* At the end of a block with local labels, restore the outer definition.  */
@@ -3066,8 +3087,8 @@ identify_goto (tree decl, location_t loc
 {
   bool complained
 = emit_diagnostic (diag_kind, loc, 0,
-		   decl ? "jump to label %qD" : "jump to case label",
-		   decl);
+		   decl ? N_("jump to label %qD")
+		   : N_("jump to case label"), decl);
   if (complained && locus)
 inform (*locus, "  from here");
   return complained;
@@ -3136,32 +3157,32 @@ check_previous_goto_1 (tree decl, cp_bin
 	{
 	case sk_try:
 	  if (!saw_eh)
-	inf = "enters try block";
+	inf = N_("enters try block");
 	  saw_eh = true;
 	  break;
 
 	case sk_catch:
 	  if (!saw_eh)
-	inf = "enters catch block";
+	inf = N_("enters catch block");
 	  saw_eh = true;
 	  break;
 
 	case sk_omp:
 	  if (!saw_omp)
-	inf = "enters OpenMP structured block";
+	inf = N_("enters OpenMP structured block");
 	  saw_omp = true;
 	  break;
 
 	case sk_transaction:
 	  if (!saw_tm)
-	inf = "enters synchronized or atomic statement";
+	inf = N_("enters synchronized or atomic statement");
 	  saw_tm = true;
 	  break;
 
 	case sk_block:
 	  if (!saw_cxif && level_for_constexpr_if (b->level_chain))
 	{
-	  inf = "enters constexpr if statement";
+	  inf = N_("enters constexpr if statement");
 	  loc = EXPR_LOCATION (b->level_chain->this_entity);
 	  saw_cxif = true;
 	}

[C++ PATCH] Kill IDENTIFIER_LABEL_VALUE

[Nathan Sidwell]

This patch removes 'label_value' from lang_identifier, shrinking it from 
72 to 64 bytes (on 64-bit machine).   We replace this by augmenting the 
already used per-function named_labels hash table.  This is a major win, 
because labels are extremely rare and there are many identifiers.  We 
also shring the binding structure by a pointer, as the shadowed_labels 
list goes away.


The slight difficulty is with the declared label extension.  These have 
a regular scope and can hide an outer binding.  We stash the outer 
binding in the named_label_entry and restore it when popping that scope.


Applying to trunk

nathan
--
Nathan Sidwell
2017-10-25  Nathan Sidwell  

	Kill IDENTIFIER_LABEL_VALUE.
	* cp-tree.h (lang_identifier): Delete label_value slot.
	(IDENTIFIER_LABEL_VALUE, SET_IDENTIFIER_LABEL_VALUE): Delete.
	(struct named_label_hasher): Rename to ...
	(struct named_label_hash): ... here.  Reimplement.
	(struct language_function): Adjust x_named_labels.
	* name-lookup.h (struct cp_label_binding): Delete.
	(struct cp_binding_level): Delete shadowed_labels slot.
	* decl.c (struct named_label_entry): Add name and outer slots.
	(pop_label): Rename to ...
	(check_label_used): ... here.  Don't pop.
	(note_label, sort_labels): Delete.
	(pop_labels, pop_local_label): Reimplement.
	(poplevel): Pop local labels as any other decl. Remove
	shadowed_labels handling.
	(named_label_hash::hash, named_label_hash::equal): New.
	(make_label_decl): Absorb into ...
	(lookup_label_1): ... here.  Add making_local_p arg, reimplement.
	(lookup_label, declare_local_label): Adjust.
	(check_goto, define_label): Adjust.
	* lex.c (make_conv_op_name): Don't clear IDENTIFIER_LABEL_VALUE.
	* ptree.c (cxx_print_identifier): Don't print identifier binding.

Index: cp-tree.h
===
--- cp-tree.h	(revision 254086)
+++ cp-tree.h	(working copy)
@@ -561,7 +561,6 @@ extern GTY(()) tree cp_global_trees[CPTI
 struct GTY(()) lang_identifier {
   struct c_common_identifier c_common;
   cxx_binding *bindings;
-  tree label_value;
 };
 
 /* Return a typed pointer version of T if it designates a
@@ -996,11 +995,6 @@ enum GTY(()) abstract_class_use {
 #define SET_IDENTIFIER_TYPE_VALUE(NODE,TYPE) (TREE_TYPE (NODE) = (TYPE))
 #define IDENTIFIER_HAS_TYPE_VALUE(NODE) (IDENTIFIER_TYPE_VALUE (NODE) ? 1 : 0)
 
-#define IDENTIFIER_LABEL_VALUE(NODE) \
-  (LANG_IDENTIFIER_CAST (NODE)->label_value)
-#define SET_IDENTIFIER_LABEL_VALUE(NODE, VALUE)   \
-  IDENTIFIER_LABEL_VALUE (NODE) = (VALUE)
-
 /* Kinds of identifiers.  Values are carefully chosen.  */
 enum cp_identifier_kind {
   cik_normal = 0,	/* Not a special identifier.  */
@@ -1662,12 +1656,22 @@ struct cxx_int_tree_map_hasher : ggc_ptr
   static bool equal (cxx_int_tree_map *, cxx_int_tree_map *);
 };
 
-struct named_label_entry;
+struct named_label_entry; /* Defined in decl.c.  */
 
-struct named_label_hasher : ggc_ptr_hash
+struct named_label_hash : ggc_remove 
 {
-  static hashval_t hash (named_label_entry *);
-  static bool equal (named_label_entry *, named_label_entry *);
+  typedef named_label_entry *value_type;
+  typedef tree compare_type; /* An identifier.  */
+
+  inline static hashval_t hash (value_type);
+  inline static bool equal (const value_type, compare_type);
+
+  inline static void mark_empty (value_type ) {p = NULL;}
+  inline static bool is_empty (value_type p) {return !p;}
+
+  /* Nothing is deletable.  Everything is insertable.  */
+  inline static bool is_deleted (value_type) { return false; }
+  inline static void mark_deleted (value_type) { gcc_unreachable (); }
 };
 
 /* Global state pertinent to the current function.  */
@@ -1696,7 +1700,8 @@ struct GTY(()) language_function {
 
   BOOL_BITFIELD invalid_constexpr : 1;
 
-  hash_table *x_named_labels;
+  hash_table *x_named_labels;
+
   cp_binding_level *bindings;
   vec *x_local_names;
   /* Tracking possibly infinite loops.  This is a vec only because
Index: decl.c
===
--- decl.c	(revision 254086)
+++ decl.c	(working copy)
@@ -189,27 +189,33 @@ struct GTY((chain_next ("%h.next"))) nam
function, and so we can check the validity of jumps to these labels.  */
 
 struct GTY((for_user)) named_label_entry {
-  /* The decl itself.  */
-  tree label_decl;
+
+  tree name;  /* Name of decl. */
+
+  tree label_decl; /* LABEL_DECL, unless deleted local label. */
+
+  named_label_entry *outer; /* Outer shadowed chain.  */
 
   /* The binding level to which the label is *currently* attached.
  This is initially set to the binding level in which the label
  is defined, but is modified as scopes are closed.  */
   cp_binding_level *binding_level;
+
   /* The head of the names list that was current when the label was
  defined, or the inner scope popped.  These are the decls that will
  be skipped when jumping to the label.  */
   tree names_in_scope;
+
   /* A vector of