[m-rev.] for review: dependent parallel conjunctions (1/2)

[Julien Fischer]

On Thu, 22 Jun 2006, Peter Wang wrote:

> Estimated hours taken: 40
> Branches: main
>
> This patch adds preliminary support for deterministic, dependent
> parallel conjunctions to the low-level backend.  In other backends
> dependent parallel conjunctions are converted into plain conjunctions.
>
> For a parallel conjunction (A & B), if the goal B is dependent on a variable
> X which is bound by goal A, we transform the conjunction such that
> goal B must wait for the value to be produced by A before it begins
> executing.  This transformation is not yet useful in practice (you might as
> well use a sequential conjunction).  A later version of this transformation
> will move the synchronisation deeper into the goals so that B can execute as
> much as possible before it waits for the value from A.
>
> There is no coroutining support yet so if there are not enough threads
> available then dependent parallel conjunctions can cause the program to
> deadlock.
>
>
> configure.in:
> runtime/mercury_conf.h.in:
> 	Check for existence of semaphore.h and #define MR_HAVE_SEMAPHORE_H
> 	if it does.
>
> library/library.m:
> library/par_builtin.m:
> library/private_builtin.m:
> mdbcomp/prim_data.m:
> mdbcomp/program_representation.m:
> 	Add a new module `par_builtin' to hold synchronisation primitives.
>
> compiler/modules.m:
> 	Import `par_builtin' module in parallel grades.
>
> compiler/dep_par_conj.m:
> compiler/transform_hlds.m:
> compiler/mercury_compile.m:
> 	Add a transformation to detect dependent parallel conjunctions
> 	and insert the necessary synchronisation code.
>
> compiler/hlds_goal.m:
> 	Consider empty parallel conjunctions as atomic, the same as plain
> 	conjunctions.
>
> compiler/inlining.m:
> 	Flatten parallel conjunctions when inlining, as for plain conjunctions.
>
> compiler/live_vars.m:
> 	Fix build_live_sets_in_par_conj to handle dependent parallel
> 	conjunctions.
>
> compiler/liveness.m:
> 	Delay deaths in parallel conjunctions the same way as for plain
> 	conjunctions.
>
> 	Update detect_resume_points_in_par_conj for dependent parallel
> 	conjunctions.
>
> compiler/mode_util.m:
> compiler/modes.m:
> compiler/unique_modes.m:
> 	Update mode, uniqueness checking for dependent parallel conjunctions.

Please provide more details about how the mode checking of parallel
conjunctions has changed here.

> compiler/simplify.m:
> 	Don't reset instmaps and seen calls after each conjunct of a parallel
> 	conjunction.
>
> compiler/store_alloc.m:
> 	Update allocation for dependent parallel conjunctions.
>
> compiler/switch_detection.m:
> 	Detect switches in parallel conjunctions the same as plain
> 	conjunctions.
>
> compiler/par_conj_gen.m:
> 	Add a todo comment.
...

> Index: compiler/dep_par_conj.m
> ===================================================================
> RCS file: compiler/dep_par_conj.m
> diff -N compiler/dep_par_conj.m
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ compiler/dep_par_conj.m	22 Jun 2006 04:04:20 -0000
> @@ -0,0 +1,544 @@
> +%-----------------------------------------------------------------------------%
> +% vim: ft=mercury ts=8 sw=4 et
> +%-----------------------------------------------------------------------------%
> +% Copyright (C) 2005-2006 The University of Melbourne.
> +% This file may only be copied under the terms of the GNU General
> +% Public License - see the file COPYING in the Mercury distribution.
> +%-----------------------------------------------------------------------------%

2005?

> +
> +% File: dep_par_conj.m.
> +% Author: wangp.
> +

A little more description would help here, e.g.

	This module transforms the HLDS to implement dependent parallel
	conjunction.  The transformation involves adding calls to the
	predicates defined in library/par_builtin.m.

	... then describe the transformation as below

