diff mbox

Lazy computation of __DBL_MAX__ etc. macros

Message ID 20100608164618.GY10293@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek June 8, 2010, 4:46 p.m. UTC
On Mon, Jun 07, 2010 at 07:41:59PM +0200, Jakub Jelinek wrote:
> An alternative would be to precompute these decimal constants,
> either for the most commonly used real_format structs and __DECIMAL_DIG__
> values, or for all of them.  There are 8 different fmt->p values though,
> 21 formats and 4 strings for each, so precomputing everything is probably
> not a good idea.  Precomputing just 12 values we need together with which
> format it is and __DECIMAL_DIG__ value for which it has been precomputed
> could be useful though if we keep the code to compute it at runtime.

Precomputation wouldn't be easy.

Here is an alternative patch that handles the __DBL_MAX__ etc. macros
lazily.

time ( i=1; while [ $i -lt 10000 ]; do ./cc1.vanilla -quiet /dev/null -o /tmp/x.s; let ++i; done )

real	1m58.998s
user	0m53.662s
sys	0m57.413s
time ( i=1; while [ $i -lt 10000 ]; do ./cc1.patched -quiet /dev/null -o /tmp/x.s; let ++i; done )

real	1m53.936s
user	0m48.371s
sys	0m57.806s

or almost 10% user time improvement for empty file compilation.
Adding a hook that would return text and be called from
_cpp_builtin_macro_text didn't unfortunately work, because for double
constants the macros expand to more than one token - ((double)1.234L) etc.
so the patch instead in a callback changes a NODE_BUILTIN macro back
to non-builtin after adjusting the token list.

2010-06-08  Jakub Jelinek  <jakub@redhat.com>

	* include/cpplib.h (struct cpp_callbacks): Add user_builtin_macro
	callback.
	(enum cpp_builtin_type): Add BT_FIRST_USER and BT_LAST_USER.
	(cpp_macro_definition): Remove const qual from second argument.
	* macro.c (enter_macro_context): Call user_builtin_macro callback for
	NODE_BUILTIN !NODE_USED macros.
	(warn_of_redefinition): Likewise.  Remove const qual from second
	argument.
	(cpp_macro_definition): Likewise.
	* pch.c (write_macdef, save_macros): Call user_builtin_macro callback
	for NODE_BUILTIN !NODE_USED macros.

	* c-family/c-cppbuiltin.c: Include cpp-id-data.h.
	(lazy_hex_fp_values, lazy_hex_fp_value_count): New variables.
	(lazy_hex_fp_value): New function.
	(builtin_define_with_hex_fp_value): Provide definitions lazily.
	* Makefile.in (c-family/c-cppbuiltin.o): Depend on $(CPP_ID_DATA_H).



	Jakub

Comments

Joseph Myers June 10, 2010, 7:42 p.m. UTC | #1
On Tue, 8 Jun 2010, Jakub Jelinek wrote:

> 2010-06-08  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* include/cpplib.h (struct cpp_callbacks): Add user_builtin_macro
> 	callback.
> 	(enum cpp_builtin_type): Add BT_FIRST_USER and BT_LAST_USER.
> 	(cpp_macro_definition): Remove const qual from second argument.
> 	* macro.c (enter_macro_context): Call user_builtin_macro callback for
> 	NODE_BUILTIN !NODE_USED macros.
> 	(warn_of_redefinition): Likewise.  Remove const qual from second
> 	argument.
> 	(cpp_macro_definition): Likewise.
> 	* pch.c (write_macdef, save_macros): Call user_builtin_macro callback
> 	for NODE_BUILTIN !NODE_USED macros.
> 
> 	* c-family/c-cppbuiltin.c: Include cpp-id-data.h.
> 	(lazy_hex_fp_values, lazy_hex_fp_value_count): New variables.
> 	(lazy_hex_fp_value): New function.
> 	(builtin_define_with_hex_fp_value): Provide definitions lazily.
> 	* Makefile.in (c-family/c-cppbuiltin.o): Depend on $(CPP_ID_DATA_H).

OK if no objections from other cpplib maintainers within 48 hours.
Tom Tromey June 11, 2010, 3:02 p.m. UTC | #2
>>>>> "Jakub" == Jakub Jelinek <jakub@redhat.com> writes:

Jakub> 2010-06-08  Jakub Jelinek  <jakub@redhat.com>
Jakub> 	* include/cpplib.h (struct cpp_callbacks): Add user_builtin_macro
Jakub> 	callback.
Jakub> 	(enum cpp_builtin_type): Add BT_FIRST_USER and BT_LAST_USER.
Jakub> 	(cpp_macro_definition): Remove const qual from second argument.
Jakub> 	* macro.c (enter_macro_context): Call user_builtin_macro callback for
Jakub> 	NODE_BUILTIN !NODE_USED macros.
Jakub> 	(warn_of_redefinition): Likewise.  Remove const qual from second
Jakub> 	argument.
Jakub> 	(cpp_macro_definition): Likewise.
Jakub> 	* pch.c (write_macdef, save_macros): Call user_builtin_macro callback
Jakub> 	for NODE_BUILTIN !NODE_USED macros.

