[m-rev.] for review: --common-struct-preds

Zoltan Somogyi zs at csse.unimelb.edu.au
Mon May 26 12:45:09 AEST 2008


The main question about this diff is whether it is likely to be useful
for the future.

Zoltan.

This diff adds an option --common-struct-preds, which allows us to restrict the
set of predicates that the common struct optimization is applied to. It helped
me track down Mantis bug #56. Since common-struct optimization has had several
bugs in the past and also helped expose bugs elsewhere, I think this option
could be helpful in the future. Its cost is only a couple of option lookups
per predicate.

compiler/options.m:
	Add the option. Since the option is for implementors only, its
	documentation is deliberately commented out.

compiler/simplify.m:
	Obey the option.

compiler/common.m:
	Avoid some unnecessary memory allocations by making a predicate return
	goal_exprs and goal_infos separately when its main caller wants it that
	way.

	When replacing a construct with an assignment, preserve the context
	of the unification.

	Make some variable names more expressive.

cvs diff: Diffing .
cvs diff: Diffing analysis
cvs diff: Diffing bindist
cvs diff: Diffing boehm_gc
cvs diff: Diffing boehm_gc/Mac_files
cvs diff: Diffing boehm_gc/cord
cvs diff: Diffing boehm_gc/cord/private
cvs diff: Diffing boehm_gc/doc
cvs diff: Diffing boehm_gc/include
cvs diff: Diffing boehm_gc/include/private
cvs diff: Diffing boehm_gc/libatomic_ops-1.2
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/doc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps/gcc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps/hpc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps/ibmc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps/icc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps/msftc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps/sunc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/tests
cvs diff: Diffing boehm_gc/tests
cvs diff: Diffing boehm_gc/windows-untested
cvs diff: Diffing boehm_gc/windows-untested/vc60
cvs diff: Diffing boehm_gc/windows-untested/vc70
cvs diff: Diffing boehm_gc/windows-untested/vc71
cvs diff: Diffing browser
cvs diff: Diffing bytecode
cvs diff: Diffing compiler
Index: compiler/common.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/common.m,v
retrieving revision 1.106
diff -u -b -r1.106 common.m
--- compiler/common.m	26 Mar 2008 01:38:47 -0000	1.106
+++ compiler/common.m	22 May 2008 12:53:11 -0000
@@ -238,36 +238,36 @@
 %---------------------------------------------------------------------------%
 
 common_optimise_unification(Unification0, _Left0, _Right0, Mode, _Context,
-        Goal0, Goal, GoalInfo0, GoalInfo, !Info) :-
+        GoalExpr0, GoalExpr, GoalInfo0, GoalInfo, !Info) :-
     (
         Unification0 = construct(Var, ConsId, ArgVars, _, _, _, SubInfo),
         (
             SubInfo = construct_sub_info(MaybeTakeAddr, _),
             MaybeTakeAddr = yes(_)
         ->
-            Goal = Goal0,
+            GoalExpr = GoalExpr0,
             GoalInfo = GoalInfo0
         ;
             common_optimise_construct(Var, ConsId, ArgVars, Mode,
-                Goal0, Goal, GoalInfo0, GoalInfo, !Info)
+                GoalExpr0, GoalExpr, GoalInfo0, GoalInfo, !Info)
         )
     ;
         Unification0 = deconstruct(Var, ConsId, ArgVars, UniModes, CanFail, _),
         common_optimise_deconstruct(Var, ConsId, ArgVars, UniModes, CanFail,
-            Mode, Goal0, Goal, GoalInfo0, GoalInfo, !Info)
+            Mode, GoalExpr0, GoalExpr, GoalInfo0, GoalInfo, !Info)
     ;
         Unification0 = assign(Var1, Var2),
         record_equivalence(Var1, Var2, !Info),
-        Goal = Goal0,
+        GoalExpr = GoalExpr0,
         GoalInfo = GoalInfo0
     ;
         Unification0 = simple_test(Var1, Var2),
         record_equivalence(Var1, Var2, !Info),
-        Goal = Goal0,
+        GoalExpr = GoalExpr0,
         GoalInfo = GoalInfo0
     ;
         Unification0 = complicated_unify(_, _, _),
