[m-rev.] For review: add --simplify-stage-215 and extra information to the exception seen in bug90.

Julien Fischer juliensf at csse.unimelb.edu.au
Fri Feb 27 15:28:53 AEDT 2009


On Fri, 27 Feb 2009, Paul Bone wrote:

>
> For review by anyone.
>
> Estimated hours taken: 1.0
> Branches: main
>
> In the case of an inconsitent rtti_type_infos error print the variable and the

s/inconsitent/inconsistent/

> rtti_type_infos that cause this error.
>
> Add an option to allow an explicit simplification pass to occur at stage 215
> even when profiling is not being used, this is helpful in detecting bugs in
> generated code before the dead procedure elimination removes them.

I suggest:

 	Add an option that forces the pre-profiling simplificatation
 	pass at stage 215 to be run, even when profiling is not be used.
 	This is helpful ...

>
> compiler/hlds_rtti.m:
> 	As above.
>
> compiler/options.m:
> 	Introduce the new --simplify-stage-215 option.
>
> compiler/handle_options.m:
> 	The logic that ensures that --profile_deep, --record-term-sizes-as-words
> 	and --record-term-sizes-as-cells imply --simplify-stage-215 has been added
> 	here, it was orgionally in the simplify predicate of mercury_compile.m
>
> compiler/mercury_compile.m:
> 	Rename simplify to maybe_simplify.
> 	Refactor maybe_simplify, it now uses --simplify-stage-215 to decide if
> 	simplification should be done before the deep profiling transformation.
>