This is fine by me.  Thanks.

Tom
Mike Stump June 12, 2010, 7:40 a.m. UTC | #3
On Jun 8, 2010, at 9:46 AM, Jakub Jelinek wrote:
> Here is an alternative patch that handles the __DBL_MAX__ etc. macros
> lazily.
> 
> time ( i=1; while [ $i -lt 10000 ]; do ./cc1.vanilla -quiet /dev/null -o /tmp/x.s; let ++i; done )
> 
> real	1m58.998s
> user	0m53.662s
> sys	0m57.413s
> time ( i=1; while [ $i -lt 10000 ]; do ./cc1.patched -quiet /dev/null -o /tmp/x.s; let ++i; done )
> 
> real	1m53.936s
> user	0m48.371s
> sys	0m57.806s
> 
> or almost 10% user time improvement for empty file compilation.
> Adding a hook that would return text and be called from
> _cpp_builtin_macro_text didn't unfortunately work, because for double
> constants the macros expand to more than one token - ((double)1.234L) etc.
> so the patch instead in a callback changes a NODE_BUILTIN macro back
> to non-builtin after adjusting the token list.
> 
> 2010-06-08  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* include/cpplib.h (struct cpp_callbacks): Add user_builtin_macro
> 	callback.
> 	(enum cpp_builtin_type): Add BT_FIRST_USER and BT_LAST_USER.
> 	(cpp_macro_definition): Remove const qual from second argument.
> 	* macro.c (enter_macro_context): Call user_builtin_macro callback for
> 	NODE_BUILTIN !NODE_USED macros.
> 	(warn_of_redefinition): Likewise.  Remove const qual from second
> 	argument.
> 	(cpp_macro_definition): Likewise.
> 	* pch.c (write_macdef, save_macros): Call user_builtin_macro callback
> 	for NODE_BUILTIN !NODE_USED macros.
> 
> 	* c-family/c-cppbuiltin.c: Include cpp-id-data.h.
> 	(lazy_hex_fp_values, lazy_hex_fp_value_count): New variables.
> 	(lazy_hex_fp_value): New function.
> 	(builtin_define_with_hex_fp_value): Provide definitions lazily.
> 	* Makefile.in (c-family/c-cppbuiltin.o): Depend on $(CPP_ID_DATA_H).

Seems to tank building on x86_64-apple-darwin10:

In file included from /usr/include/math.h:28:0,
                 from ../../gcc/gcc/genautomata.c:117:
/usr/include/architecture/i386/math.h: In function ‘__inline_isnormalf’:
/usr/include/architecture/i386/math.h:191:2: internal compiler error: in lazy_hex_fp_value, at c-family/c-cppbuiltin.c:987
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
make[3]: *** [build/genautomata.o] Error 1
make[3]: *** Waiting for unfinished jobs....
rm gcc.pod
make[2]: *** [all-stage2-gcc] Error 2

Jack and Dominique noticed this in IRC, but it doesn't seem to be fixed yet.

   191          static __inline__  int __inline_isnormalf( float __x ) { float fabsf = __builtin_fabsf(__x); if( __x != __x ) return 0; return fabsf < __builtin_inff() && fabsf >= __FLT_MIN__; }  

If I revert that patch:

  $ svn diff -c160626 | patch -R -p0

and then build again, it then builds.

> +static bool
> +lazy_hex_fp_value (cpp_reader *pfile ATTRIBUTE_UNUSED,
> +		   cpp_hashnode *node)
> +{
> +  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);
> +  real_to_decimal_for_mode (dec_str, &real, sizeof (dec_str),
> +			    lazy_hex_fp_values[idx].digits, 0,
> +			    lazy_hex_fp_values[idx].mode);
> +
> +  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);

The above line....

> +  node->value.macro->exp.tokens[idx].val.str.len = strlen (buf1);
> +  node->value.macro->exp.tokens[idx].val.str.text
> +    = (const unsigned char *) xstrdup (buf1);
> +  return true;
> +}
diff mbox

Patch