> +% For a parallel conjunction (A & B), if the goal B is dependent on a variable
> +% X which is bound by goal A, we transform the conjunction into the following:
> +%
> +%    par_builtin.new_promise(PromiseX),
> +%    (
> +%        (
> +%            A(X),  % binds X
> +%            impure par_builtin.signal(PromiseX, X)
> +%        )
> +%    &
> +%        (
> +%            par_builtin.wait(PromiseX, X1),
> +%            B(X1)  % uses X
> +%        )
> +%    )
> +%
> +% That is, goal B must wait for the value to be produced by A before it begins
> +% executing.  This transformation is not just useful in practice (you might as

Delete "just".

> +% well use a sequential conjunction).  A later version of this transformation
> +% will move signal and wait calls into goals to, hopefully, actually allow
> +% parallelism.
> +%
> +% If building in a non-parallel grade then dependent parallel conjunctions
> +% are simply converted into sequential conjunctions.
> +

There should be a todo list at the head of this module mentioning several of
the things above.

...

> +:- pred search_goal_for_par_conj(module_info::in,
> +    prog_varset::in, prog_varset::out, vartypes::in, vartypes::out,
> +    instmap::in, instmap::out, hlds_goal::in, hlds_goal::out,
> +    io::di, io::uo) is det.
> +
> +search_goal_for_par_conj(ModuleInfo, !VarSet, !VarTypes, InstMap0, InstMap,
> +        Goal0, Goal, !IO) :-
> +    search_goal_for_par_conj_2(ModuleInfo, !VarSet, !VarTypes, InstMap0,
> +        Goal0, Goal, !IO),
> +    update_instmap(Goal0, InstMap0, InstMap).
> +
> +:- pred search_goal_for_par_conj_2(module_info::in,
> +    prog_varset::in, prog_varset::out, vartypes::in, vartypes::out,
> +    instmap::in, hlds_goal::in, hlds_goal::out, io::di, io::uo) is det.
> +
> +search_goal_for_par_conj_2(_ModuleInfo, !VarSet, !VarTypes, _InstMap,
> +        G @ (Goal - _), G, !IO) :-
> +    Goal = unify(_, _, _, _Kind, _).
> +search_goal_for_par_conj_2(_ModuleInfo, !VarSet, !VarTypes, _InstMap,
> +        G @ (Goal - _), G, !IO) :-
> +    Goal = call(_CallPredId, _CallProcId, _CallArgs, _, _, _).
> +search_goal_for_par_conj_2(_ModuleInfo, !VarSet, !VarTypes, _InstMap,
> +        G @ (Goal - _), G, !IO) :-
> +    Goal = generic_call(_Details, _Args, _ArgModes, _).
> +search_goal_for_par_conj_2(ModuleInfo, !VarSet, !VarTypes, InstMap,
> +        not(Goal0) - GI, not(Goal) - GI, !IO) :-
> +    search_goal_for_par_conj(ModuleInfo, !VarSet, !VarTypes, InstMap, _,
> +        Goal0, Goal, !IO).
> +search_goal_for_par_conj_2(ModuleInfo, !VarSet, !VarTypes, InstMap,
> +        Goal0 - GI, Goal - GI, !IO) :-
> +    Goal0 = scope(Reason, ScopeGoal0),
> +    search_goal_for_par_conj(ModuleInfo, !VarSet, !VarTypes, InstMap, _,
> +        ScopeGoal0, ScopeGoal, !IO),
> +    Goal = scope(Reason, ScopeGoal).
> +search_goal_for_par_conj_2(_ModuleInfo, !VarSet, !VarTypes, _InstMap,
> +        G @ (Goal - _), G, !IO) :-
> +    Goal = foreign_proc(_Attributes, _, _, _, _, _).
> +search_goal_for_par_conj_2(_, _,_, _,_, _, shorthand(_) - _, _, _,_) :-
> +    unexpected(this_file,
> +        "shorthand goal encountered during dependent parallel " ++
> +        "conjunction transformation.").
> +search_goal_for_par_conj_2(ModuleInfo, !VarSet, !VarTypes, InstMap,
> +        Goal0 - GI, Goal - GI, !IO) :-
> +    Goal0 = switch(Var, CanFail, Cases0),
> +    search_cases_for_par_conj(ModuleInfo, !VarSet, !VarTypes, InstMap,
> +        Cases0, Cases, !IO),
> +    Goal = switch(Var, CanFail, Cases).
> +search_goal_for_par_conj_2(ModuleInfo, !VarSet, !VarTypes, InstMap0,
> +        Goal0 - GI, Goal - GI, !IO) :-
> +    Goal0 = if_then_else(Quant, If0, Then0, Else0),
> +    search_goal_for_par_conj(ModuleInfo, !VarSet, !VarTypes,
> +        InstMap0, InstMap1, If0, If, !IO),
> +    search_goal_for_par_conj(ModuleInfo, !VarSet, !VarTypes,
> +        InstMap1, _InstMap2, Then0, Then, !IO),
> +    search_goal_for_par_conj(ModuleInfo, !VarSet, !VarTypes,
> +        InstMap0, _InstMap3, Else0, Else, !IO),
> +    Goal = if_then_else(Quant, If, Then, Else).
> +search_goal_for_par_conj_2(ModuleInfo, !VarSet, !VarTypes, InstMap,
> +        Goal0 - GI, Goal - GI, !IO) :-
> +    Goal0 = disj(Goals0),
> +    search_goals_for_par_conj(ModuleInfo, !VarSet, !VarTypes, InstMap,
> +        Goals0, Goals, !IO),
> +    Goal = disj(Goals).
> +search_goal_for_par_conj_2(ModuleInfo, !VarSet, !VarTypes, InstMap,
> +        Goal0 - GI, Goal, !IO) :-
> +    Goal0 = conj(ConjType, Goals0),
> +    (
> +        ConjType = plain_conj,
> +        search_goals_for_par_conj(ModuleInfo, !VarSet, !VarTypes, InstMap,
> +            Goals0, Goals, !IO),
> +        conj_list_to_goal(Goals, GI, Goal)
> +    ;
> +	ConjType = parallel_conj,
> +	munge_par_conj(ModuleInfo, !VarSet, !VarTypes, InstMap,
> +            Goals0, GI, Goal, !IO)

That needs to be indented another level.

...

> +    % We found a parallel conjunction.  We have to check if there are
> +    % dependencies between the conjuncts and insert the synchronisation.
> +    %

How about:

	% We have found a parllel conjunction.  We need to check there
	% any dependencies between the conjuncts and, if so, insert
	% sychronisation primitives.

> +:- pred munge_par_conj(module_info::in, prog_varset::in, prog_varset::out,
> +    vartypes::in, vartypes::out, instmap::in,
> +    hlds_goals::in, hlds_goal_info::in, hlds_goal::out, io::di, io::uo) is det.
> +

I suggest calling that maybe_transform_par_conj, rather than munge_par_conj.

> +munge_par_conj(ModuleInfo, !VarSet, !VarTypes, InstMap,
> +        Conjuncts0, GoalInfo, NewGoal, !IO) :-
> +    % Search subgoals for nested parallel conjunctions.
> +    search_goals_for_par_conj(ModuleInfo, !VarSet, !VarTypes, InstMap,
> +        Conjuncts0, Conjuncts, !IO),
> +
> +    % Find the variables that are shared between conjunctions.

s/conjunctions/conjuncts/

> +    find_shared_variables(ModuleInfo, InstMap, Conjuncts, SharedVars),
> +
> +    % Dependent parallel conjunctions only supported on lowlevel C parallel
> +    % grades.  For other grades convert any dependent parallel conjunctions
> +    % into plain conjunctions.
> +    globals.io_get_target(Target, !IO),
> +    globals.io_lookup_bool_option(highlevel_code, HighLevelCode, !IO),
> +    globals.io_lookup_bool_option(parallel, Parallel, !IO),
> +    (if
> +        set.non_empty(SharedVars),
> +        Target = target_c,
> +        HighLevelCode = no,
> +        Parallel = yes
> +    then
> +        transform_conjunction(ModuleInfo, SharedVars,
> +            Conjuncts, GoalInfo, NewGoal, InstMap, !VarSet, !VarTypes)
> +    else
> +        conj_list_to_goal(Conjuncts, GoalInfo, NewGoal)
> +    ).
> +
> +    % Transforming the parallel conjunction.
> +    %
> +    % As a simple first step, all we do is insert waits and signals directly
> +    % into the conjunction.  If a conjunct produces a shared variable we turn
> +    % that conjunct into a sequential conjunction:
> +    %
> +    %   prod(Y)  ==>  ( prod(Y), impure signal(PrY, Y) )
> +    %
> +    % If the conjunct consumes a shared variable we will turn that conjunct
> +    % into a sequential conjunction:
> +    %
> +    %   consume(Y)  ==>  ( wait(PrY, Y), consume(Y) )
> +    %

I think it would be worth documenting the transformtion for an actual
conjunction as well as specifying the transformation for individual conjuncts.

> +    % References to shared variables need to be renamed apart so that the
> +    % conjuncts only share promises.
> +    %
> +:- pred transform_conjunction(module_info::in, set(prog_var)::in,
> +    hlds_goals::in, hlds_goal_info::in, hlds_goal::out, instmap::in,
> +    prog_varset::in, prog_varset::out, vartypes::in, vartypes::out) is det.
> +
> +transform_conjunction(ModuleInfo, SharedVars, Goals, GoalInfo, NewGoal,
> +        InstMap, !VarSet, !VarTypes) :-
> +    allocate_promises(ModuleInfo, SharedVars, !VarTypes, !VarSet,
> +        AllocatePromises, PromiseMap),
> +    list.map_foldl3(transform_conjunct(ModuleInfo, SharedVars, PromiseMap),
> +        Goals, NewGoals, InstMap, _, !VarSet, !VarTypes),
> +    % XXX not sure about GoalInfo

What are you not sure about the GoalInfo?

> +    Conj = AllocatePromises ++ [conj(parallel_conj, NewGoals) - GoalInfo],
> +    conj_list_to_goal(Conj, GoalInfo, NewGoal).
> +
> +:- pred transform_conjunct(module_info::in, set(prog_var)::in,
> +    promise_map::in, hlds_goal::in, hlds_goal::out,
> +    instmap::in, instmap::out, prog_varset::in, prog_varset::out,
> +    vartypes::in, vartypes::out) is det.
> +
> +transform_conjunct(ModuleInfo, SharedVars, PromiseMap, Goal0, Goal,
> +        !InstMap, !VarSet, !VarTypes) :-
> +    goal_get_nonlocals(Goal0, Nonlocals),
> +    set.intersect(Nonlocals, SharedVars, Intersect),
> +    (if set.empty(Intersect) then
> +        Goal = Goal0,
> +        update_instmap(Goal, !InstMap)
> +    else
> +        Goal0 = (_ - GoalInfo0),
> +        goal_info_get_instmap_delta(GoalInfo0, InstMapDelta0),
> +
> +        % Divide shared variables into those that are produced by this
> +        % conjunction, and those that are consumed.
> +        set.divide(produced_variable(ModuleInfo, !.InstMap, InstMapDelta0),
> +            Intersect, ProducedVars, ConsumedVars),
> +
> +        % Rename references to consumed variables.  We let the producer keep
> +        % the original name.  This helps because there may be references to
> +        % the original name following the parallel conjunction.

That last bit should be something like:

	We let the producer keep the original name in order to avoid having
	to rename references to the original name following the parallel
	conjunction.

...

> +    % If a variable is nonlocal to a conjunct, and appears in the
> +    % instmap_delta of a _different_ conjunct, then we say that variable is
> +    % shared.
> +    %
> +    % (1) A variable must be nonlocal to a conjunct if it is shared.
> +    % (2) If the variable does not appear in the instmap_delta
> +    %     of any of the conjuncts of the parallel conjunction
> +    %     then it could not have been further instantiated within
> +    %     by the conjunction as a whole.
> +    %
> +    % XXX this code is probably too complicated.  I think Thomas already had a
> +    % more elegant way to find the shared variables somewhere, using multisets.
> +    %
> +:- pred find_shared_variables(module_info::in, instmap::in, hlds_goals::in,
> +    set(prog_var)::out) is det.
> +
> +find_shared_variables(ModuleInfo, InstMap, Goals, SharedVars) :-
> +    list.map2(get_nonlocals_and_instmaps, Goals, Nonlocals, InstMapDeltas),
> +    find_shared_variables_2(ModuleInfo, 0, Nonlocals, InstMap, InstMapDeltas,
> +        set.init, SharedVars).
> +
> +:- pred get_nonlocals_and_instmaps(hlds_goal::in,
> +    set(prog_var)::out, instmap_delta::out) is det.
> +
> +get_nonlocals_and_instmaps(_Goal - GoalInfo, Nonlocals, InstMapDelta) :-
> +    goal_info_get_nonlocals(GoalInfo, Nonlocals),
> +    goal_info_get_instmap_delta(GoalInfo, InstMapDelta).
> +
> +:- pred find_shared_variables_2(module_info::in, int::in,
> +    list(set(prog_var))::in, instmap::in, list(instmap_delta)::in,
> +    set(prog_var)::in, set(prog_var)::out) is det.
> +
> +find_shared_variables_2(_ModuleInfo, _ConjunctIndex,
> +        [], _InstMap, _InstMapDeltas, !SharedVars).
> +find_shared_variables_2(ModuleInfo, ConjunctIndex,
> +        [Nonlocals | MoreNonlocals], InstMap, InstMapDeltas, !SharedVars) :-
> +    det_delete_nth(ConjunctIndex, InstMapDeltas, InstMapDeltasB),
> +    % Keep only nonlocals which were not already bound at the start of the
> +    % parallel conjunction.
> +    Filter = (pred(Var::in) is semidet :-
> +        not (
> +            instmap.lookup_var(InstMap, Var, VarInst),
> +            inst_is_bound(ModuleInfo, VarInst)
> +        )
> +    ),
> +    set.filter(Filter, Nonlocals) = UnboundNonlocals,
> +    set.fold(find_changed_vars(ModuleInfo, InstMapDeltasB),
> +        UnboundNonlocals, !SharedVars),
> +    find_shared_variables_2(ModuleInfo, ConjunctIndex+1, MoreNonlocals,
> +        InstMap, InstMapDeltas, !SharedVars).
> +
> +:- pred find_changed_vars(module_info::in, list(instmap_delta)::in,
> +    prog_var::in, set(prog_var)::in, set(prog_var)::out) is det.
> +
> +find_changed_vars(ModuleInfo, InstMapDeltas, UnboundVar, !SharedVars) :-
> +    (if
> +        % Is the unbound nonlocal bound in one of the conjuncts?
> +        InstMapDelta `member` InstMapDeltas,


s/member/list.member/
(There may be ambiguity problems building the compiler with
--intermodule-optization otherwise.)

> +        instmap_delta_search_var(InstMapDelta, UnboundVar, Inst),
> +        inst_is_bound(ModuleInfo, Inst)
> +    then
> +        svset.insert(UnboundVar, !SharedVars)
> +    else
> +        true
> +    ).
> +
> +%-----------------------------------------------------------------------------%
> +
> +:- type promise_map == map(prog_var, prog_var).

What does this type do?  Please add a comment.

> +    % Make goals to allocate promise objects for variables shared
> +    % between two parallel conjuncts (a producer and one or more consumers).
> +    %
> +:- pred allocate_promises(module_info::in, set(prog_var)::in,
> +    vartypes::in, vartypes::out, prog_varset::in, prog_varset::out,
> +    hlds_goals::out, promise_map::out) is det.
> +
> +allocate_promises(ModuleInfo, SharedVars, !VarTypes, !VarSet,
> +        AllocGoals, PromiseMap) :-
> +    set.fold4(allocate_promise(ModuleInfo), SharedVars,
> +        !VarTypes, !VarSet, [], AllocGoals, map.init, PromiseMap).
> +
> +:- pred allocate_promise(module_info::in, prog_var::in,
> +    vartypes::in, vartypes::out, prog_varset::in, prog_varset::out,
> +    hlds_goals::in, hlds_goals::out, promise_map::in, promise_map::out) is det.
> +
> +allocate_promise(ModuleInfo, SharedVar, !VarTypes, !VarSet,
> +        !AllocGoals, !PromiseMap) :-
> +    map.lookup(!.VarTypes, SharedVar, SharedVarType),
> +    make_promise(ModuleInfo, SharedVarType, SharedVar, !VarTypes, !VarSet,
> +        AllocGoal, PromiseVar),
> +    list.cons(AllocGoal, !AllocGoals),
> +    svmap.det_insert(SharedVar, PromiseVar, !PromiseMap).
> +
> +:- pred make_promise(module_info::in, mer_type::in, prog_var::in, vartypes::in,
> +    vartypes::out, prog_varset::in, prog_varset::out, hlds_goal::out,
> +    prog_var::out) is det.
> +
> +make_promise(ModuleInfo, SharedVarType, SharedVar, !VarTypes, !VarSet,
> +        AllocGoal, PromiseVar) :-
> +    construct_promise_type(SharedVarType, PromiseType),
> +    varset.lookup_name(!.VarSet, SharedVar, SharedVarName),
> +    svvarset.new_named_var("Promise" ++ SharedVarName, PromiseVar, !VarSet),
> +    svmap.det_insert(PromiseVar, PromiseType, !VarTypes),
> +
> +    ModuleName = mercury_par_builtin_module,
> +    PredName = "new_promise",
> +    Args = [PromiseVar],
> +    Features = [],
> +    InstMapSrc = [PromiseVar - ground(shared, none)],
> +    Context = term.context_init,
> +    goal_util.generate_simple_call(ModuleName, PredName, predicate,
> +        only_mode, det, Args, Features, InstMapSrc, ModuleInfo,
> +        Context, AllocGoal).
> +
> +    % Construct type promise(T) given type T.
> +    %

In par_builtin the type is called prom(T) so fix the above comment.

> +:- pred construct_promise_type(mer_type::in, mer_type::out) is det.
> +
> +construct_promise_type(T, PromiseT) :-
> +    Prom = qualified(mercury_private_builtin_module, "prom"),
> +    PromiseCtor = type_ctor(Prom, 1),
> +    construct_type(PromiseCtor, [T], PromiseT).
> +
> +%-----------------------------------------------------------------------------%
> +
> +:- pred det_delete_nth(int::in, list(T)::in, list(T)::out) is det.
> +
> +det_delete_nth(N, List0, List) :-
> +    list.det_split_list(N, List0, Left, Right),
> +    List = Left ++ det_tail(Right).

Perhaps one for the standard library?

...

> Index: compiler/mercury_compile.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/mercury_compile.m,v
> retrieving revision 1.391
> diff -u -r1.391 mercury_compile.m
> --- compiler/mercury_compile.m	14 Jun 2006 08:14:49 -0000	1.391
> +++ compiler/mercury_compile.m	21 Jun 2006 07:04:21 -0000
> @@ -94,6 +94,7 @@
>  :- import_module transform_hlds.ctgc.structure_reuse.analysis.
>  :- import_module transform_hlds.ctgc.structure_sharing.
>  :- import_module transform_hlds.ctgc.structure_sharing.analysis.
> +:- import_module transform_hlds.dep_par_conj.
>  :- import_module transform_hlds.size_prof.
>  :- import_module ll_backend.deep_profiling.
>
> @@ -2431,6 +2432,9 @@
>
>      maybe_structure_reuse_analysis(Verbose, Stats, !HLDS, !IO),
>      maybe_dump_hlds(!.HLDS, 212, "structure_reuse", !DumpInfo, !IO),
> +
> +    maybe_dependent_par_conj(Verbose, Stats, !HLDS, !IO),
> +    maybe_dump_hlds(!.HLDS, 214, "dependent_par_conj", !DumpInfo, !IO),

Is this the right spot for this?  Can it be performed any earlier or must it
be done here?

(Note: this occurs well after inlining so none of the calls to the service
predicates in par_builtin will be inlined.  At this stage I don't think it's
important, but you add a comment at the top of dep_par_conj.m that it should
be looked at again in the future.)

...

> @@ -1286,43 +1286,16 @@
>              Goal = conj(plain_conj, [])
>          ;
>              Goals0 = [_ | _],
> -            modecheck_conj_list(Goals0, Goals, !ModeInfo, !IO),
> +            modecheck_conj_list(ConjType, Goals0, Goals, !ModeInfo, !IO),
>              conj_list_to_goal(Goals, GoalInfo0, Goal - _GoalInfo)
>          ),
>          mode_checkpoint(exit, "conj", !ModeInfo, !IO)
>      ;
>          ConjType = parallel_conj,
> -        % To modecheck a parallel conjunction, we modecheck each
> -        % conjunct independently (just like for disjunctions).
> -        % To make sure that we don't try to bind a variable more than
> -        % once (by binding it in more than one conjunct), we maintain a
> -        % datastructure that keeps track of three things:
> -        %
> -        % - the set of variables that are nonlocal to the conjuncts
> -        %   (which may be a superset of the nonlocals of the par_conj
> -        %   as a whole);
> -        % - the set of nonlocal variables that have been bound in the
> -        %   current conjunct; and
> -        % - the set of variables that were bound in previous conjuncts.
> -        %
> -        % When binding a variable, we check that it wasn't in the set of
> -        % variables bound in other conjuncts, and we add it to the set of
> -        % variables bound in this conjunct.
> -        %
> -        % At the end of the conjunct, we add the set of variables bound in
> -        % this conjunct to the set of variables bound in previous conjuncts
> -        % and set the set of variables bound in the current conjunct to
> -        % empty.
> -        %
> -        % A stack of these structures is maintained to handle nested parallel
> -        % conjunctions properly.
> -        %
>          mode_checkpoint(enter, "par_conj", !ModeInfo, !IO),
> -        goal_info_get_nonlocals(GoalInfo0, NonLocals),
> -        modecheck_par_conj_list(Goals0, Goals, NonLocals, InstMapNonlocalList,
> -            !ModeInfo, !IO),
> -        Goal = conj(parallel_conj, Goals),
> -        instmap_unify(NonLocals, InstMapNonlocalList, !ModeInfo),
> +        % empty parallel conjunction should not be a common case

Sentences begin with a capital letter and end with a full stop.

> +        modecheck_conj_list(ConjType, Goals0, Goals, !ModeInfo, !IO),
> +        par_conj_list_to_goal(Goals, GoalInfo0, Goal - _GoalInfo),
>          mode_checkpoint(exit, "par_conj", !ModeInfo, !IO)
>      ).
>
> @@ -1750,7 +1723,7 @@
>          % is not, the main unification will be delayed until after the
>          % argument unifications, which turns them into assignments,
>          % and we end up repeating the process forever.
> -        mode_info_add_goals_live_vars(GoalList0, !ModeInfo),
> +        mode_info_add_goals_live_vars(plain_conj, GoalList0, !ModeInfo),
>          modecheck_conj_list_no_delay(GoalList0, GoalList, !ModeInfo, !IO),
>          Goal = conj(plain_conj, GoalList),
>          mode_info_set_checking_extra_goals(no, !ModeInfo),

