This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Adjust lazy macro definition


We provide a lazy macro definition hook, to deal with the FP macros where it is expensive to generate the string representation of the FP value. The current hook is much more general than needed, but it's not clear that generality can be used because we don't document what macro state can be manipulated. We create the macro early with a bogus FP token, then overwrite the identifier node making it look like a builtin node with an out-of-range index. Upon first use we call the hook which checks the index value, and if ok updates with the saved macro after nadgering the token of interest with the correct string.

This patch makes things cleaner. We don't overload the builtin index meaning, rather we add a lazy field to cpp_macro, and allow the user to control that. There's plenty of spare space there, so this doesn't increase the macro size.

Rather than have the user's hook fish out the cpp_macro object, we provide the object directly, along with the lazy hook value for it to inspect. It can then update the macro as desired and declare victory.

We already have _cpp_maybe_notify_macro_use that deals with lazy definition, it's simply a matter of adjusting that and using it in a few more place that are now easier.

Finally, the PCH machinery was invoking the lazy hook during PCH write out. This seems fragile to me -- changing the memory state during serialization. We don't need to do that, as the lazy machinery is all GTY marked. This does mean we'll lazily define the PCH'd macro upon every first use after reading back, but that doesn't seem costly to me.

booted & tested on x86_64-linux.

nathan
--
Nathan Sidwell
2018-08-17  Nathan Sidwell  <nathan@acm.org>

	libcpp/
	* include/cpplib.h (struct cpp_callbacks): Replace
	user_builtin_macro with user_lazy_macro.
	(struct cpp_macro): add lazy field.
	(enum cpp_builtin_type): Remove BT_FIRST_USER, BT_LAST_USER.
	(cpp_define_lazily): Declare.
	* macro.c (enter_macro_context) Use _cpp_maybe_notify_macro_use.
	(warn_of_redefinition): Use cpp_builtin_macro_p, directly call
	user_lazy_macro hook.
	(_cpp_new_macro): Clear lazy field.
	(cpp_define_lazily): Define.
	(_cpp_notify_macro_use): Adjust lazy definition code.
	(cpp_macro_definition): No need to do lazy definition here.
	* pch.c (write_macdef, save_macros): Likewise.
	gcc/c-family/
	* c-cppbuiltin.c (struct lazy_hex_fp_value_struct): Remove macro
	field.
	(laxy_hex_fp_value_count): Make unsigned.
	(lazy_hex_fp_value): Provided with macro & lazy number.  Directly
	manipulate the macro.
	(builtin_defin_with_hex_fp_value): Adjust callback name, use
	cpp_define_lazily.

