| Message ID | alpine.LSU.2.20.1601211731160.25099@wotan.suse.de |
|---|---|
| State | New |
| Headers | show |
On Thu, Jan 21, 2016 at 5:57 PM, Michael Matz <matz@suse.de> wrote: > 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? ;-) Ok. Thanks, Richard. I'm inclined to say #define INCLUDE_ALGORITHM is a better name, but just bike-shedding... and please convert the (bogus) ISL way of achieving a similar thing. I'm also inclined to say that we should remove <string> usage. Not sure about algorithm, but I'd say it's the same. Richard. > > 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"
On Thu, 2016-01-21 at 18:10 +0100, Richard Biener wrote: > On Thu, Jan 21, 2016 at 5:57 PM, Michael Matz <matz@suse.de> wrote: > > 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? ;-) > > Ok. > > Thanks, > Richard. > > I'm inclined to say #define INCLUDE_ALGORITHM is a better name, > but just bike-shedding... and please convert the (bogus) ISL way of > achieving a similar thing. > > I'm also inclined to say that we should remove <string> usage. Not > sure about algorithm, but I'd say it's the same. > <string> and <algorithm> have been put into system.h because there have been problems with malloc poisoning and C++ stdlib implementation other than libstdc++, which sometimes pull other headers which then cause trouble. The fix for this set of errors was to include some of the stdlib headers in system.h before anything else. Cheers, Oleg
Hi, On Fri, 22 Jan 2016, Oleg Endo wrote: > <string> and <algorithm> have been put into system.h because there have > been problems with malloc poisoning and C++ stdlib implementation other > than libstdc++, which sometimes pull other headers which then cause > trouble. The fix for this set of errors was to include some of the > stdlib headers in system.h before anything else. Richard meant to remove use of std::string in the compiler, at which point it's not necessary to include <string> anywhere, in system.h or whereever. That's a separate discussion, though (I would agree with it). Ciao, Michael.
On Thu, Jan 21, 2016 at 8:57 AM, Michael Matz <matz@suse.de> wrote: > 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. > This may have caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69434 H.J.
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"
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.