-        Goal = Goal0,
+        GoalExpr = GoalExpr0,
         GoalInfo = GoalInfo0
     ).
 
@@ -326,7 +326,7 @@
                 ArgVars = [_ | _],
                 UniMode = ((free - Inst) -> (Inst - Inst)),
                 generate_assign(Var, OldVar, UniMode, GoalInfo0,
-                    hlds_goal(GoalExpr, GoalInfo), !Info),
+                    GoalExpr, GoalInfo, !Info),
                 simplify_info_set_requantify(!Info),
                 goal_cost(hlds_goal(GoalExpr0, GoalInfo0), Cost),
                 simplify_info_incr_cost_delta(Cost, !Info)
@@ -501,7 +501,8 @@
 %---------------------------------------------------------------------------%
 %---------------------------------------------------------------------------%
 
-common_optimise_call(PredId, ProcId, Args, GoalInfo, Goal0, Goal, !Info) :-
+common_optimise_call(PredId, ProcId, Args, GoalInfo, GoalExpr0, GoalExpr,
+        !Info) :-
     (
         Det = goal_info_get_determinism(GoalInfo),
         check_call_detism(Det),
@@ -513,13 +514,13 @@
             InputArgs, OutputArgs, OutputModes)
     ->
         common_optimise_call_2(seen_call(PredId, ProcId), InputArgs,
-            OutputArgs, OutputModes, GoalInfo, Goal0, Goal, !Info)
+            OutputArgs, OutputModes, GoalInfo, GoalExpr0, GoalExpr, !Info)
     ;
-        Goal = Goal0
+        GoalExpr = GoalExpr0
     ).
 
 common_optimise_higher_order_call(Closure, Args, Modes, Det, GoalInfo,
-        Goal0, Goal, !Info) :-
+        GoalExpr0, GoalExpr, !Info) :-
     (
         check_call_detism(Det),
         simplify_info_get_var_types(!.Info, VarTypes),
@@ -528,9 +529,9 @@
             InputArgs, OutputArgs, OutputModes)
     ->
         common_optimise_call_2(higher_order_call, [Closure | InputArgs],
-            OutputArgs, OutputModes, GoalInfo, Goal0, Goal, !Info)
+            OutputArgs, OutputModes, GoalInfo, GoalExpr0, GoalExpr, !Info)
     ;
-        Goal = Goal0
+        GoalExpr = GoalExpr0
     ).
 
 :- pred check_call_detism(determinism::in) is semidet.
@@ -731,7 +732,7 @@
     list(prog_var)::in, list(uni_mode)::in, list(hlds_goal)::out,
     simplify_info::in, simplify_info::out) is det.
 
