[m-rev.] for review: more work on intermodule analysis
Peter Wang
wangp at students.cs.mu.OZ.AU
Thu Feb 2 15:30:47 AEDT 2006
On 2006-02-02, Julien Fischer <juliensf at cs.mu.OZ.AU> wrote:
> > + (
> > + Result = succeeded(ModuleStatus),
> > + MaybeModuleStatus = yes(ModuleStatus)
> > + ;
> > + Result = failed,
> > + MaybeModuleStatus = no
> > + ;
> > + Result = exception(_),
> > + % XXX Report error.
> > + MaybeModuleStatus = no
>
> Yes, you should ;-)
Left it for "later" ;-)
> > 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.
It's a counter that is decremented each time we do a pass, until it gets
to zero.
> > +:- 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.
That would require two levels of switches so I have left it.
> > +:- 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.
Ok.
> > + 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?
Ok, I've reverted it.
> > @@ -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?
Fixed.
> > @@ -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?
>
Removed the XXX.
> > + 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.
Ok.
> > @@ -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).
Ok, `analysis-repeat'.
> > @@ -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).
No, just a copy-and-paste bug.
> 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.
Removed the documentation, and the `--make-analysis' shorthand, since
users will not be typing it anyway.
Interdiff follows.
Peter
diff -u analysis/analysis.file.m analysis/analysis.file.m
--- analysis/analysis.file.m 31 Jan 2006 05:50:53 -0000
+++ analysis/analysis.file.m 2 Feb 2006 03:47:52 -0000
@@ -13,6 +13,10 @@
:- interface.
+ % read_module_overall_status(Compiler, ModuleId, MaybeModuleStatus,
+ % !IO)
+ % Attempt to read the overall status from a module `.analysis' file.
+ %
:- pred read_module_overall_status(Compiler::in, module_id::in,
maybe(analysis_status)::out, io::di, io::uo) is det
<= compiler(Compiler).
@@ -426,22 +430,10 @@
io.print(ModuleId, !IO),
io.nl(!IO)
), !IO),
- module_id_to_file_name(Info ^ compiler, ModuleId,
- analysis_registry_suffix, AnalysisFileName, !IO),
- file_exists(AnalysisFileName, FileExists, !IO),
- (
- FileExists = no,
- ModuleStatus = optimal,
- map__is_empty(ModuleResults)
- ->
- % If the analysis file doesn't exist and there is no useful
- % information to be written, leave the file as non-existant.
- true
- ;
- WriteHeader = write_module_status(ModuleStatus),
- write_analysis_file(AnalysisFileName,
- WriteHeader, write_result_entry, ModuleResults, !IO)
- ).
+ WriteHeader = write_module_status(ModuleStatus),
+ write_analysis_file(Info ^ compiler,
+ ModuleId, analysis_registry_suffix,
+ WriteHeader, write_result_entry, ModuleResults, !IO).
:- pred write_module_status(analysis_status::in, io::di, io::uo) is det.
@@ -543,18 +535,8 @@
], context_init), !IO).
write_module_imdg(Info, ModuleId, ModuleEntries, !IO) :-
- module_id_to_file_name(Info ^ compiler, ModuleId, imdg_suffix, FileName,
- !IO),
- file_exists(FileName, FileExists, !IO),
- (
- FileExists = no,
- map__is_empty(ModuleEntries)
- ->
- true
- ;
- write_analysis_file(Info ^ compiler, ModuleId, imdg_suffix,
- nop, write_imdg_arc(Info ^ compiler), ModuleEntries, !IO)
- ).
+ write_analysis_file(Info ^ compiler, ModuleId, imdg_suffix,
+ nop, write_imdg_arc(Info ^ compiler), ModuleEntries, !IO).
:- pred write_imdg_arc(Compiler::in)
`with_type` write_entry(imdg_arc)
@@ -598,15 +580,6 @@
WriteEntry, ModuleResults, !IO) :-
module_id_to_file_name(Compiler, ModuleId,
Suffix, AnalysisFileName, !IO),
- write_analysis_file(AnalysisFileName, WriteHeader,
- WriteEntry, ModuleResults, !IO).
-
-:- pred write_analysis_file(string::in, write_header::in(write_header),
- write_entry(T)::in(write_entry), module_analysis_map(T)::in,
- io__state::di, io__state::uo) is det.
-
-write_analysis_file(AnalysisFileName, WriteHeader,
- WriteEntry, ModuleResults, !IO) :-
io__open_output(AnalysisFileName, OpenResult, !IO),
(
OpenResult = ok(Stream),
@@ -651,19 +624,6 @@
), !IO),
io__remove_file(RequestFileName, _, !IO).
-:- pred file_exists(string::in, bool::out, io::di, io::uo) is det.
-
-file_exists(FileName, Exists, !IO) :-
- io.open_input(FileName, Result, !IO),
- (
- Result = ok(Stream),
- io.close_input(Stream, !IO),
- Exists = yes
- ;
- Result = error(_),
- Exists = no
- ).
-
:- pred nop(io::di, io::uo) is det.
nop(!IO).
diff -u analysis/analysis.m analysis/analysis.m
--- analysis/analysis.m 31 Jan 2006 05:52:08 -0000
+++ analysis/analysis.m 1 Feb 2006 23:21:48 -0000
@@ -203,6 +203,10 @@
analysis_info::in, analysis_info::out, io::di, io::uo) is det
<= compiler(Compiler).
+ % read_module_overall_status(Compiler, ModuleId, MaybeModuleStatus,
+ % !IO)
+ % Attempt to read the overall status from a module `.analysis' file.
+ %
:- pred read_module_overall_status(Compiler::in, module_id::in,
maybe(analysis_status)::out, io::di, io::uo) is det
<= compiler(Compiler).
diff -u compiler/make.m compiler/make.m
--- compiler/make.m 31 Jan 2006 00:19:05 -0000
+++ compiler/make.m 1 Feb 2006 23:33:50 -0000
@@ -147,6 +147,7 @@
command_line_targets :: set(pair(module_name, target_type)),
% Number of repeated passes of analysis allowed.
+ % It counts down to zero as analysis passes are performed.
reanalysis_passes :: int
).
@@ -280,15 +281,14 @@
ClassifiedTargets),
ShouldRebuildDeps = yes,
- globals__io_lookup_int_option(max_reanalysis_passes,
- MaxReanalysisPasses, !IO),
+ globals__io_lookup_int_option(analysis_repeat, AnalysisRepeat, !IO),
MakeInfo0 = make_info(map__init, map__init, OptionArgs, Variables,
map__init,
init_cached_direct_imports,
init_cached_transitive_dependencies,
ShouldRebuildDeps, KeepGoing,
set__init, no, set__list_to_set(ClassifiedTargets),
- MaxReanalysisPasses),
+ AnalysisRepeat),
%
% Build the targets, stopping on any errors if
diff -u compiler/make.program_target.m compiler/make.program_target.m
--- compiler/make.program_target.m 31 Jan 2006 05:57:42 -0000
+++ compiler/make.program_target.m 2 Feb 2006 04:27:48 -0000
@@ -69,7 +69,7 @@
% 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.
+ % `.analysis' files are in a valid state before we can continue.
globals__io_lookup_bool_option(intermodule_analysis,
IntermoduleAnalysis, !IO),
(
@@ -525,7 +525,7 @@
)
;
TargetType = build_analyses,
- build_analyses(MainModuleName, AllModules, Succeeded0, Succeeded,
+ build_analysis_files(MainModuleName, AllModules, Succeeded0, Succeeded,
!Info, !IO)
;
TargetType = build_library,
@@ -592,11 +592,11 @@
)
).
-:- pred build_analyses(module_name::in, list(module_name)::in,
+:- pred build_analysis_files(module_name::in, list(module_name)::in,
bool::in, bool::out, make_info::in, make_info::out, io::di, io::uo)
is det.
-build_analyses(MainModuleName, AllModules, Succeeded0, Succeeded,
+build_analysis_files(MainModuleName, AllModules, Succeeded0, Succeeded,
!Info, !IO) :-
get_target_modules(analysis_registry, AllModules, TargetModules,
!Info, !IO),
@@ -621,15 +621,15 @@
maybe_reanalyse_modules_message(!IO),
list__foldl(reset_analysis_registry_dependency_status,
InvalidModules, !Info),
- build_analyses(MainModuleName, AllModules, Succeeded0, Succeeded,
- !Info, !IO)
+ build_analysis_files(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)
+ build_analysis_files(MainModuleName, AllModules,
+ Succeeded0, Succeeded, !Info, !IO)
;
Succeeded = Succeeded0 `and` Succeeded1
)
@@ -668,12 +668,16 @@
)
;
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).
+ % The analysis file does not exist. 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).
+ %
+ % XXX: Currently modules which are basically empty do not get
+ % `.analysis' files produced. After that is fixed we can probably
+ % consider modules with missing `.analysis' files to be invalid.
%
% XXX: MaybeModuleStatus could be `no' for some other reason
+ %
modules_needing_reanalysis(ReanalyseSuboptimal, Modules,
InvalidModules, SuboptimalModules, !IO)
).
diff -u compiler/make.util.m compiler/make.util.m
--- compiler/make.util.m 31 Jan 2006 05:57:29 -0000
+++ compiler/make.util.m 2 Feb 2006 03:39:33 -0000
@@ -596,7 +596,18 @@
; Status = suboptimal
),
get_timestamp_file_timestamp(ModuleName - analysis_registry,
- MaybeTimestamp, !Info, !IO)
+ MaybeTimestamp0, !Info, !IO),
+ (
+ MaybeTimestamp0 = ok(_),
+ MaybeTimestamp = MaybeTimestamp0
+ ;
+ MaybeTimestamp0 = error(_),
+ % If the `.analysis' file exists with status `optimal' or
+ % `suboptimal' but there is no `.analysis_date' file, then the
+ % `.analysis' file must just have been created while analysing
+ % a different module.
+ MaybeTimestamp = ok(oldest_timestamp)
+ )
;
Status = invalid,
MaybeTimestamp = error("invalid module")
diff -u compiler/mercury_compile.m compiler/mercury_compile.m
--- compiler/mercury_compile.m 1 Feb 2006 05:10:26 -0000
+++ compiler/mercury_compile.m 1 Feb 2006 23:48:02 -0000
@@ -1464,7 +1464,7 @@
output_trans_opt_file(HLDS21, !DumpInfo, !IO),
FactTableObjFiles = []
; MakeAnalysisRegistry = yes ->
- output_analysis_file(ModuleName, HLDS21, _HLDS, !DumpInfo, !IO),
+ output_analysis_file(ModuleName, HLDS21, !DumpInfo, !IO),
FactTableObjFiles = []
;
mercury_compile_after_front_end(NestedSubModules,
@@ -2188,13 +2188,13 @@
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
+ % This is largely a duplicate of output_trans_opt_file.
%
:- pred output_analysis_file(module_name::in,
- module_info::in, module_info::out, dump_info::in, dump_info::out,
+ module_info::in, dump_info::in, dump_info::out,
io::di, io::uo) is det.
-output_analysis_file(ModuleName, !HLDS, !DumpInfo, !IO) :-
+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),
@@ -2222,14 +2222,13 @@
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_analysis_info(!.HLDS, AnalysisInfo),
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).
+ AnalysisInfo, _AnalysisInfo, !IO).
:- pred frontend_pass_by_phases(module_info::in, module_info::out,
bool::out, dump_info::in, dump_info::out, io::di, io::uo) is det.
diff -u compiler/options.m compiler/options.m
--- compiler/options.m 1 Feb 2006 03:42:32 -0000
+++ compiler/options.m 1 Feb 2006 23:31:31 -0000
@@ -464,7 +464,7 @@
; use_trans_opt_files
; transitive_optimization
; intermodule_analysis
- ; max_reanalysis_passes
+ ; analysis_repeat
% - HLDS
; allow_inlining
@@ -1150,7 +1150,7 @@
use_trans_opt_files - bool(no),
transitive_optimization - bool(no),
intermodule_analysis - bool(no),
- max_reanalysis_passes - int(0),
+ analysis_repeat - int(0),
check_termination - bool(no),
verbose_check_termination - bool(no),
termination - bool(no),
@@ -1620,7 +1620,6 @@
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).
@@ -1857,7 +1856,7 @@
transitive_optimization).
long_option("trans-intermod-opt", transitive_optimization).
long_option("intermodule-analysis", intermodule_analysis).
-long_option("max-reanalysis-passes", max_reanalysis_passes).
+long_option("analysis-repeat", analysis_repeat).
% HLDS->HLDS optimizations
long_option("inlining", inlining).
@@ -2977,10 +2976,6 @@
"\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.",
"-P, --convert-to-mercury",
"\tConvert to Mercury. Output to file `<module>.ugly'",
"\tThis option acts as a Mercury ugly-printer.",
@@ -3860,7 +3855,7 @@
"\tPerform analyses such as termination analysis and",
"\tunused argument elimination across module boundaries.",
"\tThis option is not yet fully implemented.",
- "--max-reanalysis-passes <n>",
+ "--analysis-repeat <n>",
"\tThe maximum number of times to repeat analyses of",
"\tsuboptimal modules with `--intermodule-analyses'",
"\t(default: 0)."
diff -u doc/user_guide.texi doc/user_guide.texi
--- doc/user_guide.texi 1 Feb 2006 03:15:41 -0000
+++ doc/user_guide.texi 1 Feb 2006 23:31:01 -0000
@@ -6905,8 +6905,8 @@
unused argument elimination across module boundaries.
This option is not yet fully implemented.
- at item --max-reanalysis-passes <n>
- at findex --max-reanalysis-passes
+ at item --analysis-repeat <n>
+ at findex --analysis-repeat
The maximum number of times to repeat analyses of suboptimal modules with
@samp{--intermodule-analyses} (default: 0). This option only works with
@samp{mmc --make}.
--------------------------------------------------------------------------
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