public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: Speedup configure and build with system.h
@ 2016-01-25 12:14 Uros Bizjak
  2016-01-25 13:53 ` Michael Matz
  0 siblings, 1 reply; 15+ messages in thread
From: Uros Bizjak @ 2016-01-25 12:14 UTC (permalink / raw)
  To: gcc-patches; +Cc: Michael Matz, Richard Biener

Hello!

> * system.h (string, algorithm): Include only conditionally.
> (new): Include always under C++.
> * bb-reorder.c (toplevel): Define USES_ALGORITHM.
> * final.c (toplevel): Ditto.
> * ipa-chkp.c (toplevel): Define USES_STRING.
> * genconditions.c (write_header): Make gencondmd.c define
> USES_STRING.
> * mem-stats.h (mem_usage::print_dash_line): Don't use std::string.
>
> * config/aarch64/aarch64.c (toplevel): Define USES_STRING.
> * common/config/aarch64/aarch64-common.c (toplevel): Ditto.

This patch caused bootstrap failure on non-c++11 bootstrap compiler
[1], e.g. CentOS 5.11.

The problem is with std::swap, which was defined in header <algorithm>
until c++11 [2].

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69464
[2] http://en.cppreference.com/w/cpp/algorithm/swap

Uros.

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Speedup configure and build with system.h
@ 2016-01-21 16:57 Michael Matz
  2016-01-21 17:10 ` Richard Biener
  2016-01-22 20:09 ` H.J. Lu
  0 siblings, 2 replies; 15+ messages in thread
From: Michael Matz @ 2016-01-21 16:57 UTC (permalink / raw)
  To: gcc-patches

Hi,

this has bothered me for some time.  The gcc configure with stage1 feels 
like taking forever because some of the decl availability tests (checking 
for C function) include system.h, and that, since a while, unconditionally 
includes <string> and <algorithm> under C++, and we meanwhile use the C++ 
compiler for configure tests (which makes sense).  Now, the difference for 
a debuggable (but not even checking-enabled) cc1plus for a file containing 
just main():

% cat blaeh.cc
#include <stdio.h>
#include <cstring>
#include <utility>
#include <new>
int main() {}
% cc1plus -quiet -ftime-report blaeh.cc
 TOTAL                 :   0.12             0.01             0.14

(This is btw. three times as expensive as with 4.8 headers (i.e. 
precompile with g++-4.8 then compile with the same cc1plus as above, 
taking 0.04 seconds; the STL headers bloat quite much over time)

Well, not quite blazing fast but then adding <string>:

% cc1plus -quiet -ftime-report blaeh-string.cc
 TOTAL                 :   0.60             0.05             0.66

Meeh.  And adding <algorithm> on top:

% cc1plus -quiet -ftime-report blaeh-string-alg.cc
 TOTAL                 :   1.13             0.09             1.23

So, more than a second for checking if some C-only decl is available, just 
because system.h unconditionally includes mostly useless STL headers.

So, how useless exactly?  A whopping single file of cc1 proper needs 
<string>, _two_ files need <algorithm>, and a single target has an unlucky 
interface in its prototypes and also needs <string>.  (One additional 
header lazily uses std::string for no particular reason).  So we pay about 
5 minutes build time per stage (there are ~400 libbackend.a files) for 
more or less nothing.

So, let's include those headers only conditionally; I'm pretty sure it's 
not unreasonable for a source file, if it needs a particular STL facility 
to #define USES_abcheader (like one normally would have to #include 
<abcheader>) before the "system.h" include.

See the patch.  I've grepped for target or language dependencies on other 
STL types, and either they were already including the right header, or 
were covered with the new system.h (i.e. I've built all targets quickly 
for which grepping for 'std::' returned anything).  The genconditions.c 
change is for the benefit of aarch64 as well, and it single function 
aarch64_get_extension_string_for_isa_flags returning a std::string.

What do people think?  Should I pass it through a proper bootstrap and put 
it to trunk?  It's a (developer time) regression, right? ;-)


Ciao,
Michael.
	* system.h (string, algorithm): Include only conditionally.
	(new): Include always under C++.
	* bb-reorder.c (toplevel): Define USES_ALGORITHM.
	* final.c (toplevel): Ditto.
	* ipa-chkp.c (toplevel): Define USES_STRING.
	* genconditions.c (write_header): Make gencondmd.c define
	USES_STRING.
	* mem-stats.h (mem_usage::print_dash_line): Don't use std::string.

	* config/aarch64/aarch64.c (toplevel): Define USES_STRING.
	* common/config/aarch64/aarch64-common.c (toplevel): Ditto.

Index: bb-reorder.c
===================================================================
--- bb-reorder.c	(revision 232675)
+++ bb-reorder.c	(working copy)
@@ -91,6 +91,7 @@
 */
 
 #include "config.h"
+#define USES_ALGORITHM /* stable_sort */
 #include "system.h"
 #include "coretypes.h"
 #include "backend.h"
Index: final.c
===================================================================
--- final.c	(revision 232675)
+++ final.c	(working copy)
@@ -43,6 +43,7 @@ along with GCC; see the file COPYING3.
    function_epilogue.  Those instructions never exist as rtl.  */
 
 #include "config.h"
+#define USES_ALGORITHM /* reverse */
 #include "system.h"
 #include "coretypes.h"
 #include "backend.h"
Index: genconditions.c
===================================================================
--- genconditions.c	(revision 232675)
+++ genconditions.c	(working copy)
@@ -51,6 +51,7 @@ write_header (void)
    machine description file.  */\n\
 \n\
 #include \"bconfig.h\"\n\
+#define USES_STRING\n\
 #include \"system.h\"\n\
 \n\
 /* It is necessary, but not entirely safe, to include the headers below\n\
Index: ipa-chkp.c
===================================================================
--- ipa-chkp.c	(revision 232675)
+++ ipa-chkp.c	(working copy)
@@ -19,6 +19,7 @@ along with GCC; see the file COPYING3.
 <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define USES_STRING
 #include "system.h"
 #include "coretypes.h"
 #include "backend.h"
Index: mem-stats.h
===================================================================
--- mem-stats.h	(revision 232675)
+++ mem-stats.h	(working copy)
@@ -200,7 +200,9 @@ struct mem_usage
   static inline void
   print_dash_line (size_t count = 140)
   {
-    fprintf (stderr, "%s\n", std::string (count, '-').c_str ());
+    while (count--)
+      fputc ('-', stderr);
+    fputc ('\n', stderr);
   }
 
   /* Dump header with NAME.  */
Index: system.h
===================================================================
--- system.h	(revision 232675)
+++ system.h	(working copy)
@@ -198,8 +198,10 @@ extern int fprintf_unlocked (FILE *, con
    the ctype macros through safe-ctype.h */
 
 #ifdef __cplusplus
+#ifdef USES_STRING
 # include <string>
 #endif
+#endif
 
 /* There are an extraordinary number of issues with <ctype.h>.
    The last straw is that it varies with the locale.  Use libiberty's
@@ -215,8 +217,11 @@ extern int errno;
 #endif
 
 #ifdef __cplusplus
+#ifdef USES_ALGORITHM
 # include <algorithm>
+#endif
 # include <cstring>
+# include <new>
 # include <utility>
 #endif
 
Index: config/aarch64/aarch64.c
===================================================================
--- config/aarch64/aarch64.c	(revision 232675)
+++ config/aarch64/aarch64.c	(working copy)
@@ -19,6 +19,7 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define USES_STRING
 #include "system.h"
 #include "coretypes.h"
 #include "backend.h"
Index: common/config/aarch64/aarch64-common.c
===================================================================
--- common/config/aarch64/aarch64-common.c	(revision 232675)
+++ common/config/aarch64/aarch64-common.c	(working copy)
@@ -19,6 +19,7 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
+#define USES_STRING
 #include "system.h"
 #include "coretypes.h"
 #include "tm.h"

^ permalink raw reply	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-01-26 16:35 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-25 12:14 Speedup configure and build with system.h Uros Bizjak
2016-01-25 13:53 ` Michael Matz
2016-01-25 14:02   ` Richard Biener
2016-01-26 10:02   ` Uros Bizjak
2016-01-26 16:35     ` Michael Matz
  -- strict thread matches above, loose matches on Subject: below --
2016-01-21 16:57 Michael Matz
2016-01-21 17:10 ` Richard Biener
2016-01-22 12:02   ` Oleg Endo
2016-01-22 13:49     ` Michael Matz
2016-01-22 16:59   ` Michael Matz
2016-01-22 20:09 ` H.J. Lu
2016-01-22 20:23   ` Jakub Jelinek
2016-01-22 22:15     ` Jakub Jelinek
2016-01-23  7:30       ` Richard Biener
2016-01-25 12:38       ` Michael Matz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).