[m-rev.] for review: more work on intermodule analysis
Julien Fischer
juliensf at cs.mu.OZ.AU
Thu Feb 2 00:26:00 AEDT 2006
On Wed, 1 Feb 2006, Peter Wang wrote:
> Estimated hours taken: 20
> Branches: main
>
> This patch mainly adds the ability to perform intermodule analysis of
> modules as a separate step from compiling. It makes a start on automatic
s/compiling/code generation/
> reanalysis of modules if their .analysis files become invalid or suboptimal.
>
> analysis/analysis.file.m:
> analysis/analysis.m:
> Add a predicate `read_module_overall_status' to read just the overall
> status of an analysis file.
>
> Fix a bug where analysis results were being discarded (upon reading) if
> the module's overall status was `invalid'. We can't do that as then we
> wouldn't know which results changed after reanalysis.
>
> Don't write out `.analysis' and `.imdg' files which would contain no
> information.
>
> Touch a `FOO.analysis_date' file after module `FOO' is analysed.
> This is needed to indicate the time at which `FOO' was last analysed,
> as `FOO.analysis' can be modified at other times.
>
> Add a mutable boolean variable `debug_analysis' that can be set
> to enable debugging messages for the analysis framework.
>
> compiler/handle_options.m:
> compiler/options.m:
> Add new compiler options `--make-analysis-registry',
> `--debug-intermodule-analysis' and `--max-reanalysis-passes'.
>
> compiler/make.dependencies.m:
> Add `.analysis' files as dependencies of compiled code files if
> `--intermodule-analysis' is used.
>
> compiler/make.m:
> compiler/make.module_target.m:
> compiler/make.program_target.m:
> compiler/make.util.m:
> Add a `FOO.analyse' pseudo-target to `mmc --make'. This produces all
> the `.analysis' files needed to compile the top-level module `FOO',
> reanalysing modules until none of the `.analysis' files are invalid.
> The `--max-reanalysis-passes' option can also be used to force
> `suboptimal' modules to be reanalysed a number of times.
>
> Perform an analysis pass when building executables or libraries to make
> sure that compilation does not proceed with invalid analysis files.
>
> Treat `.analysis' files specially when checking whether they are out of
> date. Analysis files require looking at their contents to determine
> whether they are invalid (in which case they are out of date), or valid
> (in which case we look at the timestamp of the corresponding
> `.analysis_date' file).
>
> compiler/mercury_compile.m:
> Make `.analysis' be written out only when `--make-analysis-registry'
> is used, like `--make-transitive-optimization-interface'.
> `mmc --make' runs the compiler the `--make-analysis-registry'
> option as appropriate.
>
> compiler/exception_analysis.m:
> compiler/trailing_analysis.m:
> compiler/unused_args.m:
> Only record intermodule dependencies and analysis results when
> `--make-analysis-registry' is enabled, not when
> `--intermodule-analysis' is enabled.
>
> doc/user_guide.texi:
> Document `--debug-intermodule-analysis' and `--max-reanalysis-passes'.
>
>
> Index: analysis/analysis.file.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/analysis/analysis.file.m,v
> retrieving revision 1.3
> diff -u -r1.3 analysis.file.m
> --- analysis/analysis.file.m 25 Jan 2006 03:21:03 -0000 1.3
> +++ analysis/analysis.file.m 31 Jan 2006 05:50:53 -0000
> @@ -13,6 +13,10 @@
>
> :- interface.
>
> +:- pred read_module_overall_status(Compiler::in, module_id::in,
> + maybe(analysis_status)::out, io::di, io::uo) is det
> + <= compiler(Compiler).
> +
Add a descriptive comment for that predicate. (I realise that the
rest of the module interface is rather poorly documented.)
> :- pred read_module_analysis_results(analysis_info::in, module_id::in,
> analysis_status::out, module_analysis_map(analysis_result)::out,
> io__state::di, io__state::uo) is det.
...
> +read_module_overall_status(Compiler, ModuleId, MaybeModuleStatus, !IO) :-
> + module_id_to_file_name(Compiler, ModuleId, analysis_registry_suffix,
> + AnalysisFileName, !IO),
> + io__open_input(AnalysisFileName, OpenResult, !IO),
> + (
> + OpenResult = ok(Stream),
> + io__set_input_stream(Stream, OldStream, !IO),
>
> -:- pred read_module_analysis_results_2(Compiler::in, module_id::in,
> - analysis_status::out, module_analysis_map(analysis_result)::out,
> - io::di, io::uo) is det <= compiler(Compiler).
> + promise_only_solution_io(
> + (pred(Status::out, !.IO::di, !:IO::uo) is cc_multi :-
> + try_io((pred(Status0::out, !.IO::di, !:IO::uo) is det :-
> + check_analysis_file_version_number(!IO),
> + read_module_status(Status0, !IO)
> + ), Status, !IO)
> + ), Result, !IO),
> + (
> + Result = succeeded(ModuleStatus),
> + MaybeModuleStatus = yes(ModuleStatus)
> + ;
> + Result = failed,
> + MaybeModuleStatus = no
> + ;
> + Result = exception(_),
> + % XXX Report error.
> + MaybeModuleStatus = no
Yes, you should ;-)
> + ),
> + io__set_input_stream(OldStream, _, !IO),
> + io__close_input(Stream, !IO)
> + ;
> + OpenResult = error(_),
> + MaybeModuleStatus = no
> + ).
>
...
> Index: analysis/analysis.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/analysis/analysis.m,v
> retrieving revision 1.3
> diff -u -r1.3 analysis.m
> --- analysis/analysis.m 25 Jan 2006 03:21:03 -0000 1.3
> +++ analysis/analysis.m 31 Jan 2006 05:52:08 -0000
> @@ -199,8 +199,15 @@
> % requests and results for the current compilation to the
> % analysis files.
> %
> -:- pred write_analysis_files(module_id::in, set(module_id)::in,
> - analysis_info::in, analysis_info::out, io::di, io::uo) is det.
> +:- pred write_analysis_files(Compiler::in, module_id::in, set(module_id)::in,
> + analysis_info::in, analysis_info::out, io::di, io::uo) is det
> + <= compiler(Compiler).
> +
> +:- pred read_module_overall_status(Compiler::in, module_id::in,
> + maybe(analysis_status)::out, io::di, io::uo) is det
> + <= compiler(Compiler).
> +
Add a descriptive comment for this predicate.
...
> Index: compiler/make.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/make.m,v
> retrieving revision 1.32
> diff -u -r1.32 make.m
> --- compiler/make.m 25 Jan 2006 03:27:35 -0000 1.32
> +++ compiler/make.m 31 Jan 2006 00:19:05 -0000
> @@ -144,8 +144,10 @@
> importing_module :: maybe(module_name),
>
> % Targets specified on the command line.
> - command_line_targets :: set(pair(module_name, target_type))
> + command_line_targets :: set(pair(module_name, target_type)),
>
> + % Number of repeated passes of analysis allowed.
> + reanalysis_passes :: int
I suggest naming the field max_analysis_passes or something similar?
It could be confusing as to whether this is supposed to be counting
the number of passes currently performe or whether it is a limit on
the number of passes.
...
> +:- pred force_reanalysis_of_suboptimal_module(module_name::in, bool::out,
> + make_info::in, io::di, io::uo) is det.
> +
> +force_reanalysis_of_suboptimal_module(ModuleName, ForceReanalysis, Info,
> + !IO) :-
> + (if Info ^ reanalysis_passes > 0 then
> + ModuleId = module_name_to_module_id(ModuleName),
> + analysis__read_module_overall_status(mmc, ModuleId,
> + MaybeAnalysisStatus, !IO),
> + ( MaybeAnalysisStatus = yes(suboptimal) ->
> + ForceReanalysis = yes
> + ;
> + ForceReanalysis = no
> + )
I suggest rewriting that as a switch.
...
> @@ -63,9 +66,29 @@
> ->
> Succeeded = yes
> ;
> - build_with_module_options(MainModuleName, ExtraOptions,
> - make_linked_target_2(MainModuleName - FileType),
> - Succeeded, !Info, !IO)
> + % When using `--intermodule-analysis', perform an analysis pass first.
> + % The analysis of one module may invalidate the results of a module we
> + % analysed earlier, so this step must be carried out until all the
> + % `.analysis' are in a valid state before we can continue.
all the `.analysis' files ...
...
> +:- pred build_analyses(module_name::in, list(module_name)::in,
> + bool::in, bool::out, make_info::in, make_info::out, io::di, io::uo)
> + is det.
I suggest calling this build_analysis_files.
> +build_analyses(MainModuleName, AllModules, Succeeded0, Succeeded,
> + !Info, !IO) :-
> + get_target_modules(analysis_registry, AllModules, TargetModules,
> + !Info, !IO),
> + globals__io_lookup_bool_option(keep_going, KeepGoing, !IO),
> + ( Succeeded0 = no, KeepGoing = no ->
> + Succeeded = no
> + ;
> + foldl2_maybe_stop_at_error(KeepGoing,
> + make_module_target,
> + make_dependency_list(TargetModules, analysis_registry),
> + Succeeded1, !Info, !IO),
> +
> + % Find which module analysis files are suboptimal or invalid.
> + % If there are any invalid files then we repeat the analysis pass.
> + % If there are only suboptimal files then we repeat the analysis up
> + % to the number of times given by the user.
> + ReanalysisPasses = !.Info ^ reanalysis_passes,
> + ReanalyseSuboptimal = (if ReanalysisPasses > 1 then yes else no),
> + modules_needing_reanalysis(ReanalyseSuboptimal, TargetModules,
> + InvalidModules, SuboptimalModules, !IO),
> + ( list__is_not_empty(InvalidModules) ->
> + maybe_reanalyse_modules_message(!IO),
> + list__foldl(reset_analysis_registry_dependency_status,
> + InvalidModules, !Info),
> + build_analyses(MainModuleName, AllModules, Succeeded0, Succeeded,
> + !Info, !IO)
> + ; list__is_not_empty(SuboptimalModules) ->
> + list__foldl(reset_analysis_registry_dependency_status,
> + SuboptimalModules, !Info),
> + !:Info = !.Info ^ reanalysis_passes := ReanalysisPasses - 1,
> + maybe_reanalyse_modules_message(!IO),
> + build_analyses(MainModuleName, AllModules, Succeeded0, Succeeded,
> + !Info, !IO)
> + ;
> + Succeeded = Succeeded0 `and` Succeeded1
> + )
> + ).
> +
> +:- pred modules_needing_reanalysis(bool::in, list(module_name)::in,
> + list(module_name)::out, list(module_name)::out, io::di, io::uo) is det.
> +
> +modules_needing_reanalysis(_, [], [], [], !IO).
> +modules_needing_reanalysis(ReanalyseSuboptimal, [Module | Modules],
> + InvalidModules, SuboptimalModules, !IO) :-
> + analysis.read_module_overall_status(mmc, module_name_to_module_id(Module),
> + MaybeModuleStatus, !IO),
> + (
> + MaybeModuleStatus = yes(ModuleStatus),
> + (
> + ModuleStatus = optimal,
> + modules_needing_reanalysis(ReanalyseSuboptimal, Modules,
> + InvalidModules, SuboptimalModules, !IO)
> + ;
> + ModuleStatus = suboptimal,
> + modules_needing_reanalysis(ReanalyseSuboptimal, Modules,
> + InvalidModules, SuboptimalModules0, !IO),
> + (
> + ReanalyseSuboptimal = yes,
> + SuboptimalModules = [Module | SuboptimalModules0]
> + ;
> + ReanalyseSuboptimal = no,
> + SuboptimalModules = SuboptimalModules0
> + )
> + ;
> + ModuleStatus = invalid,
> + modules_needing_reanalysis(ReanalyseSuboptimal, Modules,
> + InvalidModules0, SuboptimalModules, !IO),
> + InvalidModules = [Module | InvalidModules0]
> + )
> + ;
> + MaybeModuleStatus = no,
> + % The analysis file does not exist. The file might be optimal and
> + % without any analysis results, or for some reason it wasn't created
> + % in this or an earlier pass (and hence probably won't be created
> + % no matter how many times we repeat the analysis).
> + %
I'm don't like the idea of analysis files with no results not existing.
Wouldn't it be safe just to have an empty file that is marked as optimal?
> + % XXX: MaybeModuleStatus could be `no' for some other reason
> + modules_needing_reanalysis(ReanalyseSuboptimal, Modules,
> + InvalidModules, SuboptimalModules, !IO)
> + ).
> +
> +:- pred reset_analysis_registry_dependency_status(module_name::in,
> + make_info::in, make_info::out) is det.
> +
> +reset_analysis_registry_dependency_status(ModuleName, Info,
> + Info ^ dependency_status ^ elem(Dep) := not_considered) :-
> + Dep = target(ModuleName - analysis_registry).
> +
> :- pred shared_libraries_supported(bool::out, io::di, io::uo) is det.
>
> shared_libraries_supported(Supported, !IO) :-
...
> @@ -559,7 +567,51 @@
> MaybeTimestamp = MaybeTimestamp0
> ).
>
> -get_target_timestamp(Search, ModuleName - FileType, MaybeTimestamp,
> +get_target_timestamp(Search, Target, MaybeTimestamp, !Info, !IO) :-
> + ( Target = ModuleName - analysis_registry ->
> + get_target_timestamp_analysis_registry(Search, ModuleName,
> + MaybeTimestamp, !Info, !IO)
> + ;
> + get_target_timestamp_2(Search, Target, MaybeTimestamp, !Info, !IO)
> + ).
> +
> + % Special treatment for `.analysis' files. If the `.analysis' file is
> + % valid then we look at the corresponding `.analysis_date' file to get the
> + % last time that the module was actually analysed (the file may have been
> + % rewritten or had it's status changed while analysing other modules).
> + % If the `.analysis' file is invalid then we treat it as out of date.
> + %
> +:- pred get_target_timestamp_analysis_registry(bool::in, module_name::in,
> + maybe_error(timestamp)::out, make_info::in, make_info::out,
> + io::di, io::uo) is det.
> +
> +get_target_timestamp_analysis_registry(Search, ModuleName, MaybeTimestamp,
> + !Info, !IO) :-
> + ModuleId = module_name_to_module_id(ModuleName),
> + analysis.read_module_overall_status(mmc, ModuleId, MaybeStatus, !IO),
> + (
> + MaybeStatus = yes(Status),
> + (
> + ( Status = optimal
> + ; Status = suboptimal
> + ),
> + get_timestamp_file_timestamp(ModuleName - analysis_registry,
> + MaybeTimestamp, !Info, !IO)
> + ;
> + Status = invalid,
> + MaybeTimestamp = error("invalid module")
> + )
> + ;
> + MaybeStatus = no,
> + get_target_timestamp_2(Search, ModuleName - analysis_registry,
> + MaybeTimestamp, !Info, !IO)
> + ).
> +
> +:- pred get_target_timestamp_2(bool::in, target_file::in,
> + maybe_error(timestamp)::out, make_info::in, make_info::out,
> + io::di, io::uo) is det.
> +
> +get_target_timestamp_2(Search, ModuleName - FileType, MaybeTimestamp,
> !Info, !IO) :-
> get_file_name(Search, ModuleName - FileType, FileName, !Info, !IO),
> (
...
> Index: compiler/mercury_compile.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/mercury_compile.m,v
> retrieving revision 1.374
> diff -u -r1.374 mercury_compile.m
> --- compiler/mercury_compile.m 1 Feb 2006 04:02:46 -0000 1.374
> +++ compiler/mercury_compile.m 1 Feb 2006 05:10:26 -0000
> @@ -543,6 +543,7 @@
> generate_dependency_file, make_interface,
> make_short_interface, make_private_interface,
> make_optimization_interface, make_transitive_opt_interface,
> + make_analysis_registry,
> typecheck_only, errorcheck_only],
> BoolList = list__map((func(Opt) = Bool :-
> globals__lookup_bool_option(Globals, Opt, Bool)),
> @@ -1438,6 +1439,8 @@
> MakeOptInt, !IO),
> globals__io_lookup_bool_option(make_transitive_opt_interface,
> MakeTransOptInt, !IO),
> + globals__io_lookup_bool_option(make_analysis_registry,
> + MakeAnalysisRegistry, !IO),
> ( TypeCheckOnly = yes ->
> FactTableObjFiles = []
> ; ErrorCheckOnly = yes ->
> @@ -1460,6 +1463,9 @@
> ; MakeTransOptInt = yes ->
> output_trans_opt_file(HLDS21, !DumpInfo, !IO),
> FactTableObjFiles = []
> + ; MakeAnalysisRegistry = yes ->
> + output_analysis_file(ModuleName, HLDS21, _HLDS, !DumpInfo, !IO),
Why does this call return the HLDS?
...
> @@ -2197,6 +2188,49 @@
> maybe_dump_hlds(!.HLDS, 167, "trail_usage", !DumpInfo, !IO),
> trans_opt__write_optfile(!.HLDS, !IO).
>
> + % XXX: this is largely a duplicate of output_trans_opt_file
> + %
Why is that an XXX?
> +:- pred output_analysis_file(module_name::in,
> + module_info::in, module_info::out, dump_info::in, dump_info::out,
> + io::di, io::uo) is det.
> +
> +output_analysis_file(ModuleName, !HLDS, !DumpInfo, !IO) :-
> + globals__io_lookup_bool_option(verbose, Verbose, !IO),
> + globals__io_lookup_bool_option(statistics, Stats, !IO),
> + globals__io_lookup_bool_option(analyse_closures, ClosureAnalysis, !IO),
> + %
> + % Closure analysis assumes that lambda expressions have
> + % been converted into separate predicates.
> + %
> + (
> + ClosureAnalysis = yes,
> + process_lambdas(Verbose, Stats, !HLDS, !IO)
> + ;
> + ClosureAnalysis = no
> + ),
> + maybe_dump_hlds(!.HLDS, 110, "lambda", !DumpInfo, !IO),
> + maybe_closure_analysis(Verbose, Stats, !HLDS, !IO),
> + maybe_dump_hlds(!.HLDS, 117, "closure_analysis", !DumpInfo, !IO),
> + maybe_exception_analysis(Verbose, Stats, !HLDS, !IO),
> + maybe_dump_hlds(!.HLDS, 118, "exception_analysis", !DumpInfo, !IO),
> + maybe_termination(Verbose, Stats, !HLDS, !IO),
> + maybe_dump_hlds(!.HLDS, 120, "termination", !DumpInfo, !IO),
> + maybe_termination2(Verbose, Stats, !HLDS, !IO),
> + maybe_dump_hlds(!.HLDS, 121, "termination_2", !DumpInfo, !IO),
> + maybe_unused_args(Verbose, Stats, !HLDS, !IO),
> + maybe_dump_hlds(!.HLDS, 165, "unused_args", !DumpInfo, !IO),
> + maybe_analyse_trail_usage(Verbose, Stats, !HLDS, !IO),
> + maybe_dump_hlds(!.HLDS, 167, "trail_usage", !DumpInfo, !IO),
> +
> + module_info_get_analysis_info(!.HLDS, AnalysisInfo0),
> + module_info_get_all_deps(!.HLDS, ImportedModules),
> + ModuleId = module_name_to_module_id(ModuleName),
> + ImportedModuleIds = set.map(module_name_to_module_id,
> + ImportedModules),
> + analysis__write_analysis_files(mmc, ModuleId, ImportedModuleIds,
> + AnalysisInfo0, AnalysisInfo, !IO),
> + module_info_set_analysis_info(AnalysisInfo, !HLDS).
There's no point in doing that; we're never going to use the analysis_info
again after this point.
...
> + debug_intermodule_analysis - bool(no)
> ]).
> option_defaults_2(output_option, [
> % Output Options (mutually exclusive)
> @@ -898,6 +902,7 @@
> make_private_interface - bool(no),
> make_optimization_interface - bool(no),
> make_transitive_opt_interface - bool(no),
> + make_analysis_registry - bool(no),
> convert_to_mercury - bool(no),
> typecheck_only - bool(no),
> errorcheck_only - bool(no),
> @@ -1139,12 +1144,13 @@
> opt_level_number - int(-2),
> opt_space - special,
> intermodule_optimization - bool(no),
> - intermodule_analysis - bool(no),
> read_opt_files_transitively - bool(yes),
> automatic_intermodule_optimization - bool(yes),
> use_opt_files - bool(no),
> use_trans_opt_files - bool(no),
> transitive_optimization - bool(no),
> + intermodule_analysis - bool(no),
> + max_reanalysis_passes - int(0),
I think just `max_analysis_passes' would be better name for it,
or maybe `analysis-repeat' (since we alread have --optimise-repeat).
> @@ -1591,6 +1597,7 @@
> long_option("debug-make", debug_make).
> long_option("debug-closure", debug_closure).
> long_option("debug-trail-usage", debug_trail_usage).
> +long_option("debug-intermodule-analysis", debug_intermodule_analysis).
>
> % output options (mutually exclusive)
> long_option("generate-source-file-mapping",
> @@ -1612,6 +1619,8 @@
> long_option("make-transitive-optimisation-interface",
> make_transitive_opt_interface).
> long_option("make-trans-opt", make_transitive_opt_interface).
> +long_option("make-analysis-registry", make_analysis_registry).
> +long_option("make-analysis", make_analysis_registry).
> long_option("convert-to-mercury", convert_to_mercury).
> long_option("convert-to-Mercury", convert_to_mercury).
> long_option("pretty-print", convert_to_mercury).
> @@ -1835,7 +1844,6 @@
> long_option("intermod-opt", intermodule_optimization).
> long_option("intermodule-optimization", intermodule_optimization).
> long_option("intermodule-optimisation", intermodule_optimization).
> -long_option("intermodule-analysis", intermodule_analysis).
> long_option("read-opt-files-transitively", read_opt_files_transitively).
> long_option("automatic-intermodule-optimization",
> automatic_intermodule_optimization).
> @@ -1848,6 +1856,8 @@
> long_option("transitive-intermodule-optimisation",
> transitive_optimization).
> long_option("trans-intermod-opt", transitive_optimization).
> +long_option("intermodule-analysis", intermodule_analysis).
> +long_option("max-reanalysis-passes", max_reanalysis_passes).
>
> % HLDS->HLDS optimizations
> long_option("inlining", inlining).
> @@ -2915,6 +2925,9 @@
> % "\tOutput detailed debugging traces of the closure analysis."
> "--debug-trail-usage",
> "\tOutput detailed debugging traces of the `--analyse-trail-usage'",
> + "\toption.",
> + "--debug-intermodule-analysis",
> + "\tOutput detailed debugging traces of the `--intermodule-analysis'",
> "\toption."
> ]).
>
> @@ -2964,6 +2977,10 @@
> "\tOutput transitive optimization information",
> "\tinto the `<module>.trans_opt' file.",
> "\tThis option should only be used by mmake.",
> + "--make-analysis",
> + "--make-analysis-registry",
> + "\tOutput analysis information into the `<module>.analysis' file.",
> + "\tThis option should only be used by mmake.",
Is it useful to use it with mmake? (for users not developers). Indeed,
it's probably better if it's left undocumented as I can't think when an
end-user is going to want to use it.
The rest of that looks okay.
Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
post: mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------
More information about the reviews
mailing list