-create_output_unifications(GoalInfo, OutputArgs, OldOutputArgs, UniModes,
+create_output_unifications(OldGoalInfo, OutputArgs, OldOutputArgs, UniModes,
         Goals, !Info) :-
     (
         OutputArgs = [OutputArg | OutputArgsTail],
@@ -743,14 +744,15 @@
             % with a partially instantiated deconstruction.
             OutputArg \= OldOutputArg
         ->
-            generate_assign(OutputArg, OldOutputArg, UniMode, GoalInfo,
-                Goal, !Info),
-            create_output_unifications(GoalInfo,
+            generate_assign(OutputArg, OldOutputArg, UniMode, OldGoalInfo,
+                GoalExpr, GoalInfo, !Info),
+            Goal = hlds_goal(GoalExpr, GoalInfo),
+            create_output_unifications(OldGoalInfo,
                 OutputArgsTail, OldOutputArgsTail, UniModesTail,
                 GoalsTail, !Info),
             Goals = [Goal | GoalsTail]
         ;
-            create_output_unifications(GoalInfo,
+            create_output_unifications(OldGoalInfo,
                 OutputArgsTail, OldOutputArgsTail, UniModesTail, Goals, !Info)
         )
     ;
@@ -766,10 +768,11 @@
 %---------------------------------------------------------------------------%
 
 :- pred generate_assign(prog_var::in, prog_var::in, uni_mode::in,
-    hlds_goal_info::in, hlds_goal::out, simplify_info::in, simplify_info::out)
-    is det.
+    hlds_goal_info::in, hlds_goal_expr::out, hlds_goal_info::out,
+    simplify_info::in, simplify_info::out) is det.
 
-generate_assign(ToVar, FromVar, UniMode, _, Goal, !Info) :-
+generate_assign(ToVar, FromVar, UniMode, OldGoalInfo, GoalExpr, GoalInfo,
+        !Info) :-
     apply_induced_substitutions(ToVar, FromVar, !Info),
     simplify_info_get_var_types(!.Info, VarTypes),
     map.lookup(VarTypes, ToVar, ToVarType),
@@ -798,8 +801,10 @@
     % use instmap_delta_restrict on the original instmap_delta here.
     instmap_delta_from_assoc_list([ToVar - ToVarInst], InstMapDelta),
 
-    goal_info_init(NonLocals, InstMapDelta, detism_det, purity_pure, GoalInfo),
-    Goal = hlds_goal(GoalExpr, GoalInfo),
+    goal_info_init(NonLocals, InstMapDelta, detism_det, purity_pure,
+        GoalInfo0),
+    Context = goal_info_get_context(OldGoalInfo),
+    goal_info_set_context(Context, GoalInfo0, GoalInfo),
     record_equivalence(ToVar, FromVar, !Info).
 
 :- pred types_match_exactly(mer_type::in, mer_type::in) is semidet.
Index: compiler/options.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/options.m,v
retrieving revision 1.616
diff -u -b -r1.616 options.m
--- compiler/options.m	16 May 2008 08:10:51 -0000	1.616
+++ compiler/options.m	22 May 2008 13:07:23 -0000
@@ -546,6 +546,7 @@
     ;       inline_vars_threshold
     ;       intermod_inline_simple_threshold
     ;       common_struct
+    ;       common_struct_preds
     ;       common_goal
     ;       constraint_propagation
     ;       local_constraint_propagation
@@ -1357,6 +1358,7 @@
                                         % Has no effect until
                                         % --intermodule-optimization.
     common_struct                       -   bool(no),
+    common_struct_preds                 -   string(""),
     common_goal                         -   bool(yes),
                                         % common_goal is not really an
                                         % optimization, since it affects
@@ -2112,6 +2114,7 @@
                     intermod_inline_simple_threshold).
 long_option("inline-vars-threshold",        inline_vars_threshold).
 long_option("common-struct",        common_struct).
+long_option("common-struct-preds",  common_struct_preds).
 long_option("common-goal",          common_goal).
 long_option("excess-assign",        excess_assign).
 long_option("optimize-duplicate-calls", optimize_duplicate_calls).
@@ -4399,6 +4402,8 @@
         "\tslow compilation.",
         "--no-common-struct",
         "\tDisable optimization of common term structures.",
+%       "--common-struct-preds <predids>",
+%       "\tLimit common struct optimization to the preds with the given ids.",
 
 %       Common goal optimization should not be turned off, since it can
 %       break programs that would otherwise compile properly (e.g.,
Index: compiler/simplify.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/simplify.m,v
retrieving revision 1.229
diff -u -b -r1.229 simplify.m
--- compiler/simplify.m	26 Mar 2008 01:38:47 -0000	1.229
+++ compiler/simplify.m	22 May 2008 13:15:52 -0000
@@ -374,9 +374,29 @@
         % either the compiler runs out of memory or the user runs out of
         % patience. The fact that we would generate better code if the
         % compilation finished is therefore of limited interest.
-        Simplifications = Simplifications0 ^ do_common_struct := no
+        Simplifications1 = Simplifications0 ^ do_common_struct := no
     ;
-        Simplifications = Simplifications0
+        Simplifications1 = Simplifications0
+    ),
+    module_info_get_globals(!.ModuleInfo, Globals),
+    globals.lookup_string_option(Globals, common_struct_preds,
+        CommonStructPreds),
+    ( CommonStructPreds = "" ->
+        Simplifications = Simplifications1
+    ;
+        CommonStructPredIdStrs = string.split_at_char(',', CommonStructPreds),
+        (
+            list.map(string.to_int, CommonStructPredIdStrs,
+                CommonStructPredIdInts)
+        ->
+            ( list.member(pred_id_to_int(PredId), CommonStructPredIdInts) ->
+                Simplifications = Simplifications1
+            ;
+                Simplifications = Simplifications1 ^ do_common_struct := no
+            )
+        ;
+            Simplifications = Simplifications1
+        )
     ),
     det_info_init(!.ModuleInfo, VarTypes0, PredId, ProcId,
         pess_extra_vars_report, DetInfo0),
cvs diff: Diffing compiler/notes
cvs diff: Diffing debian
cvs diff: Diffing debian/patches
cvs diff: Diffing deep_profiler
cvs diff: Diffing deep_profiler/notes
cvs diff: Diffing doc
cvs diff: Diffing extras
cvs diff: Diffing extras/base64
cvs diff: Diffing extras/cgi
cvs diff: Diffing extras/complex_numbers
cvs diff: Diffing extras/complex_numbers/samples
cvs diff: Diffing extras/complex_numbers/tests
cvs diff: Diffing extras/concurrency
cvs diff: Diffing extras/curs
cvs diff: Diffing extras/curs/samples
cvs diff: Diffing extras/curses
cvs diff: Diffing extras/curses/sample
cvs diff: Diffing extras/dynamic_linking
cvs diff: Diffing extras/error
cvs diff: Diffing extras/fixed
cvs diff: Diffing extras/gator
cvs diff: Diffing extras/gator/generations
cvs diff: Diffing extras/gator/generations/1
cvs diff: Diffing extras/graphics
cvs diff: Diffing extras/graphics/easyx
cvs diff: Diffing extras/graphics/easyx/samples
cvs diff: Diffing extras/graphics/mercury_allegro
cvs diff: Diffing extras/graphics/mercury_allegro/examples
cvs diff: Diffing extras/graphics/mercury_allegro/samples
cvs diff: Diffing extras/graphics/mercury_allegro/samples/demo
cvs diff: Diffing extras/graphics/mercury_allegro/samples/mandel
cvs diff: Diffing extras/graphics/mercury_allegro/samples/pendulum2
cvs diff: Diffing extras/graphics/mercury_allegro/samples/speed
cvs diff: Diffing extras/graphics/mercury_glut
cvs diff: Diffing extras/graphics/mercury_opengl
cvs diff: Diffing extras/graphics/mercury_tcltk
cvs diff: Diffing extras/graphics/samples
cvs diff: Diffing extras/graphics/samples/calc
cvs diff: Diffing extras/graphics/samples/gears
cvs diff: Diffing extras/graphics/samples/maze
cvs diff: Diffing extras/graphics/samples/pent
cvs diff: Diffing extras/lazy_evaluation
cvs diff: Diffing extras/lex
cvs diff: Diffing extras/lex/samples
cvs diff: Diffing extras/lex/tests
cvs diff: Diffing extras/log4m
cvs diff: Diffing extras/logged_output
cvs diff: Diffing extras/moose
cvs diff: Diffing extras/moose/samples
cvs diff: Diffing extras/moose/tests
cvs diff: Diffing extras/mopenssl
cvs diff: Diffing extras/morphine
cvs diff: Diffing extras/morphine/non-regression-tests
cvs diff: Diffing extras/morphine/scripts
cvs diff: Diffing extras/morphine/source
cvs diff: Diffing extras/net
cvs diff: Diffing extras/odbc
cvs diff: Diffing extras/posix
cvs diff: Diffing extras/posix/samples
cvs diff: Diffing extras/quickcheck
cvs diff: Diffing extras/quickcheck/tutes
cvs diff: Diffing extras/references
cvs diff: Diffing extras/references/samples
cvs diff: Diffing extras/references/tests
cvs diff: Diffing extras/solver_types
cvs diff: Diffing extras/solver_types/library
cvs diff: Diffing extras/trailed_update
cvs diff: Diffing extras/trailed_update/samples
cvs diff: Diffing extras/trailed_update/tests
cvs diff: Diffing extras/windows_installer_generator
cvs diff: Diffing extras/windows_installer_generator/sample
cvs diff: Diffing extras/windows_installer_generator/sample/images
cvs diff: Diffing extras/xml
cvs diff: Diffing extras/xml/samples
cvs diff: Diffing extras/xml_stylesheets
cvs diff: Diffing java
cvs diff: Diffing java/runtime
cvs diff: Diffing library
cvs diff: Diffing mdbcomp
cvs diff: Diffing profiler
cvs diff: Diffing robdd
cvs diff: Diffing runtime
cvs diff: Diffing runtime/GETOPT
cvs diff: Diffing runtime/machdeps
cvs diff: Diffing samples
cvs diff: Diffing samples/c_interface
cvs diff: Diffing samples/c_interface/c_calls_mercury
cvs diff: Diffing samples/c_interface/cplusplus_calls_mercury
cvs diff: Diffing samples/c_interface/mercury_calls_c
cvs diff: Diffing samples/c_interface/mercury_calls_cplusplus
cvs diff: Diffing samples/c_interface/mercury_calls_fortran
cvs diff: Diffing samples/c_interface/simpler_c_calls_mercury
cvs diff: Diffing samples/c_interface/simpler_cplusplus_calls_mercury
cvs diff: Diffing samples/c_interface/standalone_c
cvs diff: Diffing samples/diff
cvs diff: Diffing samples/muz
cvs diff: Diffing samples/rot13
cvs diff: Diffing samples/solutions
cvs diff: Diffing samples/solver_types
cvs diff: Diffing samples/tests
cvs diff: Diffing samples/tests/c_interface
cvs diff: Diffing samples/tests/c_interface/c_calls_mercury
cvs diff: Diffing samples/tests/c_interface/cplusplus_calls_mercury
cvs diff: Diffing samples/tests/c_interface/mercury_calls_c
cvs diff: Diffing samples/tests/c_interface/mercury_calls_cplusplus
cvs diff: Diffing samples/tests/c_interface/mercury_calls_fortran
cvs diff: Diffing samples/tests/c_interface/simpler_c_calls_mercury
cvs diff: Diffing samples/tests/c_interface/simpler_cplusplus_calls_mercury
cvs diff: Diffing samples/tests/diff
cvs diff: Diffing samples/tests/muz
cvs diff: Diffing samples/tests/rot13
cvs diff: Diffing samples/tests/solutions
cvs diff: Diffing samples/tests/toplevel
cvs diff: Diffing scripts
cvs diff: Diffing slice
cvs diff: Diffing ssdb
cvs diff: Diffing tests
cvs diff: Diffing tests/benchmarks
cvs diff: Diffing tests/debugger
cvs diff: Diffing tests/debugger/declarative
cvs diff: Diffing tests/dppd
cvs diff: Diffing tests/general
cvs diff: Diffing tests/general/accumulator
cvs diff: Diffing tests/general/string_format
cvs diff: Diffing tests/general/structure_reuse
cvs diff: Diffing tests/grade_subdirs
cvs diff: Diffing tests/hard_coded
cvs diff: Diffing tests/hard_coded/exceptions
cvs diff: Diffing tests/hard_coded/purity
cvs diff: Diffing tests/hard_coded/sub-modules
cvs diff: Diffing tests/hard_coded/typeclasses
cvs diff: Diffing tests/invalid
cvs diff: Diffing tests/invalid/purity
cvs diff: Diffing tests/misc_tests
cvs diff: Diffing tests/mmc_make
cvs diff: Diffing tests/mmc_make/lib
cvs diff: Diffing tests/par_conj
cvs diff: Diffing tests/recompilation
cvs diff: Diffing tests/tabling
cvs diff: Diffing tests/term
cvs diff: Diffing tests/trailing
cvs diff: Diffing tests/valid
cvs diff: Diffing tests/warnings
cvs diff: Diffing tools
cvs diff: Diffing trace
cvs diff: Diffing util
cvs diff: Diffing vim
cvs diff: Diffing vim/after
cvs diff: Diffing vim/ftplugin
cvs diff: Diffing vim/syntax
--------------------------------------------------------------------------
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