Message ID | 20230125210636.2960049-6-ben.boeckel@kitware.com |
---|---|
State | New |
Headers | show |
Series | P1689R5 support | expand |
On 1/25/23 16:06, Ben Boeckel wrote: > It affects the build, and if used as a static file, can reliably be > tracked using the `-MF` mechanism. Hmm, this seems a bit like making all .o depend on the Makefile; it shouldn't be necessary to rebuild all TUs that use modules when we add another module to the mapper file. What is your expected use case for this dependency? > gcc/cp/: > > * mapper-client.cc, mapper-client.h (open_module_client): Accept > dependency tracking and track module mapper files as > dependencies. > * module.cc (make_mapper, get_mapper): Pass the dependency > tracking class down. > > Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com> > --- > gcc/cp/mapper-client.cc | 4 ++++ > gcc/cp/mapper-client.h | 1 + > gcc/cp/module.cc | 18 +++++++++--------- > 3 files changed, 14 insertions(+), 9 deletions(-) > > diff --git a/gcc/cp/mapper-client.cc b/gcc/cp/mapper-client.cc > index 39e80df2d25..0ce5679d659 100644 > --- a/gcc/cp/mapper-client.cc > +++ b/gcc/cp/mapper-client.cc > @@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see > #include "diagnostic-core.h" > #include "mapper-client.h" > #include "intl.h" > +#include "mkdeps.h" > > #include "../../c++tools/resolver.h" > > @@ -132,6 +133,7 @@ spawn_mapper_program (char const **errmsg, std::string &name, > > module_client * > module_client::open_module_client (location_t loc, const char *o, > + class mkdeps *deps, > void (*set_repo) (const char *), > char const *full_program_name) > { > @@ -285,6 +287,8 @@ module_client::open_module_client (location_t loc, const char *o, > errmsg = "opening"; > else > { > + /* Add the mapper file to the dependency tracking. */ > + deps_add_dep (deps, name.c_str ()); > if (int l = r->read_tuple_file (fd, ident, false)) > { > if (l > 0) > diff --git a/gcc/cp/mapper-client.h b/gcc/cp/mapper-client.h > index b32723ce296..a3b0b8adc51 100644 > --- a/gcc/cp/mapper-client.h > +++ b/gcc/cp/mapper-client.h > @@ -55,6 +55,7 @@ public: > > public: > static module_client *open_module_client (location_t loc, const char *option, > + class mkdeps *, > void (*set_repo) (const char *), > char const *); > static void close_module_client (location_t loc, module_client *); > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index dbd1b721616..37066bf072b 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -3969,12 +3969,12 @@ static GTY(()) vec<tree, va_gc> *partial_specializations; > /* Our module mapper (created lazily). */ > module_client *mapper; > > -static module_client *make_mapper (location_t loc); > -inline module_client *get_mapper (location_t loc) > +static module_client *make_mapper (location_t loc, class mkdeps *deps); > +inline module_client *get_mapper (location_t loc, class mkdeps *deps) > { > auto *res = mapper; > if (!res) > - res = make_mapper (loc); > + res = make_mapper (loc, deps); > return res; > } > > @@ -14031,7 +14031,7 @@ get_module (const char *ptr) > /* Create a new mapper connecting to OPTION. */ > > module_client * > -make_mapper (location_t loc) > +make_mapper (location_t loc, class mkdeps *deps) > { > timevar_start (TV_MODULE_MAPPER); > const char *option = module_mapper_name; > @@ -14039,7 +14039,7 @@ make_mapper (location_t loc) > option = getenv ("CXX_MODULE_MAPPER"); > > mapper = module_client::open_module_client > - (loc, option, &set_cmi_repo, > + (loc, option, deps, &set_cmi_repo, > (save_decoded_options[0].opt_index == OPT_SPECIAL_program_name) > && save_decoded_options[0].arg != progname > ? save_decoded_options[0].arg : nullptr); > @@ -19503,7 +19503,7 @@ maybe_translate_include (cpp_reader *reader, line_maps *lmaps, location_t loc, > dump.push (NULL); > > dump () && dump ("Checking include translation '%s'", path); > - auto *mapper = get_mapper (cpp_main_loc (reader)); > + auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader)); > > size_t len = strlen (path); > path = canonicalize_header_name (NULL, loc, true, path, len); > @@ -19619,7 +19619,7 @@ module_begin_main_file (cpp_reader *reader, line_maps *lmaps, > static void > name_pending_imports (cpp_reader *reader) > { > - auto *mapper = get_mapper (cpp_main_loc (reader)); > + auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader)); > > if (!vec_safe_length (pending_imports)) > /* Not doing anything. */ > @@ -20089,7 +20089,7 @@ init_modules (cpp_reader *reader) > > if (!flag_module_lazy) > /* Get the mapper now, if we're not being lazy. */ > - get_mapper (cpp_main_loc (reader)); > + get_mapper (cpp_main_loc (reader), cpp_get_deps (reader)); > > if (!flag_preprocess_only) > { > @@ -20299,7 +20299,7 @@ late_finish_module (cpp_reader *reader, module_processing_cookie *cookie, > > if (!errorcount) > { > - auto *mapper = get_mapper (cpp_main_loc (reader)); > + auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader)); > mapper->ModuleCompiled (state->get_flatname ()); > } > else if (cookie->cmi_name)
On Fri, Jun 23, 2023 at 10:44:11 -0400, Jason Merrill wrote: > On 1/25/23 16:06, Ben Boeckel wrote: > > It affects the build, and if used as a static file, can reliably be > > tracked using the `-MF` mechanism. > > Hmm, this seems a bit like making all .o depend on the Makefile; it Technically this is true: the command line for the TU lives in said Makefile; if I updated it, a new TU would be really nice. This is a long-standing limitation of `make` though. FWIW, `ninja` fixes it by tracking the command line used and CMake's Makefiles generator handles it by storing TU flags in an included file and depending on that file from the TU output. > shouldn't be necessary to rebuild all TUs that use modules when we add > another module to the mapper file. If I change it from: ``` mod.a mod.a.cmi ``` to: ``` mod.a mod.a.replace.cmi ``` I'd expect a recompile. As with anything, this depends on the granularity of the mapper files. A global mapper file is very similar to a global response file and given that we don't have line-change granularity dependency tracking… > What is your expected use case for > this dependency? CMake, at least, uses a per-TU mapper file, so any build system using a similar strategy handling the above case would only affect TUs that actually list `mod.a`. --Ben
diff --git a/gcc/cp/mapper-client.cc b/gcc/cp/mapper-client.cc index 39e80df2d25..0ce5679d659 100644 --- a/gcc/cp/mapper-client.cc +++ b/gcc/cp/mapper-client.cc @@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see #include "diagnostic-core.h" #include "mapper-client.h" #include "intl.h" +#include "mkdeps.h" #include "../../c++tools/resolver.h" @@ -132,6 +133,7 @@ spawn_mapper_program (char const **errmsg, std::string &name, module_client * module_client::open_module_client (location_t loc, const char *o, + class mkdeps *deps, void (*set_repo) (const char *), char const *full_program_name) { @@ -285,6 +287,8 @@ module_client::open_module_client (location_t loc, const char *o, errmsg = "opening"; else { + /* Add the mapper file to the dependency tracking. */ + deps_add_dep (deps, name.c_str ()); if (int l = r->read_tuple_file (fd, ident, false)) { if (l > 0) diff --git a/gcc/cp/mapper-client.h b/gcc/cp/mapper-client.h index b32723ce296..a3b0b8adc51 100644 --- a/gcc/cp/mapper-client.h +++ b/gcc/cp/mapper-client.h @@ -55,6 +55,7 @@ public: public: static module_client *open_module_client (location_t loc, const char *option, + class mkdeps *, void (*set_repo) (const char *), char const *); static void close_module_client (location_t loc, module_client *); diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index dbd1b721616..37066bf072b 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -3969,12 +3969,12 @@ static GTY(()) vec<tree, va_gc> *partial_specializations; /* Our module mapper (created lazily). */ module_client *mapper; -static module_client *make_mapper (location_t loc); -inline module_client *get_mapper (location_t loc) +static module_client *make_mapper (location_t loc, class mkdeps *deps); +inline module_client *get_mapper (location_t loc, class mkdeps *deps) { auto *res = mapper; if (!res) - res = make_mapper (loc); + res = make_mapper (loc, deps); return res; } @@ -14031,7 +14031,7 @@ get_module (const char *ptr) /* Create a new mapper connecting to OPTION. */ module_client * -make_mapper (location_t loc) +make_mapper (location_t loc, class mkdeps *deps) { timevar_start (TV_MODULE_MAPPER); const char *option = module_mapper_name; @@ -14039,7 +14039,7 @@ make_mapper (location_t loc) option = getenv ("CXX_MODULE_MAPPER"); mapper = module_client::open_module_client - (loc, option, &set_cmi_repo, + (loc, option, deps, &set_cmi_repo, (save_decoded_options[0].opt_index == OPT_SPECIAL_program_name) && save_decoded_options[0].arg != progname ? save_decoded_options[0].arg : nullptr); @@ -19503,7 +19503,7 @@ maybe_translate_include (cpp_reader *reader, line_maps *lmaps, location_t loc, dump.push (NULL); dump () && dump ("Checking include translation '%s'", path); - auto *mapper = get_mapper (cpp_main_loc (reader)); + auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader)); size_t len = strlen (path); path = canonicalize_header_name (NULL, loc, true, path, len); @@ -19619,7 +19619,7 @@ module_begin_main_file (cpp_reader *reader, line_maps *lmaps, static void name_pending_imports (cpp_reader *reader) { - auto *mapper = get_mapper (cpp_main_loc (reader)); + auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader)); if (!vec_safe_length (pending_imports)) /* Not doing anything. */ @@ -20089,7 +20089,7 @@ init_modules (cpp_reader *reader) if (!flag_module_lazy) /* Get the mapper now, if we're not being lazy. */ - get_mapper (cpp_main_loc (reader)); + get_mapper (cpp_main_loc (reader), cpp_get_deps (reader)); if (!flag_preprocess_only) { @@ -20299,7 +20299,7 @@ late_finish_module (cpp_reader *reader, module_processing_cookie *cookie, if (!errorcount) { - auto *mapper = get_mapper (cpp_main_loc (reader)); + auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader)); mapper->ModuleCompiled (state->get_flatname ()); } else if (cookie->cmi_name)
It affects the build, and if used as a static file, can reliably be tracked using the `-MF` mechanism. gcc/cp/: * mapper-client.cc, mapper-client.h (open_module_client): Accept dependency tracking and track module mapper files as dependencies. * module.cc (make_mapper, get_mapper): Pass the dependency tracking class down. Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com> --- gcc/cp/mapper-client.cc | 4 ++++ gcc/cp/mapper-client.h | 1 + gcc/cp/module.cc | 18 +++++++++--------- 3 files changed, 14 insertions(+), 9 deletions(-)