On Thu, Jan 21, 2016 at 5:57 PM, Michael Matz <m...@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"