--- libcpp/include/cpplib.h.jj	2010-04-27 08:47:23.000000000 +0200
+++ libcpp/include/cpplib.h	2010-06-08 17:27:33.000000000 +0200
@@ -1,6 +1,6 @@ 
 /* Definitions for CPP library.
    Copyright (C) 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003,
-   2004, 2005, 2007, 2008, 2009
+   2004, 2005, 2007, 2008, 2009, 2010
    Free Software Foundation, Inc.
    Written by Per Bothner, 1994-95.
 
@@ -512,6 +512,9 @@  struct cpp_callbacks
   /* Called whenever a macro is expanded or tested.
      Second argument is the location of the start of the current expansion.  */
   void (*used) (cpp_reader *, source_location, cpp_hashnode *);
+
+  /* Callback that can change a user builtin into normal macro.  */
+  bool (*user_builtin_macro) (cpp_reader *, cpp_hashnode *);
 };
 
 #ifdef VMS
@@ -602,7 +605,9 @@  enum cpp_builtin_type
   BT_STDC,			/* `__STDC__' */
   BT_PRAGMA,			/* `_Pragma' operator */
   BT_TIMESTAMP,			/* `__TIMESTAMP__' */
-  BT_COUNTER			/* `__COUNTER__' */
+  BT_COUNTER,			/* `__COUNTER__' */
+  BT_FIRST_USER,		/* User defined builtin macros.  */
+  BT_LAST_USER = BT_FIRST_USER + 31
 };
 
 #define CPP_HASHNODE(HNODE)	((cpp_hashnode *) (HNODE))
@@ -729,7 +734,7 @@  extern const cpp_token *cpp_get_token (c
 extern const cpp_token *cpp_get_token_with_location (cpp_reader *,
 						     source_location *);
 extern const unsigned char *cpp_macro_definition (cpp_reader *,
-						  const cpp_hashnode *);
+						  cpp_hashnode *);
 extern void _cpp_backup_tokens (cpp_reader *, unsigned int);
 extern const cpp_token *cpp_peek_token (cpp_reader *, int);
 