Index: gcc/c-family/c-cppbuiltin.c
===================================================================
--- gcc/c-family/c-cppbuiltin.c	(revision 263622)
+++ gcc/c-family/c-cppbuiltin.c	(working copy)
@@ -1570,7 +1570,6 @@ builtin_define_with_int_value (const cha
 struct GTY(()) lazy_hex_fp_value_struct
 {
   const char *hex_str;
-  cpp_macro *macro;
   machine_mode mode;
   int digits;
   const char *fp_suffix;
@@ -1583,36 +1582,35 @@ struct GTY(()) lazy_hex_fp_value_struct
 #define LAZY_HEX_FP_VALUES_CNT (4 * (3 + NUM_FLOATN_NX_TYPES))
 static GTY(()) struct lazy_hex_fp_value_struct
   lazy_hex_fp_values[LAZY_HEX_FP_VALUES_CNT];
-static GTY(()) int lazy_hex_fp_value_count;
+static GTY(()) unsigned lazy_hex_fp_value_count;
 
-static bool
-lazy_hex_fp_value (cpp_reader *pfile ATTRIBUTE_UNUSED,
-		   cpp_hashnode *node)
+static void
+lazy_hex_fp_value (cpp_reader *, cpp_macro *macro, unsigned num)
 {
   REAL_VALUE_TYPE real;
   char dec_str[64], buf1[256];
-  unsigned int idx;
-  if (node->value.builtin < BT_FIRST_USER
-      || (int) node->value.builtin >= BT_FIRST_USER + lazy_hex_fp_value_count)
-    return false;
 
-  idx = node->value.builtin - BT_FIRST_USER;
-  real_from_string (&real, lazy_hex_fp_values[idx].hex_str);
+  gcc_checking_assert (num < lazy_hex_fp_value_count);
+
+  real_from_string (&real, lazy_hex_fp_values[num].hex_str);
   real_to_decimal_for_mode (dec_str, &real, sizeof (dec_str),
-			    lazy_hex_fp_values[idx].digits, 0,
-			    lazy_hex_fp_values[idx].mode);
+			    lazy_hex_fp_values[num].digits, 0,
+			    lazy_hex_fp_values[num].mode);
+
+  size_t len
+    = sprintf (buf1, "%s%s", dec_str, lazy_hex_fp_values[num].fp_suffix);
+  gcc_assert (len < sizeof (buf1));
+  for (unsigned idx = 0; idx < macro->count; idx++)
+    if (macro->exp.tokens[idx].type == CPP_NUMBER)
+      {
+	macro->exp.tokens[idx].val.str.len = len;
+	macro->exp.tokens[idx].val.str.text
+	  = (const unsigned char *) ggc_strdup (buf1);
+	return;
+      }
 
-  sprintf (buf1, "%s%s", dec_str, lazy_hex_fp_values[idx].fp_suffix);
-  node->flags &= ~(NODE_BUILTIN | NODE_USED);
-  node->value.macro = lazy_hex_fp_values[idx].macro;
-  for (idx = 0; idx < node->value.macro->count; idx++)
-    if (node->value.macro->exp.tokens[idx].type == CPP_NUMBER)
-      break;
-  gcc_assert (idx < node->value.macro->count);
-  node->value.macro->exp.tokens[idx].val.str.len = strlen (buf1);
-  node->value.macro->exp.tokens[idx].val.str.text
-    = (const unsigned char *) ggc_strdup (buf1);
-  return true;
+  /* We must have replaced a token.  */
+  gcc_unreachable ();
 }
 
 /* Pass an object-like macro a hexadecimal floating-point value.  */
@@ -1631,23 +1629,18 @@ builtin_define_with_hex_fp_value (const
       && flag_dump_macros == 0
       && !cpp_get_options (parse_in)->traditional)
     {
-      struct cpp_hashnode *node;
       if (lazy_hex_fp_value_count == 0)
-	cpp_get_callbacks (parse_in)->user_builtin_macro = lazy_hex_fp_value;
+	cpp_get_callbacks (parse_in)->user_lazy_macro = lazy_hex_fp_value;
       sprintf (buf2, fp_cast, "1.1");
       sprintf (buf1, "%s=%s", macro, buf2);
       cpp_define (parse_in, buf1);
-      node = C_CPP_HASHNODE (get_identifier (macro));
+      struct cpp_hashnode *node = C_CPP_HASHNODE (get_identifier (macro));
       lazy_hex_fp_values[lazy_hex_fp_value_count].hex_str
 	= ggc_strdup (hex_str);
       lazy_hex_fp_values[lazy_hex_fp_value_count].mode = TYPE_MODE (type);
       lazy_hex_fp_values[lazy_hex_fp_value_count].digits = digits;
       lazy_hex_fp_values[lazy_hex_fp_value_count].fp_suffix = fp_suffix;
-      lazy_hex_fp_values[lazy_hex_fp_value_count].macro = node->value.macro;
-      node->flags |= NODE_BUILTIN;
-      node->value.builtin
-	= (enum cpp_builtin_type) (BT_FIRST_USER + lazy_hex_fp_value_count);
-      lazy_hex_fp_value_count++;
+      cpp_define_lazily (parse_in, node, lazy_hex_fp_value_count++);
       return;
     }
 
Index: libcpp/include/cpplib.h
===================================================================
--- libcpp/include/cpplib.h	(revision 263622)
+++ libcpp/include/cpplib.h	(working copy)
@@ -605,8 +605,8 @@ struct cpp_callbacks
   /* Callback to identify whether an attribute exists.  */
   int (*has_attribute) (cpp_reader *);
 
-  /* Callback that can change a user builtin into normal macro.  */
-  bool (*user_builtin_macro) (cpp_reader *, cpp_hashnode *);
+  /* Callback that can change a user lazy into normal macro.  */
+  void (*user_lazy_macro) (cpp_reader *, cpp_macro *, unsigned);
 
   /* Callback to parse SOURCE_DATE_EPOCH from environment.  */
   time_t (*get_source_date_epoch) (cpp_reader *);
@@ -698,6 +698,9 @@ struct GTY(()) cpp_macro {
   /* Number of parameters.  */
   unsigned short paramc;
 
+  /* Non-zero if this is a user-lazy macro, value provided by user.  */
+  unsigned char lazy;
+
   /* The kind of this macro (ISO, trad or assert) */
   unsigned kind : 2;
 
@@ -778,9 +781,7 @@ enum cpp_builtin_type
   BT_PRAGMA,			/* `_Pragma' operator */
   BT_TIMESTAMP,			/* `__TIMESTAMP__' */
   BT_COUNTER,			/* `__COUNTER__' */
-  BT_HAS_ATTRIBUTE,		/* `__has_attribute__(x)' */
-  BT_FIRST_USER,		/* User defined builtin macros.  */
-  BT_LAST_USER = BT_FIRST_USER + 63
+  BT_HAS_ATTRIBUTE		/* `__has_attribute__(x)' */
 };
 
 #define CPP_HASHNODE(HNODE)	((cpp_hashnode *) (HNODE))
@@ -1001,6 +1002,9 @@ extern void cpp_assert (cpp_reader *, co
 extern void cpp_undef (cpp_reader *, const char *);
 extern void cpp_unassert (cpp_reader *, const char *);
 
+/* Mark a node as a lazily defined macro.  */
+extern void cpp_define_lazily (cpp_reader *, cpp_hashnode *node, unsigned N);
+
 /* Undefine all macros and assertions.  */
 extern void cpp_undef_all (cpp_reader *);
 
Index: libcpp/macro.c
===================================================================
--- libcpp/macro.c	(revision 263622)
+++ libcpp/macro.c	(working copy)
@@ -1273,15 +1273,6 @@ enter_macro_context (cpp_reader *pfile,
      function where set this flag to FALSE.  */
   pfile->about_to_expand_macro_p = true;
 
-  if ((node->flags & NODE_BUILTIN) && !(node->flags & NODE_USED))
-    {
-      node->flags |= NODE_USED;
-      if ((!pfile->cb.user_builtin_macro
-	   || !pfile->cb.user_builtin_macro (pfile, node))
-	  && pfile->cb.used_define)
-	pfile->cb.used_define (pfile, pfile->directive_line, node);
-    }
-
   if (cpp_user_macro_p (node))
     {
       cpp_macro *macro = node->value.macro;
@@ -1328,13 +1319,9 @@ enter_macro_context (cpp_reader *pfile,
       /* Disable the macro within its expansion.  */
       node->flags |= NODE_DISABLED;
 
-      if (!(node->flags & NODE_USED))
-	{
-	  node->flags |= NODE_USED;
-	  if (pfile->cb.used_define)
-	    pfile->cb.used_define (pfile, pfile->directive_line, node);
-	}
-
+      /* Laziness can only affect the expansion tokens of the macro,
+	 not its fun-likeness or parameters.  */
+      _cpp_maybe_notify_macro_use (pfile, node);
       if (pfile->cb.used)
 	pfile->cb.used (pfile, location, node);
 
@@ -2993,7 +2980,6 @@ static bool
 warn_of_redefinition (cpp_reader *pfile, cpp_hashnode *node,
 		      const cpp_macro *macro2)
 {
-  const cpp_macro *macro1;
   unsigned int i;
 
   /* Some redefinitions need to be warned about regardless.  */
@@ -3002,9 +2988,7 @@ warn_of_redefinition (cpp_reader *pfile,
 
   /* Suppress warnings for builtins that lack the NODE_WARN flag,
      unless Wbuiltin-macro-redefined.  */
-  if (node->flags & NODE_BUILTIN
-      && (!pfile->cb.user_builtin_macro
-	  || !pfile->cb.user_builtin_macro (pfile, node)))
+  if (cpp_builtin_macro_p (node))
     return CPP_OPTION (pfile, warn_builtin_macro_redefined);
 
   /* Redefinitions of conditional (context-sensitive) macros, on
@@ -3012,9 +2996,17 @@ warn_of_redefinition (cpp_reader *pfile,
   if (node->flags & NODE_CONDITIONAL)
     return false;
 
+  cpp_macro *macro1 = node->value.macro;
+  if (macro1->lazy)
+    {
+      /* We don't want to mark MACRO as used, but do need to finalize
+	 its laziness.  */
+      pfile->cb.user_lazy_macro (pfile, macro1, macro1->lazy - 1);
+      macro1->lazy = 0;
+    }
+
   /* Redefinition of a macro is allowed if and only if the old and new
      definitions are the same.  (6.10.3 paragraph 2).  */
-  macro1 = node->value.macro;
 
   /* Don't check count here as it can be different in valid
      traditional redefinitions with just whitespace differences.  */
@@ -3481,6 +3473,7 @@ _cpp_new_macro (cpp_reader *pfile, cpp_m
 
   macro->line = pfile->directive_line;
   macro->params = 0;
+  macro->lazy = 0;
   macro->paramc = 0;
   macro->variadic = 0;
   macro->used = !CPP_OPTION (pfile, warn_unused_macros);
@@ -3553,6 +3546,16 @@ _cpp_create_definition (cpp_reader *pfil
   return true;
 }
 
+extern void
+cpp_define_lazily (cpp_reader *pfile, cpp_hashnode *node, unsigned num)
+{
+  cpp_macro *macro = node->value.macro;
+
+  gcc_checking_assert (pfile->cb.user_lazy_macro && macro && num < 255);
+
+  macro->lazy = num + 1;
+}
+
 /* Notify the use of NODE in a macro-aware context (i.e. expanding it,
    or testing its existance).  Also applies any lazy definition.  */
 
@@ -3563,9 +3566,15 @@ _cpp_notify_macro_use (cpp_reader *pfile
   switch (node->type)
     {
     case NT_MACRO:
-      if ((node->flags & NODE_BUILTIN)
-	  && pfile->cb.user_builtin_macro)
-	pfile->cb.user_builtin_macro (pfile, node);
+      if (!(node->flags & NODE_BUILTIN))
+	{
+	  cpp_macro *macro = node->value.macro;
+	  if (macro->lazy)
+	    {
+	      pfile->cb.user_lazy_macro (pfile, macro, macro->lazy - 1);
+	      macro->lazy = 0;
+	    }
+	}
 
       if (pfile->cb.used_define)
 	pfile->cb.used_define (pfile, pfile->directive_line, node);
@@ -3641,23 +3650,12 @@ const unsigned char *
 cpp_macro_definition (cpp_reader *pfile, cpp_hashnode *node)
 {
   unsigned int i, len;
-  const cpp_macro *macro;
   unsigned char *buffer;
 
-  if (node->type != NT_MACRO || (node->flags & NODE_BUILTIN))
-    {
-      if (node->type != NT_MACRO
-	  || !pfile->cb.user_builtin_macro
-          || !pfile->cb.user_builtin_macro (pfile, node))
-	{
-	  cpp_error (pfile, CPP_DL_ICE,
-		     "invalid hash type %d in cpp_macro_definition",
-		     node->type);
-	  return 0;
-	}
-    }
+  gcc_checking_assert (cpp_user_macro_p (node));
+
+  const cpp_macro *macro = node->value.macro;
 
-  macro = node->value.macro;
   /* Calculate length.  */
   len = NODE_LEN (node) * 10 + 2;		/* ' ' and NUL.  */
   if (macro->fun_like)
Index: libcpp/pch.c
===================================================================
--- libcpp/pch.c	(revision 263622)
+++ libcpp/pch.c	(working copy)
@@ -59,9 +59,7 @@ write_macdef (cpp_reader *pfile, cpp_has
       /* FALLTHRU */
 
     case NT_MACRO:
-      if ((hn->flags & NODE_BUILTIN)
-	  && (!pfile->cb.user_builtin_macro
-	      || !pfile->cb.user_builtin_macro (pfile, hn)))
+      if (hn->flags & NODE_BUILTIN)
 	return 1;
 
       {
@@ -760,11 +758,6 @@ save_macros (cpp_reader *r, cpp_hashnode
 {
   struct save_macro_data *data = (struct save_macro_data *)data_p;
 
-  if ((h->flags & NODE_BUILTIN)
-      && h->type == NT_MACRO
-      && r->cb.user_builtin_macro)
-    r->cb.user_builtin_macro (r, h);
-
   if (h->type != NT_VOID
       && (h->flags & NODE_BUILTIN) == 0)
     {

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]