[m-rev.] for review: analysis framework (2/2)
Julien Fischer
juliensf at cs.mu.OZ.AU
Tue Jan 17 15:09:38 AEDT 2006
On Mon, 16 Jan 2006, Peter Wang wrote:
> Index: compiler/add_pragma.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/add_pragma.m,v
> retrieving revision 1.19
> diff -u -r1.19 add_pragma.m
> --- compiler/add_pragma.m 28 Nov 2005 04:11:37 -0000 1.19
> +++ compiler/add_pragma.m 13 Jan 2006 02:52:55 -0000
> @@ -105,6 +105,7 @@
>
> :- implementation.
>
> +:- import_module analysis.
> :- import_module backend_libs.
> :- import_module backend_libs.foreign.
> :- import_module check_hlds.mode_util.
> @@ -599,7 +600,10 @@
> ->
> module_info_get_trailing_info(!.ModuleInfo, TrailingInfo0),
> proc_id_to_int(ProcId, ModeNum),
> - map.set(TrailingInfo0, proc(PredId, ProcId), TrailingStatus,
> + % The `optimal' part is only used by --intermodule-analysis
> + % whereas this is for --intermodule-optimisation.
> + % It should not matter what we put there.
> + map.set(TrailingInfo0, proc(PredId, ProcId), {TrailingStatus, optimal},
See below.
> Index: compiler/hlds_module.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/hlds_module.m,v
> retrieving revision 1.127
> diff -u -r1.127 hlds_module.m
> --- compiler/hlds_module.m 28 Nov 2005 04:11:42 -0000 1.127
> +++ compiler/hlds_module.m 5 Jan 2006 00:37:17 -0000
> @@ -107,7 +107,7 @@
> % Map from proc to an indication of whether or not it
> % modifies the trail.
> %
> -:- type trailing_info == map(pred_proc_id, trailing_status).
> +:- type trailing_info == map(pred_proc_id, {trailing_status, analysis_status}).
I suggest: s/maybe(analysis_status)/
I suggest making that maybe(analysis_status). In fact rather than
having a tuple there it would be preferable to have a du type for it,
e.g.
:- type proc_trailing_info
---> proc_trailing_info(
proc_trailing_status :: trailing_status,
proc_analysis_status :: analysis_status
).
That may mean that some of the fields of the proc_result type may need
to be renamed.
> Index: compiler/mercury_compile.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/mercury_compile.m,v
> retrieving revision 1.371
> diff -u -r1.371 mercury_compile.m
> --- compiler/mercury_compile.m 3 Jan 2006 04:07:50 -0000 1.371
> +++ compiler/mercury_compile.m 4 Jan 2006 07:15:00 -0000
> @@ -1512,13 +1512,19 @@
> !IO),
> (
> IntermodAnalysis = yes,
> - module_info_get_analysis_info(HLDS50, AnalysisInfo),
> - analysis__write_analysis_files(
> - module_name_to_module_id(ModuleName), AnalysisInfo, !IO)
> + module_info_get_analysis_info(HLDS50, AnalysisInfo0),
> + module_info_get_all_deps(HLDS50, ImportedModules),
> + ModuleId = module_name_to_module_id(ModuleName),
> + ImportedModuleIds = set.map(module_name_to_module_id,
> + ImportedModules),
> + analysis__write_analysis_files(ModuleId, ImportedModuleIds,
> + AnalysisInfo0, AnalysisInfo, !IO),
> + module_info_set_analysis_info(AnalysisInfo, HLDS50, HLDS50B)
> ;
> - IntermodAnalysis = no
> + IntermodAnalysis = no,
> + HLDS50B = HLDS50
Ideally you should avoid this style of numbering for versions of the
HLDS. In fact, it would be worth changing that entire predicate to use
state variables to pass around the HLDS.
> Index: compiler/mmc_analysis.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/mmc_analysis.m,v
> retrieving revision 1.9
> diff -u -r1.9 mmc_analysis.m
> --- compiler/mmc_analysis.m 28 Oct 2005 02:10:22 -0000 1.9
> +++ compiler/mmc_analysis.m 15 Dec 2005 02:19:05 -0000
> @@ -36,6 +36,7 @@
> :- import_module parse_tree.modules.
> :- import_module parse_tree.prog_out.
> :- import_module parse_tree.prog_util.
> +:- import_module transform_hlds.trailing_analysis.
> :- import_module transform_hlds.unused_args.
>
> :- import_module bool.
> @@ -45,10 +46,14 @@
> :- instance compiler(mmc) where [
> compiler_name(mmc) = "mmc",
>
> - analyses(mmc, "unused_args") =
> + analyses(mmc, "trailing_info") =
s/trail_usage/trailing_info/ (There's no reason that the analysis
name needs to be the same as the pragma name.)
...
> Index: compiler/trailing_analysis.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/trailing_analysis.m,v
> retrieving revision 1.4
> diff -u -r1.4 trailing_analysis.m
> --- compiler/trailing_analysis.m 24 Dec 2005 08:44:12 -0000 1.4
> +++ compiler/trailing_analysis.m 6 Jan 2006 02:21:39 -0000
> @@ -53,6 +53,7 @@
> :- module transform_hlds.trailing_analysis.
> :- interface.
>
> +:- import_module analysis.
> :- import_module hlds.hlds_module.
> :- import_module hlds.hlds_pred.
>
> @@ -70,6 +71,14 @@
> :- pred write_pragma_trailing_info(module_info::in, trailing_info::in,
> pred_id::in, io::di, io::uo) is det.
>
> + % Types and instances for the intermodule analysis framework.
> + %
> +:- type trailing_analysis_answer.
> +:- instance analysis(any_call, trailing_analysis_answer).
> +:- instance partial_order(trailing_analysis_answer).
> +:- instance answer_pattern(trailing_analysis_answer).
> +:- instance to_string(trailing_analysis_answer).
> +
> %----------------------------------------------------------------------------%
> %----------------------------------------------------------------------------%
>
> @@ -96,6 +105,7 @@
> :- import_module parse_tree.prog_util.
> :- import_module parse_tree.prog_type.
> :- import_module transform_hlds.dependency_graph.
> +:- import_module transform_hlds.mmc_analysis.
>
> :- import_module bool.
> :- import_module list.
> @@ -155,20 +165,22 @@
>
> :- type proc_result
> ---> proc_result(
> - ppid :: pred_proc_id,
> - status :: trailing_status
> + ppid :: pred_proc_id,
> + status :: trailing_status,
> + analysis_status :: analysis_status
> ).
>
> :- pred process_scc(bool::in, bool::in, scc::in,
> module_info::in, module_info::out, io::di, io::uo) is det.
>
> process_scc(Debug, Pass1Only, SCC, !ModuleInfo, !IO) :-
> - ProcResults = check_procs_for_trail_mods(SCC, !.ModuleInfo),
> + check_procs_for_trail_mods(SCC, ProcResults, !ModuleInfo, !IO),
> %
> % The `Results' above are the results of analysing each individual
> % procedure in the SCC - we now have to combine them in a meaningful way.
> %
> Status = combine_individual_proc_results(ProcResults),
> + AnalysisStatus = combine_individual_proc_statuses(ProcResults),
>
I suggest making combine_individual_proc_results into a predicate that
returns two outputs, one the trailing status and the other the analysis
status.
> % Print out debugging information.
> %
> @@ -183,10 +195,21 @@
> %
> module_info_get_trailing_info(!.ModuleInfo, TrailingInfo0),
> Update = (pred(PPId::in, Info0::in, Info::out) is det :-
> - Info = Info0 ^ elem(PPId) := Status
> + Info = Info0 ^ elem(PPId) := {Status, AnalysisStatus}
> ),
> list.foldl(Update, SCC, TrailingInfo0, TrailingInfo),
> module_info_set_trailing_info(TrailingInfo, !ModuleInfo),
> + %
> + % Record the analysis results for the intermodule analysis
> + %
> + globals__io_lookup_bool_option(intermodule_analysis, Intermod, !IO),
> + (
> + Intermod = yes,
> + record_trailing_analysis_results(Status, AnalysisStatus, SCC,
> + !ModuleInfo)
> + ;
> + Intermod = no
> + ),
> (
> Pass1Only = no,
> list.foldl(annotate_proc, SCC, !ModuleInfo)
> @@ -196,10 +219,12 @@
>
> % Check each procedure in the SCC individually.
> %
> -:- func check_procs_for_trail_mods(scc, module_info) = proc_results.
> +:- pred check_procs_for_trail_mods(scc::in, proc_results::out,
> + module_info::in, module_info::out, io::di, io::uo) is det.
>
> -check_procs_for_trail_mods(SCC, ModuleInfo) = Result :-
> - list.foldl(check_proc_for_trail_mods(SCC, ModuleInfo), SCC, [], Result).
> +check_procs_for_trail_mods(SCC, Result, !ModuleInfo, !IO) :-
> + list.foldl3(check_proc_for_trail_mods(SCC), SCC, [], Result,
> + !ModuleInfo, !IO).
>
> % Examine how the procedures interact with other procedures that
> % are mutually-recursive to them.
> @@ -230,60 +255,76 @@
> SCC_Result = may_modify_trail
> ).
>
> +:- func combine_individual_proc_statuses(proc_results) = analysis_status.
> +
> +combine_individual_proc_statuses(ProcResults) =
> + list.foldl((func(proc_result(_, _, Status), Acc)
> + = analysis.lub(Status, Acc)),
> + ProcResults, optimal).
> +
See above.
...
> @@ -876,6 +958,166 @@
> true
> ).
>
> +%-----------------------------------------------------------------------------%
> +%
> +% Stuff for the intermodule analysis framework
> +%
> +
> +:- type trailing_analysis_answer
> + ---> trailing_analysis_answer(trailing_status).
> +
> +:- func analysis_name = string.
> +analysis_name = "trailing_info". % same name as used for the pragmas
s/trail_usage/trailing_info/
> +
> +:- instance analysis(any_call, trailing_analysis_answer) where [
> + analysis_name(_, _) = analysis_name,
> + analysis_version_number(_, _) = 1,
> + preferred_fixpoint_type(_, _) = least_fixpoint,
> + % XXX: I have no idea if this is correct.
It's correct.
> + bottom(_) = trailing_analysis_answer(will_not_modify_trail),
> + top(_) = trailing_analysis_answer(may_modify_trail)
> +].
> +
> +:- instance answer_pattern(trailing_analysis_answer) where [].
> +:- instance partial_order(trailing_analysis_answer) where [
> + (more_precise_than(
> + trailing_analysis_answer(Status1),
> + trailing_analysis_answer(Status2)) :-
> + trailing_status_more_precise_than(Status1, Status2)),
> + equivalent(Status, Status)
> +].
> +
> +:- pred trailing_status_more_precise_than(trailing_status::in,
> + trailing_status::in) is semidet.
> +
> +trailing_status_more_precise_than(will_not_modify_trail, may_modify_trail).
> +trailing_status_more_precise_than(will_not_modify_trail, conditional).
> +trailing_status_more_precise_than(conditional, may_modify_trail).
> +
> +:- instance to_string(trailing_analysis_answer) where [
> + func(to_string/1) is trailing_analysis_answer_to_string,
> + func(from_string/1) is trailing_analysis_answer_from_string
> +].
> +
> +:- func trailing_analysis_answer_to_string(trailing_analysis_answer) = string.
> +
> +trailing_analysis_answer_to_string(trailing_analysis_answer(Status)) = Str :-
> + trailing_status_to_string(Status, Str).
> +
> +:- func trailing_analysis_answer_from_string(string) =
> + trailing_analysis_answer is semidet.
> +
> +trailing_analysis_answer_from_string(Str) = trailing_analysis_answer(Status) :-
> + trailing_status_to_string(Status, Str).
> +
> +:- pred trailing_status_to_string(trailing_status, string).
> +:- mode trailing_status_to_string(in, out) is det.
> +:- mode trailing_status_to_string(out, in) is semidet.
> +
> +trailing_status_to_string(may_modify_trail, "may_modify_trail").
> +trailing_status_to_string(will_not_modify_trail, "will_not_modify_trail").
> +trailing_status_to_string(conditional, "conditional").
> +
> +:- pred search_analysis_status(pred_proc_id::in,
> + trailing_status::out, analysis_status::out, scc::in,
> + module_info::in, module_info::out, io::di, io::uo) is det.
> +
> +search_analysis_status(PPId, Result, AnalysisStatus, CallerSCC,
> + !ModuleInfo, !IO) :-
> + globals__io_lookup_bool_option(intermodule_analysis, Intermod, !IO),
> + (
> + Intermod = yes,
> + module_info_get_analysis_info(!.ModuleInfo, AnalysisInfo0),
> + search_analysis_status_2(!.ModuleInfo, PPId, Result, AnalysisStatus,
> + CallerSCC, AnalysisInfo0, AnalysisInfo, !IO),
> + module_info_set_analysis_info(AnalysisInfo, !ModuleInfo)
> + ;
> + Intermod = no,
> + % If we do not have any information about the callee procedure
> + % then assume that it modifies the trail.
> + Result = may_modify_trail,
> + AnalysisStatus = optimal % unused anyway
> + ).
> +
> +:- pred search_analysis_status_2(module_info::in, pred_proc_id::in,
> + trailing_status::out, analysis_status::out, scc::in,
> + analysis_info::in, analysis_info::out, io::di, io::uo) is det.
> +
> +search_analysis_status_2(ModuleInfo, PPId, Result, AnalysisStatus, CallerSCC,
> + !AnalysisInfo, !IO) :-
> + module_id_func_id(ModuleInfo, PPId, ModuleId, FuncId),
> + Call = any_call,
> + analysis.lookup_best_result(ModuleId, FuncId, Call,
> + MaybeBestStatus, !AnalysisInfo, !IO),
> + (
> + MaybeBestStatus = yes({BestCall, trailing_analysis_answer(Result),
> + AnalysisStatus}),
> + record_dependencies(analysis_name, ModuleId, FuncId, BestCall,
> + ModuleInfo, CallerSCC, !AnalysisInfo)
> + ;
> + MaybeBestStatus = no,
> + % If we do not have any information about the callee procedure
> + % then assume that it modifies the trail.
> + top(Call) = trailing_analysis_answer(Result),
> + AnalysisStatus = suboptimal,
> + analysis.record_request(analysis_name, ModuleId, FuncId, Call,
> + !AnalysisInfo),
> + record_dependencies(analysis_name, ModuleId, FuncId, Call,
> + ModuleInfo, CallerSCC, !AnalysisInfo)
> + ).
> +
> + % XXX if the procedures in CallerSCC definitely come from the
> + % same module then we don't need to record the dependency so many
> + % times, at least while we only have module-level granularity.
> + %
> +:- pred record_dependencies(analysis_name::in, module_id::in, func_id::in,
> + Call::in, module_info::in, scc::in,
> + analysis_info::in, analysis_info::out) is det
> + <= call_pattern(Call).
> +
> +record_dependencies(AnalysisName, ModuleId, FuncId, Call,
> + ModuleInfo, CallerSCC, !AnalysisInfo) :-
> + list.foldl((pred(CallerPPId::in, Info0::in, Info::out) is det :-
> + module_id_func_id(ModuleInfo, CallerPPId, CallerModuleId, _),
> + analysis.record_dependency(CallerModuleId,
> + AnalysisName, ModuleId, FuncId, Call, Info0, Info)
> + ), CallerSCC, !AnalysisInfo).
> +
> +:- pred record_trailing_analysis_results(trailing_status::in,
> + analysis_status::in, scc::in,
> + module_info::in, module_info::out) is det.
> +
> +record_trailing_analysis_results(Status, ResultStatus, SCC, !ModuleInfo) :-
> + module_info_get_analysis_info(!.ModuleInfo, AnalysisInfo0),
> + list.foldl(
> + record_trailing_analysis_result(!.ModuleInfo, Status, ResultStatus),
> + SCC, AnalysisInfo0, AnalysisInfo),
> + module_info_set_analysis_info(AnalysisInfo, !ModuleInfo).
> +
> +:- pred record_trailing_analysis_result(module_info::in, trailing_status::in,
> + analysis_status::in, pred_proc_id::in,
> + analysis_info::in, analysis_info::out) is det.
> +
> +record_trailing_analysis_result(ModuleInfo, Status, ResultStatus, PPId,
> + AnalysisInfo0, AnalysisInfo) :-
> + module_id_func_id(ModuleInfo, PPId, ModuleId, FuncId),
> + record_result(ModuleId, FuncId, any_call,
> + trailing_analysis_answer(Status), ResultStatus,
> + AnalysisInfo0, AnalysisInfo).
> +
> +:- pred module_id_func_id(module_info::in, pred_proc_id::in,
> + module_id::out, func_id::out) is det.
> +
> +module_id_func_id(ModuleInfo, proc(PredId, ProcId), ModuleId, FuncId) :-
> + module_info_pred_info(ModuleInfo, PredId, PredInfo),
> + PredModule = pred_info_module(PredInfo),
> + PredName = pred_info_name(PredInfo),
> + PredOrFunc = pred_info_is_pred_or_func(PredInfo),
> + PredArity = pred_info_orig_arity(PredInfo),
> + ModuleId = module_name_to_module_id(PredModule),
> + FuncId = pred_or_func_name_arity_to_func_id(PredOrFunc,
> + PredName, PredArity, ProcId).
> +
Consider adding this predicate to mmc_analysis.m.
> Index: compiler/unused_args.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/unused_args.m,v
> retrieving revision 1.116
> diff -u -r1.116 unused_args.m
> --- compiler/unused_args.m 4 Jan 2006 07:14:31 -0000 1.116
> +++ compiler/unused_args.m 10 Jan 2006 04:35:38 -0000
> @@ -62,13 +62,14 @@
> % Instances used by mmc_analysis.m
> %
>
...
> %-----------------------------------------------------------------------------%
> @@ -143,6 +144,11 @@
> % Types and instances used by mmc_analysis.m
> %
>
> +:- type unused_args_call
> + ---> unused_args_call(arity).
> + % Stands for any call. The arity is extra information which is
> + % not part of the call pattern.
> +
> % The list of unused arguments is in sorted order.
> :- type unused_args_answer
> ---> unused_args(
> @@ -153,30 +159,45 @@
>
> get_unused_args(UnusedArgs) = UnusedArgs ^ args.
>
> -:- instance analysis(unused_args_func_info, any_call, unused_args_answer)
> +:- instance analysis(unused_args_call, unused_args_answer)
> where [
> - analysis_name(_, _, _) = "unused_args",
> - analysis_version_number(_, _, _) = 1,
> - preferred_fixpoint_type(_, _, _) = least_fixpoint
> + analysis_name(_, _) = analysis_name,
> + analysis_version_number(_, _) = 2,
> + preferred_fixpoint_type(_, _) = least_fixpoint,
> + bottom(unused_args_call(Arity)) = unused_args(1 `..` Arity),
`..` is an infix operator so there's no need for the quotes.
...
> Index: library/list.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/library/list.m,v
> retrieving revision 1.143
> diff -u -r1.143 list.m
> --- library/list.m 15 Nov 2005 04:59:23 -0000 1.143
> +++ library/list.m 19 Dec 2005 00:34:52 -0000
> @@ -845,6 +845,24 @@
> :- mode list__map_foldl2(pred(in, out, in, out, in, out) is nondet,
> in, out, in, out, in, out) is nondet.
>
> + % Same as list__map_foldl, but with two mapped outputs and two
> + % accumulators.
> + %
> +:- pred list__map2_foldl2(pred(L, M, N, A, A, B, B), list(L), list(M), list(N),
> + A, A, B, B).
...
You should mention this new predicate in the NEWS file.
Cheers,
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