Re: [ccache] [PATCH] Support for Clang precompiled headers
On 13 July 2012 11:48, Lubos Lunak wrote: > Right. I guess that was an oversight when I moved the "Multiple precompiled > headers used" error to one place. Fixed version attached. Thanks, applied. -- Joel ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
Re: [ccache] [PATCH] Support for Clang precompiled headers
On Friday 13 of July 2012, Lubos Lunak wrote: > If somebody would feel like playing with this, note that Clang 3.2 will > have -E -Wp,-rewrite-includes, Make that -E -frewrite-includes, looks like upstream has changed that. > which only processes #include directives and > nothing more (I wrote the feature to solve a similar problem when using > Clang with Icecream for distributed builds, and I assume this is the only > thing ccache in fact wants from -E too). So using this in ccache for Clang > should presumably reduce this overhead. -- Lubos Lunak l.lu...@suse.cz ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
Re: [ccache] [PATCH] Support for Clang precompiled headers
On Thursday 12 of July 2012, Joel Rosdahl wrote: > On 6 July 2012 19:09, Lubos Lunak wrote: > > the attached patches modify ccache to also support Clang PCH in addition > > to GCC PCH. > > Thanks! > > Regarding 0002-hash-clang-s-.pch-file-explicitly.patch: Why is the third > assignment of pch_file done outside the "if (stat(pchpath, &st) == 0)" > block? This means that pch_file and included_pch_file can be set even when > there is no .pch file, so remember_include_file will fail to stat the file > and then turn off the direct mode. The test suite also fails after the > patch, but passes if the pch_file assignment is moved into the block. Right. I guess that was an oversight when I moved the "Multiple precompiled headers used" error to one place. Fixed version attached. > The patches look good otherwise. > > > PS: Since I've noticed in the archives the recent mail about issues with > > ccache and warnings about unused arguments: The proper way to use ccache > > with Clang is to set CCACHE_CPP2, which not only avoids these warnings, > > but in general Clang works suboptimally if passed preprocessed output > > (warning/error messages quoting sources are affected, some warnings are > > not supressed in headers). > > Do you have any idea about the performance impact of using CCACHE_CPP2 for > clang? Debug build of LibreOffice, warm disk caches, empty ccache, make -j4 in xmloff module: CCACHE_CPP2=1: 494.64user 34.99system 2:15.60elapsed 390%CPU (0avgtext+0avgdata 1309824maxresident)k 0inputs+3752560outputs (507major+13592550minor)pagefaults 0swaps not set: 457.17user 28.94system 2:06.15elapsed 385%CPU (0avgtext+0avgdata 1309312maxresident)k 0inputs+3758600outputs (444major+12098612minor)pagefaults 0swaps So in this case CCACHE_CPP2 adds about 8% overhead. It might make a difference for somebody, but I myself would prefer to get better error messages from Clang (and I actually use ccache only for a buildbot, so I don't find this worth the effort). If somebody would feel like playing with this, note that Clang 3.2 will have -E -Wp,-rewrite-includes, which only processes #include directives and nothing more (I wrote the feature to solve a similar problem when using Clang with Icecream for distributed builds, and I assume this is the only thing ccache in fact wants from -E too). So using this in ccache for Clang should presumably reduce this overhead. -- Lubos Lunak l.lu...@suse.cz From d897f0388b35ca0b8e333e7e8edbd57a0c4ba3e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= Date: Fri, 6 Jul 2012 18:45:05 +0200 Subject: [PATCH 2/2] hash clang's .pch file explicitly It does not appear anywhere in the preprocessed output, so process_preprocessed_file() would miss it. --- ccache.c | 31 +++ 1 files changed, 31 insertions(+), 0 deletions(-) diff --git a/ccache.c b/ccache.c index 12b62a4..926d6b2 100644 --- a/ccache.c +++ b/ccache.c @@ -171,6 +171,11 @@ static bool profile_generate = false; */ static bool using_precompiled_header = false; +/* + * The .gch/.pch file used for compilation. + */ +static char *included_pch_file = NULL; + /* How long (in microseconds) to wait before breaking a stale lock. */ unsigned lock_staleness_limit = 200; @@ -532,6 +537,16 @@ process_preprocessed_file(struct mdfour *hash, const char *path) hash_buffer(hash, p, (end - p)); free(data); + + /* Explicitly check the .gch/.pch file, Clang does not include any mention of it + in the preprocessed output. */ + if (included_pch_file) { + char *path = x_strdup(included_pch_file); + path = make_relative_path(path); + hash_string(hash, path); + remember_include_file(path, strlen(included_pch_file), hash); + } + return true; } @@ -1680,6 +1695,7 @@ cc_process_args(struct args *orig_args, struct args **preprocessor_args, */ if (compopt_takes_path(argv[i])) { char *relpath; + char *pch_file = NULL; if (i == argc-1) { cc_log("Missing argument to %s", argv[i]); stats_update(STATS_ARGS); @@ -1696,23 +1712,37 @@ cc_process_args(struct args *orig_args, struct args **preprocessor_args, if (stat(argv[i+1], &st) == 0) { cc_log("Detected use of precompiled header: %s", argv[i+1]); found_pch = true; + pch_file = x_strdup(argv[i+1]); } } else { char* gchpath = format("%s.gch", argv[i+1]); if (stat(gchpath, &st) == 0) { cc_log("Detected use of precompiled header: %s", gchpath); found_pch = true; + pch_file = x_strdup(gchpath); } else { char* pchpath = format("%s.pch", argv[i+1]); if (stat(pchpath, &st) == 0) { cc_log("Detected use of precompiled header: %s", pchpath); found_pch = true; + pch_file = x_strdup(pchpath); } free(pchpath); } free(gchpath); } + if (pch_file) { +if (included_pch_file) { + cc_log("Multiple precompiled headers used: %s and %s\n", + included_pch_file, pch_file); +
Re: [ccache] [PATCH] Support for Clang precompiled headers
On 6 July 2012 19:09, Lubos Lunak wrote: > the attached patches modify ccache to also support Clang PCH in addition to > GCC PCH. Thanks! Regarding 0002-hash-clang-s-.pch-file-explicitly.patch: Why is the third assignment of pch_file done outside the "if (stat(pchpath, &st) == 0)" block? This means that pch_file and included_pch_file can be set even when there is no .pch file, so remember_include_file will fail to stat the file and then turn off the direct mode. The test suite also fails after the patch, but passes if the pch_file assignment is moved into the block. The patches look good otherwise. > PS: Since I've noticed in the archives the recent mail about issues with > ccache and warnings about unused arguments: The proper way to use ccache with > Clang is to set CCACHE_CPP2, which not only avoids these warnings, but in > general Clang works suboptimally if passed preprocessed output (warning/error > messages quoting sources are affected, some warnings are not supressed in > headers). Do you have any idea about the performance impact of using CCACHE_CPP2 for clang? -- Joel ___ ccache mailing list ccache@lists.samba.org https://lists.samba.org/mailman/listinfo/ccache
[ccache] [PATCH] Support for Clang precompiled headers
Hello, the attached patches modify ccache to also support Clang PCH in addition to GCC PCH. The changes are: - Clang uses .pch as the extension, not .gch - the .pch file can be directly passed using -include-pch, instead of using -include and trying to find the .pch file - the use of a .pch is not reflected in the resulting .ii , so it needs to be explicitly added to the hash for checking if used files have changed - I also believe that path_len passed to remember_include_file() is incorrect, as make_relative_path() may change the string PS: Since I've noticed in the archives the recent mail about issues with ccache and warnings about unused arguments: The proper way to use ccache with Clang is to set CCACHE_CPP2, which not only avoids these warnings, but in general Clang works suboptimally if passed preprocessed output (warning/error messages quoting sources are affected, some warnings are not supressed in headers). -- Lubos Lunak l.lu...@suse.cz From 768ffacb434d116759fc73cf0d2723aba6b5af4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= Date: Fri, 6 Jul 2012 18:09:36 +0200 Subject: [PATCH 1/3] support for precompiled headers with clang Support the clang-specific -include-pch option, which references the PCH file itself, and support the .pch extension when using the gcc -include way. --- MANUAL.txt |2 ++ ccache.c | 29 + compopt.c |1 + 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/MANUAL.txt b/MANUAL.txt index 4be33ae..478d36b 100644 --- a/MANUAL.txt +++ b/MANUAL.txt @@ -629,6 +629,8 @@ things to make it work properly: -- ** use the *-include* compiler option to include the precompiled header (i.e., don't use *#include* in the source code to include the header); or +** (Clang compiler) use the *-include-pch* compiler option to include + the PCH file generated from the precompiled header; or ** add the *-fpch-preprocess* compiler option when compiling. If you don't do this, either the non-precompiled version of the header file diff --git a/ccache.c b/ccache.c index 8b50c36..12b62a4 100644 --- a/ccache.c +++ b/ccache.c @@ -167,7 +167,7 @@ static bool profile_use = false; static bool profile_generate = false; /* - * Whether we are using a precompiled header (either via -include or #include). + * Whether we are using a precompiled header (either via -include, #include or clang's -include-pch). */ static bool using_precompiled_header = false; @@ -1355,7 +1355,7 @@ find_compiler(char** argv) bool is_precompiled_header(const char *path) { - return str_eq(get_extension(path), ".gch"); + return str_eq(get_extension(path), ".gch") || str_eq(get_extension(path), ".pch"); } /* @@ -1680,7 +1680,6 @@ cc_process_args(struct args *orig_args, struct args **preprocessor_args, */ if (compopt_takes_path(argv[i])) { char *relpath; - char *pchpath; if (i == argc-1) { cc_log("Missing argument to %s", argv[i]); stats_update(STATS_ARGS); @@ -1693,13 +1692,27 @@ cc_process_args(struct args *orig_args, struct args **preprocessor_args, args_add(stripped_args, relpath); /* Try to be smart about detecting precompiled headers */ - pchpath = format("%s.gch", argv[i+1]); - if (stat(pchpath, &st) == 0) { -cc_log("Detected use of precompiled header: %s", pchpath); -found_pch = true; + if (str_eq(argv[i], "-include-pch")) { +if (stat(argv[i+1], &st) == 0) { + cc_log("Detected use of precompiled header: %s", argv[i+1]); + found_pch = true; +} + } else { +char* gchpath = format("%s.gch", argv[i+1]); +if (stat(gchpath, &st) == 0) { + cc_log("Detected use of precompiled header: %s", gchpath); + found_pch = true; +} else { + char* pchpath = format("%s.pch", argv[i+1]); + if (stat(pchpath, &st) == 0) { + cc_log("Detected use of precompiled header: %s", pchpath); + found_pch = true; + } + free(pchpath); +} +free(gchpath); } - free(pchpath); free(relpath); i++; continue; diff --git a/compopt.c b/compopt.c index 77b57f5..908302e 100644 --- a/compopt.c +++ b/compopt.c @@ -61,6 +61,7 @@ static const struct compopt compopts[] = { {"-imacros",AFFECTS_CPP | TAKES_ARG | TAKES_PATH}, {"-imultilib", AFFECTS_CPP | TAKES_ARG | TAKES_PATH}, {"-include",AFFECTS_CPP | TAKES_ARG | TAKES_PATH}, + {"-include-pch",AFFECTS_CPP | TAKES_ARG | TAKES_PATH}, {"-install_name", TAKES_ARG}, /* Darwin linker option */ {"-iprefix",AFFECTS_CPP | TAKES_ARG | TAKES_PATH}, {"-iquote", AFFECTS_CPP | TAKES_ARG | TAKES_PATH}, -- 1.7.7 From bbfc0ed55a6f730023d068773fc9fece16b94c74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= Date: Fri, 6 Jul 2012 18:46:25 +0200 Subject: [PATCH 3/3] do not pass incorrect length to remember_include_file() --- ccache.c |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) diff --gi