> Index: compiler/hlds_rtti.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/hlds_rtti.m,v
> retrieving revision 1.14
> diff -u -p -b -r1.14 hlds_rtti.m
> --- compiler/hlds_rtti.m	26 Mar 2008 01:38:47 -0000	1.14
> +++ compiler/hlds_rtti.m	24 Feb 2009 06:47:11 -0000
> @@ -326,6 +326,7 @@
> :- import_module map.
> :- import_module pair.
> :- import_module solutions.
> +:- import_module string.
> :- import_module svmap.
> :- import_module term.
> :- import_module varset.
> @@ -639,7 +640,9 @@ apply_substs_to_type_map(TRenaming, TSub
>         ( Type = ExistingType ->
>             true
>         ;
> -            unexpected(this_file, "inconsistent type_infos")
> +            unexpected(this_file, string.format("inconsistent type_infos: "
> +                ++ " Type: %s ExistingType: %s Var0: %s",
> +                [s(string(Type)), s(string(ExistingType)), s(string(Var0))]))
>         )
>     ;
>         svmap.det_insert(Var, Type, !Map)

In my opinion there is no point printing out the third of these.
It is too much information for users and too little (and in most
cases the wrong) information for developers.


> Index: compiler/mercury_compile.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/mercury_compile.m,v
> retrieving revision 1.488
> diff -u -p -b -r1.488 mercury_compile.m
> --- compiler/mercury_compile.m	20 Feb 2009 08:19:04 -0000	1.488
> +++ compiler/mercury_compile.m	26 Feb 2009 06:23:54 -0000
> @@ -2614,7 +2614,8 @@ frontend_pass_by_phases(!HLDS, FoundErro
>         check_stratification(Verbose, Stats, !HLDS, FoundStratError, !IO),
>         maybe_dump_hlds(!.HLDS, 60, "stratification", !DumpInfo, !IO),
>
> -        simplify(yes, frontend, Verbose, Stats, !HLDS, SimplifySpecs, !IO),
> +        maybe_simplify(yes, frontend, Verbose, Stats, !HLDS, SimplifySpecs,
> +            !IO),
>         maybe_dump_hlds(!.HLDS, 65, "frontend_simplify", !DumpInfo, !IO),
>
>         % Once the other passes have all been converted to return error_specs,
> @@ -2780,8 +2781,8 @@ middle_pass(ModuleName, !HLDS, !DumpInfo
>     % opportunities the other optimizations have provided for constant
>     % propagation and we cannot do that once the term-size profiling or deep
>     % profiling transformations have been applied.
> -    simplify(no, pre_prof_transforms, Verbose, Stats, !HLDS, SimplifySpecs,
> -        !IO),
> +    maybe_simplify(no, pre_prof_transforms, Verbose, Stats, !HLDS,
> +        SimplifySpecs, !IO),
>     expect(unify(contains_errors(Globals, SimplifySpecs), no), this_file,
>         "middle_pass: simplify has errors"),
>     maybe_dump_hlds(!.HLDS, 215, "pre_prof_transform_simplify", !DumpInfo,
> @@ -2875,7 +2876,7 @@ backend_pass_by_phases(!HLDS, !GlobalDat
>     maybe_followcode(Verbose, Stats, !HLDS, !IO),
>     maybe_dump_hlds(!.HLDS, 320, "followcode", !DumpInfo, !IO),
>
> -    simplify(no, ll_backend, Verbose, Stats, !HLDS, SimplifySpecs, !IO),
> +    maybe_simplify(no, ll_backend, Verbose, Stats, !HLDS, SimplifySpecs, !IO),
>     expect(unify(contains_errors(Globals, SimplifySpecs), no), this_file,
>         "backend_pass_by_phases: simplify has errors"),
>     maybe_dump_hlds(!.HLDS, 325, "ll_backend_simplify", !DumpInfo, !IO),
> @@ -3482,31 +3483,14 @@ maybe_warn_dead_procs(Verbose, Stats, !H
>     ;       ll_backend.
>             % The first stage of LLDS code generation.
>
> -:- pred simplify(bool::in, simplify_pass::in, bool::in, bool::in,
> +    % This predicate set up and maybe run the simplification pass.
> +    %
> +:- pred maybe_simplify(bool::in, simplify_pass::in, bool::in, bool::in,
>     module_info::in, module_info::out, list(error_spec)::out,
>     io::di, io::uo) is det.
>
> -simplify(Warn, SimplifyPass, Verbose, Stats, !HLDS, Specs, !IO) :-
> +maybe_simplify(Warn, SimplifyPass, Verbose, Stats, !HLDS, Specs, !IO) :-
>     module_info_get_globals(!.HLDS, Globals),
> -    globals.lookup_bool_option(Globals, constant_propagation, ConstProp),
> -    globals.lookup_bool_option(Globals, profile_deep, DeepProf),
> -    globals.lookup_bool_option(Globals, record_term_sizes_as_words, TSWProf),
> -    globals.lookup_bool_option(Globals, record_term_sizes_as_cells, TSCProf),
> -    %
> -    % We run the simplify pass before the profiling transformations,
> -    % only if those transformations are being applied - otherwise we
> -    % just leave things to the backend simplification passes.
> -    %
> -    IsProfPass = bool.or_list([DeepProf, TSWProf, TSCProf]),
> -    (
> -        SimplifyPass = pre_prof_transforms,
> -        IsProfPass = no
> -    ->
> -        Specs = []
> -    ;
> -        maybe_write_string(Verbose, "% Simplifying goals...\n", !IO),
> -        maybe_flush_output(Verbose, !IO),
> -
>         some [!SimpList] (
>             simplify.find_simplifications(Warn, Globals, Simplifications0),
>             !:SimpList = simplifications_to_list(Simplifications0),
> @@ -3518,8 +3502,21 @@ simplify(Warn, SimplifyPass, Verbose, St
>                 list.cons(simp_do_once, !SimpList)
>             ;
>                 SimplifyPass = pre_prof_transforms,
> +            %
> +            % We run the simplify pass before the profiling transformations,
> +            % only if those transformations are being applied - otherwise we
> +            % just leave things to the backend simplification passes.
> +            %
> +            globals.lookup_bool_option(Globals, simplify_stage_215,
> +                Simplify215),
> +            (
> +                Simplify215 = yes,
>                 list.cons(simp_do_once, !SimpList)
>             ;
> +                Simplify215 = no,
> +                !:SimpList = []
> +            )
> +        ;
>                 SimplifyPass = ml_backend,
>                 list.cons(simp_do_once, !SimpList)
>             ;
> @@ -3529,10 +3526,18 @@ simplify(Warn, SimplifyPass, Verbose, St
>                 %
>                 % NOTE: Any changes made here may also need to be made
>                 % to the relevant parts of backend_pass_by_preds_4/12.
> -
> +            globals.lookup_bool_option(Globals, constant_propagation,
> +                ConstProp),
> +            globals.lookup_bool_option(Globals, profile_deep, DeepProf),
> +            globals.lookup_bool_option(Globals, record_term_sizes_as_words,
> +                TSWProf),
> +            globals.lookup_bool_option(Globals, record_term_sizes_as_cells,
> +                TSCProf),
>                 (
>                     ConstProp = yes,
> -                    IsProfPass = no
> +                DeepProf = no,
> +                TSWProf = no,
> +                TSCProf = no
>                 ->
>                     list.cons(simp_constant_prop, !SimpList)
>                 ;
> @@ -3542,14 +3547,22 @@ simplify(Warn, SimplifyPass, Verbose, St
>                 list.cons(simp_do_once, !SimpList),
>                 list.cons(simp_elim_removable_scopes, !SimpList)
>             ),
> -            Simplifications = list_to_simplifications(!.SimpList)
> +        SimpList = !.SimpList
>         ),
> +    (
> +        SimpList = [_ | _],
>
> +        maybe_write_string(Verbose, "% Simplifying goals...\n", !IO),
> +        maybe_flush_output(Verbose, !IO),
> +        Simplifications = list_to_simplifications(SimpList),
>         process_all_nonimported_procs_errors(
>             update_pred_error(simplify_pred(Simplifications)),
>             !HLDS, [], Specs, !IO),
>         maybe_write_string(Verbose, "% done.\n", !IO),
>         maybe_report_stats(Stats, !IO)
> +    ;
> +        SimpList = [],
> +        Specs = []
>     ).
>
> %-----------------------------------------------------------------------------%
> @@ -3919,7 +3932,8 @@ maybe_untuple_arguments(Verbose, Stats,
>         maybe_flush_output(Verbose, !IO),
>         untuple_arguments(!HLDS, !IO),
>         maybe_write_string(Verbose, "% done.\n", !IO),
> -        simplify(no, post_untuple, Verbose, Stats, !HLDS, SimplifySpecs, !IO),
> +        maybe_simplify(no, post_untuple, Verbose, Stats, !HLDS, SimplifySpecs,
> +            !IO),
>         expect(unify(contains_errors(Globals, SimplifySpecs), no), this_file,
>             "maybe_untuple_arguments: simplify has errors"),
>         maybe_report_stats(Stats, !IO)
> @@ -4989,7 +5003,7 @@ mlds_backend(!HLDS, !:MLDS, !DumpInfo, !
>     globals.lookup_bool_option(Globals, verbose, Verbose),
>     globals.lookup_bool_option(Globals, statistics, Stats),
>
> -    simplify(no, ml_backend, Verbose, Stats, !HLDS, SimplifySpecs, !IO),
> +    maybe_simplify(no, ml_backend, Verbose, Stats, !HLDS, SimplifySpecs, !IO),
>     expect(unify(contains_errors(Globals, SimplifySpecs), no), this_file,
>         "ml_backend: simplify has errors"),
>     maybe_dump_hlds(!.HLDS, 405, "ml_backend_simplify", !DumpInfo, !IO),
> Index: compiler/options.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/options.m,v
> retrieving revision 1.643
> diff -u -p -b -r1.643 options.m
> --- compiler/options.m	20 Feb 2009 08:19:04 -0000	1.643
> +++ compiler/options.m	26 Feb 2009 06:23:54 -0000
> @@ -298,7 +298,12 @@
>             % we only want to use the `yes' value, but we keep support for
>             % the `no' value for benchmarks for the paper.
>
> -            % Perform coverage profiling, (enables deep profiling).
> +    ;       simplify_stage_215
> +            % Run the simplification pass at stage 215 (before profiling) this
> +            % is implided by some of the profiling settings.


s/implided/implied/

You also want to say that the option runs it, even when deep profiling
is not enabled.

I wouldn't name the option after the stage number.  I thank that
simplification pass has a name, pre_prof_transforms or something, so
use that instead, e.g.

 	--pre-prof-transform-simplify

> +
> +            % Perform coverage profiling, this affects only deep profiling
> +            % grades.
>     ;       coverage_profiling
>     ;       coverage_profiling_via_calls
>
> @@ -1179,6 +1184,7 @@ option_defaults_2(compilation_model_opti
>     profile_memory                      -   bool(no),
>     profile_deep                        -   bool(no),
>     use_activation_counts               -   bool(no),
> +    simplify_stage_215                  -   bool(no),
>     coverage_profiling                  -   bool(no),
>     coverage_profiling_via_calls        -   bool(no),
>     profile_deep_coverage_after_goal    -   bool(yes),
> @@ -2018,6 +2024,7 @@ long_option("profile-time",         prof
> long_option("profile-memory",       profile_memory).
> long_option("profile-deep",         profile_deep).
> long_option("use-activation-counts",    use_activation_counts).
> +long_option("simplify-stage-215",   simplify_stage_215).
> long_option("coverage-profiling",
>                     coverage_profiling).
> long_option("coverage-profiling-via-calls",

There should be commented out documentation for the new options as well.
(It should be explained why it s commented out, e.g. because it is
for developer-only use.)

> Index: runtime/mercury_deep_profiling.h
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_deep_profiling.h,v
> retrieving revision 1.21
> diff -u -p -b -r1.21 mercury_deep_profiling.h
> --- runtime/mercury_deep_profiling.h	20 Sep 2008 11:38:05 -0000	1.21
> +++ runtime/mercury_deep_profiling.h	19 Feb 2009 06:29:55 -0000
> @@ -1,4 +1,7 @@
> /*
> +** vim: ts=8 sw=8 noexpandtab
> +*/
> +/*
> ** Copyright (C) 2001-2004, 2006-2008 The University of Melbourne.
> ** This file may only be copied under the terms of the GNU Library General
> ** Public License - see the file COPYING.LIB in the Mercury distribution.
>

This change is not mentioned in the log message.

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