[m-rev.] for review: mmc --make --track-flags
Julien Fischer
juliensf at csse.unimelb.edu.au
Thu Apr 16 04:24:16 AEST 2009
On Thu, 9 Apr 2009, Peter Wang wrote:
> Branches: main
>
> Add mmc --make --track-flags:
>
> --track-flags
> With `--make', keep track of the options used when compiling
> each module. If an option for a module is added or removed,
> `mmc --make' will then know to recompile the module even if the
> timestamp on the file itself has not changed. Warning and
> verbosity options are not tracked.
>
> The motivation was to be able to set the execution trace level on selected
> modules by editing Mercury.options, without needing to manually touch the
> affected files afterwards to force recompilation. It's probably useful for
> anyone that's worried about consistent builds.
>
> The implementation works by recording a hash of the option table which is the
> product of the options set for each particular module in .track_flags files,
> not a hash of the actual options passed by the user, which might be reordered
> or redundant, etc. Hashes are used as they are much quicker to compare and
> take less space (the full options tables are quite big).
Good idea.
> compiler/options.m:
> Add the new option.
>
> Add a predicate to return options that shouldn't be tracked
> (e.g. adding -v shouldn't cause everything to recompile).
>
> compiler/make.m:
> When --track-flags is enabled, write `.tracks_flags' files which need
> to be changed.
>
> compiler/make.dependencies.m:
> When --track-flags is enabled, make compiled code files depend on the
> `.tracks_flags' files.
>
> compiler/make.program_target.m:
> Delete .tracks_flags files on realclean.
s/tracks_flags/track_flags/ in a couple of spots there.
> compiler/make.module_target.m:
> compiler/make.util.m:
> Conform to changes above.
>
> compiler/libs.m:
> compiler/md4.m:
> Add an implementation of the MD4 digest function to libs.
>
> doc/user_guide.texi:
> NEWS:
> Document the new option.
>
> Recommend `mmc --make' instead of `mmake' in the user guide.
>
> library/term_io.m:
> Avoid unnecessary memory allocation in should_atom_be_quoted.
...
> diff --git compiler/make.m compiler/make.m
> index f017019..73fa2f7 100644
> --- compiler/make.m
> +++ compiler/make.m
> @@ -63,7 +63,10 @@
> :- import_module backend_libs.compile_target_code.
> :- import_module hlds.
> :- import_module libs.
> +:- import_module libs.compiler_util.
> :- import_module libs.globals.
> +:- import_module libs.handle_options.
> +:- import_module libs.md4.
> :- import_module libs.options.
> :- import_module libs.timestamp.
> :- import_module make.dependencies.
> @@ -76,9 +79,11 @@
> :- import_module top_level. % XXX unwanted dependency
> :- import_module top_level.mercury_compile. % XXX unwanted dependency
>
> +:- import_module assoc_list.
> :- import_module bool.
> :- import_module dir.
> :- import_module int.
> +:- import_module getopt_io.
> :- import_module map.
> :- import_module maybe.
> :- import_module pair.
> @@ -211,6 +216,7 @@
> ; module_target_unqualified_short_interface
> ; module_target_intermodule_interface
> ; module_target_analysis_registry
> + ; module_target_track_flags
> ; module_target_c_header(c_header_type)
> ; module_target_c_code
> ; module_target_il_code
> @@ -381,18 +387,32 @@ make_process_args(Variables, OptionArgs, Targets0, !IO) :-
>
> make_target(Target, Success, !Info, !IO) :-
> Target = ModuleName - TargetType,
> + globals.io_lookup_bool_option(track_flags, TrackFlags, !IO),
> (
> - TargetType = module_target(ModuleTargetType),
> - TargetFile = target_file(ModuleName, ModuleTargetType),
> - make_module_target(dep_target(TargetFile), Success,
> - !Info, !IO)
> + TrackFlags = no,
> + TrackFlagsSuccess = yes
> ;
> - TargetType = linked_target(ProgramTargetType),
> - LinkedTargetFile = linked_target_file(ModuleName, ProgramTargetType),
> - make_linked_target(LinkedTargetFile, Success, !Info, !IO)
> + TrackFlags = yes,
> + make_track_flags_files(ModuleName, TrackFlagsSuccess, !Info, !IO)
> + ),
> + (
> + TrackFlagsSuccess = yes,
> + (
> + TargetType = module_target(ModuleTargetType),
> + TargetFile = target_file(ModuleName, ModuleTargetType),
> + make_module_target(dep_target(TargetFile), Success, !Info, !IO)
> + ;
> + TargetType = linked_target(ProgramTargetType),
> + LinkedTargetFile = linked_target_file(ModuleName,
> + ProgramTargetType),
> + make_linked_target(LinkedTargetFile, Success, !Info, !IO)
> + ;
> + TargetType = misc_target(MiscTargetType),
> + make_misc_target(ModuleName - MiscTargetType, Success, !Info, !IO)
> + )
> ;
> - TargetType = misc_target(MiscTargetType),
> - make_misc_target(ModuleName - MiscTargetType, Success, !Info, !IO)
> + TrackFlagsSuccess = no,
> + Success = no
> ).
>
> %-----------------------------------------------------------------------------%
> @@ -515,5 +535,164 @@ search_backwards_for_dot(String, Index, DotIndex) :-
> ).
>
> %-----------------------------------------------------------------------------%
> +
> +:- type last_hash
> + ---> last_hash(
> + lh_options :: list(string),
> + lh_hash :: string
> + ).
> +
> + % Generate the .track_flags files for local modules reachable from the
> + % target module. The files contain hashes of the options which are set for
> + % that particular module (deliberately ignoring some options), and are only
> + % updated if they have changed since the last --make run. We use hashes as
> + % the full option tables are quite large.
> + %
> +:- pred make_track_flags_files(module_name::in, bool::out,
> + make_info::in, make_info::out, io::di, io::uo) is det.
> +
> +make_track_flags_files(ModuleName, Success, !Info, !IO) :-
> + find_reachable_local_modules(ModuleName, Success0, Modules, !Info, !IO),
> + (
> + Success0 = yes,
> + KeepGoing = no,
> + DummyLashHash = last_hash([], ""),
> + foldl3_maybe_stop_at_error(KeepGoing, make_track_flags_files_2,
> + set.to_sorted_list(Modules), Success, DummyLashHash, _LastHash,
> + !Info, !IO)
> + ;
> + Success0 = no,
> + Success = no
> + ).
> +
> +:- pred make_track_flags_files_2(module_name::in, bool::out,
> + last_hash::in, last_hash::out, make_info::in, make_info::out,
> + io::di, io::uo) is det.
> +
> +make_track_flags_files_2(ModuleName, Success, !LastHash, !Info, !IO) :-
> + lookup_mmc_module_options(!.Info ^ options_variables, ModuleName,
> + OptionsResult, !IO),
> + (
> + OptionsResult = yes(ModuleOptionArgs),
> + OptionArgs = !.Info ^ option_args,
> + AllOptionArgs = list.condense([ModuleOptionArgs, OptionArgs]),
> +
> + % The set of options from one module to the next is usually identical,
> + % so we can easily avoid running handle_options and stringifying and
> + % hashing the option table, all of which can contribute to an annoying
> + % delay when mmc --make starts.
> + ( !.LastHash = last_hash(AllOptionArgs, HashPrime) ->
> + Hash = HashPrime
> + ;
> + option_table_hash(AllOptionArgs, Hash, !IO),
> + !:LastHash = last_hash(AllOptionArgs, Hash)
> + ),
> +
> + module_name_to_file_name(ModuleName, ".track_flags", do_create_dirs,
> + HashFileName, !IO),
> + compare_hash_file(HashFileName, Hash, Same, !IO),
> + ( Same = yes ->
> + Success = yes
> + ;
> + write_hash_file(HashFileName, Hash, Success, !IO)
> + )
That should be a switch.
> + ;
> + OptionsResult = no,
> + Success = no
> + ).
> +
> +:- pred option_table_hash(list(string)::in, string::out,
> + io::di, io::uo) is det.
> +
> +option_table_hash(AllOptionArgs, Hash, !IO) :-
> + globals.io_get_globals(Globals, !IO),
> + handle_options(AllOptionArgs, OptionsErrors, _, _, _, !IO),
> + (
> + OptionsErrors = []
> + ;
> + OptionsErrors = [_ | _],
> + unexpected($file, $pred ++ ": " ++
> + "handle_options returned with errors")
> + ),
> + globals.io_get_globals(UpdatedGlobals, !IO),
> + globals.get_options(UpdatedGlobals, OptionTable),
> + map.to_sorted_assoc_list(OptionTable, OptionList),
> + inconsequential_options(InconsequentialOptions),
> + list.filter(is_consequential_option(InconsequentialOptions),
> + OptionList, ConsequentialOptionList),
> + Hash = md4sum(string(ConsequentialOptionList)),
> + globals.io_set_globals(Globals, !IO).
> +
> +:- pred is_consequential_option(set(option)::in,
> + pair(option, option_data)::in) is semidet.
> +
> +is_consequential_option(InconsequentialOptions, Option - _) :-
> + not set.contains(InconsequentialOptions, Option).
> +
> +:- pred compare_hash_file(string::in, string::in, bool::out,
> + io::di, io::uo) is det.
> +
> +compare_hash_file(FileName, Hash, Same, !IO) :-
> + io.open_input(FileName, OpenResult, !IO),
> + (
> + OpenResult = ok(Stream),
> + io.read_line_as_string(Stream, ReadResult, !IO),
> + (
> + ReadResult = ok(Line),
> + ( Line = Hash ->
> + Same = yes
> + ;
> + Same = no
> + )
> + ;
> + ReadResult = eof,
> + Same = no
> + ;
> + ReadResult = error(_),
> + Same = no
> + ),
> + io.close_input(Stream, !IO)
> + ;
> + OpenResult = error(_),
> + % Probably missing file.
> + Same = no
> + ),
> + globals.io_lookup_bool_option(verbose, Verbose, !IO),
> + (
> + Verbose = yes,
> + io.write_string("% ", !IO),
> + io.write_string(FileName, !IO),
> + (
> + Same = yes,
> + io.write_string(" does not need updating.\n", !IO)
> + ;
> + Same = no,
> + io.write_string(" will be UPDATED.\n", !IO)
> + )
> + ;
> + Verbose = no
> + ).
> +
> +:- pred write_hash_file(string::in, string::in, bool::out, io::di, io::uo)
> + is det.
> +
> +write_hash_file(FileName, Hash, Success, !IO) :-
> + io.open_output(FileName, OpenResult, !IO),
> + (
> + OpenResult = ok(Stream),
> + io.write_string(Stream, Hash, !IO),
> + io.close_output(Stream, !IO),
> + Success = yes
> + ;
> + OpenResult = error(Error),
> + io.write_string("Error creating `", !IO),
> + io.write_string(FileName, !IO),
> + io.write_string("': ", !IO),
> + io.write_string(io.error_message(Error), !IO),
> + io.nl(!IO),
> + Success = no
> + ).
> +
> +%-----------------------------------------------------------------------------%
> :- end_module make.
> %-----------------------------------------------------------------------------%
> + if (md->tail_len) {
> + int len = 64 - md->tail_len;
> + if (len > n) {
> + len = n;
> + }
> + memcpy(md->tail+md->tail_len, in, len);
s/memcpy/MR_memcpy/ there are below.
Likewise with memset.
> + md->tail_len += len;
> + n -= len;
> + in += len;
> + if (md->tail_len == 64) {
> + copy64(M, md->tail);
> + mdfour64(md, M);
> + md->totalN += 64;
> + md->tail_len = 0;
> + }
> + }
> +
> + while (n >= 64) {
> + copy64(M, in);
> + mdfour64(md, M);
> + in += 64;
> + n -= 64;
> + md->totalN += 64;
> + }
> +
> + if (n) {
> + memcpy(md->tail, in, n);
> + md->tail_len = n;
> + }
> +}
> +
> +static void mdfour_tail(struct mdfour *m, const unsigned char *in, int n)
> +{
> + unsigned char buf[128];
> + MR_uint_least32_t M[16];
> + MR_uint_least32_t b;
> +
> + m->totalN += n;
> +
> + b = m->totalN * 8;
> +
> + memset(buf, 0, 128);
> + if (n) {
> + memcpy(buf, in, n);
> + }
> + buf[n] = 0x80;
> +
> + if (n <= 55) {
> + copy4(buf+56, b);
> + copy64(M, buf);
> + mdfour64(m, M);
> + } else {
> + copy4(buf+120, b);
> + copy64(M, buf);
> + mdfour64(m, M);
> + copy64(M, buf+64);
> + mdfour64(m, M);
> + }
> +}
> +
> +static void mdfour_result(const struct mdfour *m, unsigned char *out)
> +{
> + copy4(out, m->A);
> + copy4(out+4, m->B);
> + copy4(out+8, m->C);
> + copy4(out+12, m->D);
> +}
> +
> +").
> +
> +:- pragma foreign_proc("C",
> + md4sum(In::in) = (Digest::out),
> + [will_not_call_mercury, promise_pure, thread_safe, may_not_duplicate],
> +"
> + const char hex[16] = ""0123456789abcdef"";
> + struct mdfour md;
> + unsigned char sum[16];
> + char hexbuf[sizeof(sum) * 2 + 1];
> + char *p;
> + int i;
> +
> + mdfour_begin(&md);
> + mdfour_update(&md, (const unsigned char *)In, strlen(In));
> + mdfour_update(&md, NULL, 0);
> + mdfour_result(&md, sum);
> +
> + /* Convert to hexadecimal string representation. */
> + p = hexbuf;
> + for (i = 0; i < sizeof(sum); i++) {
> + *p++ = hex[(sum[i] & 0xf0) >> 4];
> + *p++ = hex[(sum[i] & 0x0f)];
> + }
> + *p = '\\0';
> +
> + MR_make_aligned_string_copy(Digest, hexbuf);
> +").
> +
> +%-----------------------------------------------------------------------------%
> +
> + % Exercise for the reader.
> + %
Implementation for non-C backends surely ;-)
> +md4sum(_) = _ :-
> + sorry($file, $pred).
...
> diff --git compiler/options.m compiler/options.m
> index a13ff28..d7ea5c5 100644
> --- compiler/options.m
> +++ compiler/options.m
> @@ -24,6 +24,7 @@
> :- import_module char.
> :- import_module getopt_io.
> :- import_module io.
> +:- import_module set.
>
> %-----------------------------------------------------------------------------%
>
> @@ -47,6 +48,12 @@
> :- pred special_handler(option::in, special_data::in, option_table::in,
> maybe_option_table::out) is semidet.
>
> + % Return the set of options which are inconsequential as far as the
> + % `--track-flags' option is concerned. That is, adding or removing such
> + % an option to a module should not force the module to be recompiled.
> + %
> +:- pred inconsequential_options(set(option)::out) is det.
> +
> :- pred options_help(io::di, io::uo) is det.
>
> :- type option_table == option_table(option).
> @@ -879,6 +886,7 @@
> ; keep_going
> ; rebuild
> ; jobs
> + ; track_flags
> ; invoked_by_mmc_make
> ; extra_init_command
> ; pre_link_command
> @@ -944,6 +952,7 @@
> :- import_module libs.compiler_util.
> :- import_module libs.handle_options.
>
> +:- import_module assoc_list.
> :- import_module bool.
> :- import_module dir.
> :- import_module int.
> @@ -1714,6 +1723,7 @@ option_defaults_2(build_system_option, [
> keep_going - bool(no),
> rebuild - bool(no),
> jobs - int(1),
> + track_flags - bool(no),
> invoked_by_mmc_make - bool(no),
> pre_link_command - maybe_string(no),
> extra_init_command - maybe_string(no),
> @@ -2603,6 +2613,8 @@ long_option("make", make).
> long_option("keep-going", keep_going).
> long_option("rebuild", rebuild).
> long_option("jobs", jobs).
> +long_option("track-flags", track_flags).
> +long_option("track-options", track_flags).
> long_option("invoked-by-mmc-make", invoked_by_mmc_make).
> long_option("pre-link-command", pre_link_command).
> long_option("extra-init-command", extra_init_command).
> @@ -3171,6 +3183,18 @@ quote_char_unix('$').
>
> %-----------------------------------------------------------------------------%
>
> +inconsequential_options(InconsequentialOptions) :-
> + option_defaults_2(warning_option, WarningOptions),
> + option_defaults_2(verbosity_option, VerbosityOptions),
> + option_defaults_2(internal_use_option, InternalUseOptions),
> + assoc_list.keys(WarningOptions, WarningKeys),
> + assoc_list.keys(VerbosityOptions, VerbosityKeys),
> + assoc_list.keys(InternalUseOptions, InternalUseKeys),
> + Keys = WarningKeys ++ VerbosityKeys ++ InternalUseKeys,
> + InconsequentialOptions = set.from_list(Keys).
Is this predicate worth memoing?
Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to: mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions: mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------
More information about the reviews
mailing list