...

>  :- type impurity_errors == list(mode_error_info).
>
> -    % Flatten conjunctions as we go.  Call modecheck_conj_list_3 to do
> -    % the actual scheduling.
> +    % Flatten conjunctions as we go, as long as they are of the same type).

There's an unmatched parenthesis in that comment.

...

> Index: compiler/modules.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/modules.m,v
> retrieving revision 1.394
> diff -u -r1.394 modules.m
> --- compiler/modules.m	15 Jun 2006 19:37:05 -0000	1.394
> +++ compiler/modules.m	21 Jun 2006 06:35:35 -0000
> @@ -2754,6 +2754,7 @@
>      mercury_table_builtin_module(MercuryTableBuiltin),
>      mercury_profiling_builtin_module(MercuryProfilingBuiltin),
>      mercury_term_size_prof_builtin_module(MercuryTermSizeProfBuiltin),
> +    mercury_par_builtin_module(MercuryParBuiltin),
>      !:ImportDeps = [MercuryPublicBuiltin | !.ImportDeps],
>      !:UseDeps = [MercuryPrivateBuiltin | !.UseDeps],
>      (
> @@ -2793,6 +2794,13 @@
>          !:UseDeps = [MercuryTermSizeProfBuiltin | !.UseDeps]
>      ;
>          true
> +    ),
> +    globals.lookup_bool_option(Globals, parallel, Parallel),
> +    (
> +        Parallel = yes,
> +        !:UseDeps = [MercuryParBuiltin | !.UseDeps]
> +    ;
> +        Parallel = no
>      ).

Won't that also implicitly import the par_builtin module in the high-level
parallel grades?  (I thought that par_builtin was only needed by the
low-level grades.)

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
--------------------------------------------------------------------------