--- libcpp/macro.c.jj	2010-06-08 16:31:30.000000000 +0200
+++ libcpp/macro.c	2010-06-08 18:00:01.000000000 +0200
@@ -65,7 +65,7 @@  static bool create_iso_definition (cpp_r
 
 static cpp_token *alloc_expansion_token (cpp_reader *, cpp_macro *);
 static cpp_token *lex_expansion_token (cpp_reader *, cpp_macro *);
-static bool warn_of_redefinition (cpp_reader *, const cpp_hashnode *,
+static bool warn_of_redefinition (cpp_reader *, cpp_hashnode *,
 				  const cpp_macro *);
 static bool parse_params (cpp_reader *, cpp_macro *);
 static void check_trad_stringification (cpp_reader *, const cpp_macro *,
@@ -835,7 +835,9 @@  enter_macro_context (cpp_reader *pfile, 
   if ((node->flags & NODE_BUILTIN) && !(node->flags & NODE_USED))
     {
       node->flags |= NODE_USED;
-      if (pfile->cb.used_define)
+      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);
     }
 
@@ -1430,7 +1432,7 @@  _cpp_backup_tokens (cpp_reader *pfile, u
 
 /* Returns nonzero if a macro redefinition warning is required.  */
 static bool
-warn_of_redefinition (cpp_reader *pfile, const cpp_hashnode *node,
+warn_of_redefinition (cpp_reader *pfile, cpp_hashnode *node,
 		      const cpp_macro *macro2)
 {
   const cpp_macro *macro1;
@@ -1442,7 +1444,11 @@  warn_of_redefinition (cpp_reader *pfile,
 
   /* Suppress warnings for builtins that lack the NODE_WARN flag.  */
   if (node->flags & NODE_BUILTIN)
-    return false;
+    {
+      if (!pfile->cb.user_builtin_macro
+	  || !pfile->cb.user_builtin_macro (pfile, node))
+	return false;
+    }
 
   /* Redefinitions of conditional (context-sensitive) macros, on
      the other hand, must be allowed silently.  */
@@ -1982,19 +1988,26 @@  check_trad_stringification (cpp_reader *
    Caller is expected to generate the "#define" bit if needed.  The
    returned text is temporary, and automatically freed later.  */
 const unsigned char *
-cpp_macro_definition (cpp_reader *pfile, const cpp_hashnode *node)
+cpp_macro_definition (cpp_reader *pfile, cpp_hashnode *node)
 {
   unsigned int i, len;
-  const cpp_macro *macro = node->value.macro;
+  const cpp_macro *macro;
   unsigned char *buffer;
 
   if (node->type != NT_MACRO || (node->flags & NODE_BUILTIN))
     {
-      cpp_error (pfile, CPP_DL_ICE,
-		 "invalid hash type %d in cpp_macro_definition", node->type);
-      return 0;
+      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;
+	}
     }
 
+  macro = node->value.macro;
   /* Calculate length.  */
   len = NODE_LEN (node) + 2;			/* ' ' and NUL.  */
   if (macro->fun_like)
--- libcpp/pch.c.jj	2010-04-08 08:49:41.000000000 +0200
+++ libcpp/pch.c	2010-06-08 17:56:48.000000000 +0200
@@ -58,7 +58,9 @@  write_macdef (cpp_reader *pfile, cpp_has
 	return 1;
 
     case NT_MACRO:
-      if ((hn->flags & NODE_BUILTIN))
+      if ((hn->flags & NODE_BUILTIN)
+	  && (!pfile->cb.user_builtin_macro
+	      || !pfile->cb.user_builtin_macro (pfile, hn)))
 	return 1;
 
       {
@@ -759,6 +761,12 @@  static int
 save_macros (cpp_reader *r, cpp_hashnode *h, void *data_p)
 {
   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)
     {
--- gcc/c-family/c-cppbuiltin.c.jj	2010-06-07 11:24:57.000000000 +0200
+++ gcc/c-family/c-cppbuiltin.c	2010-06-08 17:43:24.000000000 +0200
@@ -1,5 +1,5 @@ 
 /* Define builtin-in macros for the C family front ends.
-   Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009
+   Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010
    Free Software Foundation, Inc.
 
 This file is part of GCC.
@@ -33,6 +33,7 @@  along with GCC; see the file COPYING3.  
 #include "toplev.h"
 #include "tm_p.h"		/* For TARGET_CPU_CPP_BUILTINS & friends.  */
 #include "target.h"
+#include "cpp-id-data.h"
 
 #ifndef TARGET_OS_CPP_BUILTINS
 # define TARGET_OS_CPP_BUILTINS()
@@ -946,6 +947,50 @@  builtin_define_with_int_value (const cha
   cpp_define (parse_in, buf);
 }
 
+/* builtin_define_with_hex_fp_value is very expensive, so the following
+   array and function allows it to be done lazily when __DBL_MAX__
+   etc. is first used.  */
+
+static struct
+{
+  const char *hex_str;
+  cpp_macro *macro;
+  enum machine_mode mode;
+  int digits;
+  const char *fp_suffix;
+} lazy_hex_fp_values[12];
+static int lazy_hex_fp_value_count;
+
+static bool
+lazy_hex_fp_value (cpp_reader *pfile ATTRIBUTE_UNUSED,
+		   cpp_hashnode *node)
+{
+  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);
+  real_to_decimal_for_mode (dec_str, &real, sizeof (dec_str),
+			    lazy_hex_fp_values[idx].digits, 0,
+			    lazy_hex_fp_values[idx].mode);
+
+  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 *) xstrdup (buf1);
+  return true;
+}
+
 /* Pass an object-like macro a hexadecimal floating-point value.  */
 static void
 builtin_define_with_hex_fp_value (const char *macro,
@@ -957,6 +1002,29 @@  builtin_define_with_hex_fp_value (const 
   REAL_VALUE_TYPE real;
   char dec_str[64], buf1[256], buf2[256];
 
+  /* This is very expensive, so if possible expand them lazily.  */
+  if (lazy_hex_fp_value_count < 12
+      && 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;
+      sprintf (buf2, fp_cast, "1.1");
+      sprintf (buf1, "%s=%s", macro, buf2);
+      cpp_define (parse_in, buf1);
+      node = C_CPP_HASHNODE (get_identifier (macro));
+      lazy_hex_fp_values[lazy_hex_fp_value_count].hex_str = xstrdup (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 = BT_FIRST_USER + lazy_hex_fp_value_count;
+      lazy_hex_fp_value_count++;
+      return;
+    }
+
   /* Hex values are really cool and convenient, except that they're
      not supported in strict ISO C90 mode.  First, the "p-" sequence
      is not valid as part of a preprocessor number.  Second, we get a
--- gcc/Makefile.in.jj	2010-06-07 11:25:06.000000000 +0200
+++ gcc/Makefile.in	2010-06-08 17:46:49.000000000 +0200
@@ -2076,7 +2076,7 @@  c-family/c-common.o : c-family/c-common.
 c-family/c-cppbuiltin.o : c-family/c-cppbuiltin.c $(CONFIG_H) $(SYSTEM_H) \
 	coretypes.h $(TM_H) $(TREE_H) version.h $(C_COMMON_H) $(C_PRAGMA_H) \
 	$(FLAGS_H) $(TOPLEV_H) output.h $(EXCEPT_H) $(TREE_H) $(TARGET_H) \
-	$(TM_P_H) $(BASEVER) debug.h
+	$(TM_P_H) $(BASEVER) debug.h $(CPP_ID_DATA_H)
 	$(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) \
 		-DBASEVER=$(BASEVER_s) $< $(OUTPUT_OPTION)