Re: Speedup configure and build with system.h

[Richard Biener <richard dot guenther at gmail dot com>]

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"