[m-rev.] for post-commit review: Coverage Profiling completed.

Zoltan Somogyi zs at csse.unimelb.edu.au
Fri Sep 26 17:06:19 AEST 2008


I have reviewed the changes to deep_profiling.m, as well as some code
that was already there. I made a bunch of changes. Most were minor:
giving variables better names, fixing documentation, grouping related
predicates together etc. I also added some XXXs for you to look at.

I did make two significant changes. First, I fixed a bug in the code that
created the foreign_proc invocation of increment_coverage_pointer_counter:
it used to set the instmap delta of the foreign_proc to say that it bound
both its arguments, even though they are both input. Second, I added a
new implementor-only option, --coverage-profiling-use-calls, that tells
deep_profiling.m to generate a call to increment_coverage_pointer_counter
instead of a foreign_proc. This should be useful if assertions are turned
on.

Next I will have a look at the code to compute coverage counts in the
deep profiler.

Zoltan.

-----------------------------------------------------------------------------

Post-commit review of Paul's latest change to coverage profiling, together
with some misc cleanups.

compiler/options.m:
	Add the new option --coverage-profiling-use-calls.

compiler/deep_profiling.m:
	Fix some misleading variable names and some misleading comments.

	Make some comments more readable.

	Group related predicates together.

	Mark suspect code and unrevealing documentation with XXXs.

	Fix an old bug: increment_coverage_point_counter does not bind its
	arguments.

	Implement the new option.

library/profiling_builtin.m:
	Add a real body for increment_coverage_point_counter, since the new
	option needs it.

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/deep_profiling.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/deep_profiling.m,v
retrieving revision 1.87
diff -u -b -r1.87 deep_profiling.m
--- compiler/deep_profiling.m	25 Sep 2008 13:44:48 -0000	1.87
+++ compiler/deep_profiling.m	26 Sep 2008 06:53:39 -0000
@@ -1929,7 +1929,7 @@
 % Coverage Profiling.
 %-----------------------------------------------------------------------------%
 
-    % This structure is passed between predicates in this section.
+    % Information used by the coverage profiling transformation.
     %
 :- type proc_coverage_info
     --->    proc_coverage_info(
@@ -1944,6 +1944,8 @@
                 % introduced.
                 ci_var_info                 :: var_info,
 
+                % The following fields are static; they are not modified
+                % after initialization.
                 ci_module_info              :: module_info,
                 ci_pred_proc_id             :: pred_proc_id,
                 ci_maybe_rec_info           :: maybe(deep_recursion_info),
@@ -1955,8 +1957,8 @@
 :- type coverage_profiling_options
     --->    coverage_profiling_options(
                 % These fields correspond to coverage profiling options that
-                % may be specified on the command line.  Except cpu_use_2pass.
-
+                % may be specified on the command line.
+                cpo_use_calls               :: bool,
                 cpo_coverage_after_goal     :: bool,
                 cpo_branch_ite              :: bool,
                 cpo_branch_switch           :: bool,
@@ -1964,14 +1966,12 @@
                 cpo_use_portcounts          :: bool,
                 cpo_use_trivial             :: bool,
 
-                % cpu_use_2pass is true if some information needs to be
-                % collected in an initial forwards-pass.
-                cpo_use_2pass               :: bool
+                % cpo_run_first_pass is true if some information needs to be
+                % collected in an initial pass.
+                cpo_run_first_pass          :: bool
             ).
 
-    % Return wheather each coverage point type should be enabled and iff
-    % coverage any coverage points are enabled then perform the coverage
-    % profiling pass.
+    % Initialize the coverage_profiling_options structure.
     %
 :- pred coverage_profiling_options(module_info::in,
     coverage_profiling_options::out) is det.
@@ -1979,6 +1979,10 @@
 coverage_profiling_options(ModuleInfo, CoveragePointOptions) :-
     module_info_get_globals(ModuleInfo, Globals),
 
+    % Options controlling what instrumentation code we generate.
+    globals.lookup_bool_option(Globals, coverage_profiling_use_calls,
+        UseCalls),
+
     % Coverage point types.
     globals.lookup_bool_option(Globals, profile_deep_coverage_after_goal,
         CoverageAfterGoal),
@@ -1989,19 +1993,19 @@
     globals.lookup_bool_option(Globals, profile_deep_coverage_branch_disj,
         BranchDisj),
 
-    % Interpret options for tuning the coverage profiling pass.
+    % Options for tuning the coverage profiling pass.
     globals.lookup_bool_option(Globals, profile_deep_coverage_use_portcounts,
         UsePortCounts),
     globals.lookup_bool_option(Globals, profile_deep_coverage_use_trivial,
         UseTrivial),
-    bool.or(UsePortCounts, UseTrivial, Use2Pass),
+    bool.or(UsePortCounts, UseTrivial, RunFirstPass),
 
-    CoveragePointOptions = coverage_profiling_options(CoverageAfterGoal,
-        BranchIf, BranchSwitch, BranchDisj, UsePortCounts, UseTrivial,
-        Use2Pass).
+    CoveragePointOptions = coverage_profiling_options(UseCalls,
+        CoverageAfterGoal, BranchIf, BranchSwitch, BranchDisj,
+        UsePortCounts, UseTrivial, RunFirstPass).
 
-    % Transform the goal if coverage profiling should be performed, otherwise
-    % return it un-altered.
+    % Perform the coverage profiling transformation on the given goal,
+    % and return a list of the coverage points created.
     %
 :- pred coverage_prof_transform_goal(module_info::in, pred_proc_id::in,
     maybe(deep_recursion_info)::in, hlds_goal::in, hlds_goal::out,
@@ -2012,12 +2016,13 @@
     coverage_profiling_options(ModuleInfo, CoverageProfilingOptions),
     CoverageInfo0 = init_proc_coverage_info(!.VarInfo, ModuleInfo,
         PredProcId, MaybeRecInfo, CoverageProfilingOptions),
+    RunFirstPass = CoverageProfilingOptions ^ cpo_run_first_pass,
     (
-        CoverageProfilingOptions ^ cpo_use_2pass = yes,
+        RunFirstPass = yes,
         coverage_prof_first_pass(CoverageProfilingOptions, !Goal,
             port_counts_give_coverage_after, _)
     ;
-        CoverageProfilingOptions ^ cpo_use_2pass = no
+        RunFirstPass = no
     ),
     coverage_prof_second_pass_goal(!Goal, coverage_before_known, _,
         CoverageInfo0, CoverageInfo, _),
@@ -2030,10 +2035,10 @@
     %
     % Step 1: Apply transformation recursively.
     %
-    % Step 2: Consider inserting a coverage point after this goal to measure
-    % how many times it succeeds.
+    % Step 2: Decide whether to insert a coverage point after this goal
+    % to measure how many times it succeeds.
     % 
-    % Step 3: Insert the coverage point if we decided to earlier.
+    % Step 3: Insert the coverage point if we decided to do so.
     %
 :- pred coverage_prof_second_pass_goal(hlds_goal::in, hlds_goal::out,
     coverage_before_known::in, coverage_before_known::out,
@@ -2050,10 +2055,11 @@
         IsMDProfInst = goal_is_not_mdprof_inst,
         CoverageBeforeKnown = coverage_before_unknown
     ->
-        unexpected(this_file, string.format(
+        UnknownMsg = string.format(
             "coverage_prof_second_pass_goal: Coverage information is unknown\n"
             ++ "\tGoalPath: %s",
-            [s(goal_path_to_string(GoalPath))]))
+            [s(goal_path_to_string(GoalPath))]),
+        unexpected(this_file, UnknownMsg)
     ;
         true
     ),
@@ -2067,14 +2073,50 @@
     %
     % Apply transformation recursively.
     (
-        ( GoalExpr0 = unify(_, _, _, _, _)
-        ; GoalExpr0 = plain_call(_, _, _, _, _, _)
-        ; GoalExpr0 = generic_call(_, _, _, _)
-        ; GoalExpr0 = call_foreign_proc(_, _, _, _, _, _, _)
+        (
+            GoalExpr0 = unify(_, _, _, _, _)
+        ;
+            GoalExpr0 = call_foreign_proc(_, _, _, _, _, _, _)
+            % Even though the deep profiler creates a call site when these may
+            % call Mercury, the coverage propagation code cannot make use of
+            % the call site's port counts, since they measure how often
+            % Mercury is re-entered, not how often this call is made.
+        ),
+        coverage_known_after_goal_with_detism(Detism,
+            CoverageBeforeKnown, NextCoverageBeforeKnown0),
+        AddedImpurityInner = no,
+        GoalExpr1 = GoalExpr0
+    ;
+        (
+            GoalExpr0 = plain_call(_, _, _, BuiltinState, _, _),
+            (
+                ( BuiltinState = out_of_line_builtin
+                ; BuiltinState = not_builtin
+                ),
+                GathersCoverageAfter = yes
+            ;
+                BuiltinState = inline_builtin,
+                GathersCoverageAfter = no
+            )
+        ;
+            GoalExpr0 = generic_call(GenericCall, _, _, _),
+            (
+                ( GenericCall = higher_order(_, _, _, _)
+                ; GenericCall = class_method(_, _, _, _)
         ),
-        ( goal_expr_has_call_site(GoalExpr0) ->
+                GathersCoverageAfter = yes
+            ;
+                ( GenericCall = cast(_)
+                ; GenericCall = event_call(_)
+                ),
+                GathersCoverageAfter = no
+            )
+        ),
+        (
+            GathersCoverageAfter = yes,
             NextCoverageBeforeKnown0 = coverage_before_known
         ;
+            GathersCoverageAfter = no,
             coverage_known_after_goal_with_detism(Detism, 
                 CoverageBeforeKnown, NextCoverageBeforeKnown0)
         ),
@@ -2101,9 +2143,8 @@
     ;
         GoalExpr0 = negation(NegGoal0),
         coverage_prof_second_pass_goal(NegGoal0, NegGoal, 
-            CoverageBeforeKnown, _, !Info,
-            AddedImpurityInner),
-        % The coverage after a negated goal cannot be inferred from it's inner
+            CoverageBeforeKnown, _, !Info, AddedImpurityInner),
+        % The coverage after a negated goal cannot be inferred from its inner
         % goals.
         coverage_known_after_goal_with_detism(Detism, 
             CoverageBeforeKnown, NextCoverageBeforeKnown0),
@@ -2134,10 +2175,10 @@
         unexpected(this_file, "coverage_prof_second_pass_goal: shorthand")
     ),
 
-    % Step 3.
+    % Step 2.
     %
-    % Consider inserting a coverage point after this goal to measure how many
-    % times it succeeds.
+    % Decide whether we need to insert a coverage point after this goal
+    % to measure how many times execution reaches there.
     (
         (   
             % Never insert coverage points on goals that are part of the deep
@@ -2155,65 +2196,40 @@
             GoalExpr0 = conj(_, _)
         )
     ->
-        MaybeCPType = no,
+        MaybeAddCP = no,
         NextCoverageBeforeKnown = NextCoverageBeforeKnown0
     ;
         CoverageAfterGoals = CPOptions ^ cpo_coverage_after_goal,
         (
             CoverageAfterGoals  = yes,
-            MaybeCPType = yes(cp_type_coverage_after),
+            MaybeAddCP = yes(cp_type_coverage_after),
             NextCoverageBeforeKnown = coverage_before_known
         ;
             CoverageAfterGoals = no,
-            MaybeCPType = no,
+            MaybeAddCP = no,
             NextCoverageBeforeKnown = NextCoverageBeforeKnown0
         )
     ),
 
-    % Step 4.
+    % Step 3.
     %
-    % Insert the coverage point if we decided to earlier.
+    % Insert the coverage point if we decided to.
     add_impurity_if_needed(AddedImpurityInner, GoalInfo0, GoalInfo1),
     Goal1 = hlds_goal(GoalExpr1, GoalInfo1),
     (
-        MaybeCPType = yes(CPType),
+        MaybeAddCP = yes(CPType),
         CPInfo = coverage_point_info(GoalPath, CPType),
 
-        make_coverage_point(CPInfo, CPGoals, !Info),
+        make_coverage_point(CPOptions, CPInfo, CPGoals, !Info),
         create_conj_from_list([Goal1 | CPGoals], plain_conj, Goal),
 
         AddedImpurity = yes
     ;
-        MaybeCPType = no,
+        MaybeAddCP = no,
         Goal = Goal1,
         AddedImpurity = AddedImpurityInner
     ). 
 
-    % True if the deep profiler will instrument this call site and the port
-    % counts from it are useful for coverage profiling.
-    %
-:- pred goal_expr_has_call_site(hlds_goal_expr::in) is semidet.
-
-goal_expr_has_call_site(GoalExpr) :-
-    (
-        GoalExpr = plain_call(_, _, _, BuiltinState, _, _),
-        ( BuiltinState = out_of_line_builtin
-        ; BuiltinState = not_builtin
-        )
-    ;
-        GoalExpr = generic_call(GenericCall, _, _, _),
-        ( GenericCall = higher_order(_, _, _, _)
-        ; GenericCall = class_method(_, _, _, _)
-        )
-    ;        
-        % Even though the deep profiler creates a call site when these may
-        % call mercury the coverage propagation code cannot make use of the
-        % port counts, since they measure how often Mercury is re-entered,
-        % not how often this call is made.
-        GoalExpr = call_foreign_proc(_, _, _, _, _, _, _),
-        fail
-    ).
-
 :- pred coverage_known_after_goal_with_detism(determinism::in,
     coverage_before_known::in, coverage_before_known::out) is det.
 
@@ -2236,6 +2252,7 @@
     % Perform the coverage profiling transformation for conjuncts.  The goal
     % list represents the list of goals within a conjunction minus 'Pos' goals
     % removed from the head.
+    % XXX What does that mean?
     %
 :- pred coverage_prof_second_pass_conj(conj_type::in,
     list(hlds_goal)::in, list(hlds_goal)::out,
@@ -2243,30 +2260,28 @@
     proc_coverage_info::in, proc_coverage_info::out, bool::out) is det.
 
 coverage_prof_second_pass_conj(_, [], [], !CoverageBeforeKnown, !Info, no).
-coverage_prof_second_pass_conj(ConjType, [Goal0 | Goals0], Goals,
+coverage_prof_second_pass_conj(ConjType, [HeadGoal0 | TailGoals0], Goals,
         CoverageBeforeKnown, NextCoverageBeforeKnown, !Info, AddedImpurity) :-
-    coverage_prof_second_pass_goal(Goal0, Goal1,
-        CoverageBeforeKnown, CoverageBeforeTailKnown, !Info, AddedImpurityHead),
-
-    % Flatten the conjunction, this is necessary when coverage points are
-    % inserted into conjuncts.
+    coverage_prof_second_pass_goal(HeadGoal0, HeadGoal,
+        CoverageBeforeKnown, CoverageBeforeTailKnown, !Info,
+        AddedImpurityHead),
+    coverage_prof_second_pass_conj(ConjType, TailGoals0, TailGoals,
+        CoverageBeforeTailKnown, NextCoverageBeforeKnown, !Info,
+        AddedImpurityTail),
+    % Flatten the conjunction. We need to do this if we replaced the head
+    % with a goal that is itself a conjunction.
     (
-        Goal1 = hlds_goal(conj(plain_conj, ConjGoals), _),
+        HeadGoal = hlds_goal(conj(plain_conj, HeadConjGoals), _),
         ConjType = plain_conj
     ->
-        Goals = ConjGoals ++ Goals1
+        Goals = HeadConjGoals ++ TailGoals
     ;
-        Goals = [Goal1 | Goals1]
+        Goals = [HeadGoal | TailGoals]
     ),
-
-    coverage_prof_second_pass_conj(ConjType, Goals0, Goals1,
-        CoverageBeforeTailKnown, NextCoverageBeforeKnown, !Info,
-        AddedImpurityTail),
     bool.or(AddedImpurityHead, AddedImpurityTail, AddedImpurity).
 
     % Perform the coverage profiling transformation over goals within a
-    % disjunction.  The list of goals represents the tail of the disjunction
-    % currently being transformed.
+    % disjunction.
     %
 :- pred coverage_prof_second_pass_disj(dp_goal_info::in, 
     coverage_before_known::in, coverage_before_known::out,
@@ -2274,37 +2289,42 @@
     proc_coverage_info::in, proc_coverage_info::out, bool::out) is det.
 
 coverage_prof_second_pass_disj(DPInfo, CoverageBeforeKnown,
-        NextCoverageBeforeKnown, Disjs0, Disjs, !Info, AddedImpurity) :-
+        NextCoverageBeforeKnown, Disjuncts0, Disjuncts, !Info,
+        AddedImpurity) :-
     % If the disjunction was introduced by the deep profiling pass, it has two
-    % disjuncts and it's second disjunct has 'failure' determinism, then
+    % disjuncts and its second disjunct has 'failure' determinism, then
     % perform the coverage profiling pass on the first disjunct as if this
     % is the only goal. 
     (
         DPInfo = dp_goal_info(goal_is_mdprof_inst, _),
-        Disjs0 = [FirstDisj0, SecondDisj],
-        SecondDisj ^ hlds_goal_info ^ goal_info_get_determinism = detism_failure
+        Disjuncts0 = [FirstDisjunct0, SecondDisjunct],
+        goal_info_get_determinism(SecondDisjunct ^ hlds_goal_info) =
+            detism_failure
+        % XXX Would this be a better test here?
+        % goal_has_feature(SecondDisjunct, feature_preserve_backtrack_into)
     ->
-        coverage_prof_second_pass_goal(FirstDisj0, FirstDisj,
+        coverage_prof_second_pass_goal(FirstDisjunct0, FirstDisjunct,
             CoverageBeforeKnown, NextCoverageBeforeKnown, !Info,
             AddedImpurity),
-        Disjs = [FirstDisj, SecondDisj]
+        Disjuncts = [FirstDisjunct, SecondDisjunct]
     ;
         coverage_prof_second_pass_disj_2(DPInfo, CoverageBeforeKnown, 
-            Disjs0, Disjs, !Info, AddedImpurity),
+            Disjuncts0, Disjuncts, !Info, AddedImpurity),
         NextCoverageBeforeKnown = coverage_before_unknown
     ).
 
 :- pred coverage_prof_second_pass_disj_2(dp_goal_info::in, 
-    coverage_before_known::in,
-    list(hlds_goal)::in, list(hlds_goal)::out, 
+    coverage_before_known::in, list(hlds_goal)::in, list(hlds_goal)::out,
     proc_coverage_info::in, proc_coverage_info::out, bool::out) is det.
 
 coverage_prof_second_pass_disj_2(_, _, [], [], !Info, no).
 coverage_prof_second_pass_disj_2(DPInfo, CoverageBeforeKnown0, 
-        [Goal0 | Goals0], [Goal | Goals], !Info, AddedImpurity) :-
-    % If coverage is unknown before the disjunct, then decide to insert a
-    % branch coverage point at the beginning of the disjunct.
-    CPOBranchDisj = !.Info ^ ci_coverage_profiling_opts ^ cpo_branch_disj,
+        [HeadDisjunct0 | TailDisjuncts0], [HeadDisjunct | TailDisjuncts],
+        !Info, AddedImpurity) :-
+    % Decide whether we want to insert a branch coverage point at the beginning
+    % of the head disjunct.
+    CPOptions = !.Info ^ ci_coverage_profiling_opts,
+    CPOBranchDisj = CPOptions ^ cpo_branch_disj,
     DPInfo = dp_goal_info(IsMDProfInst, _),
     (
         CPOBranchDisj = yes,
@@ -2318,26 +2338,23 @@
         CoverageBeforeKnown = CoverageBeforeKnown0
     ),
 
-    % Transform the disjunct its self.
-    coverage_prof_second_pass_goal(Goal0, Goal1,
+    coverage_prof_second_pass_goal(HeadDisjunct0, HeadDisjunct1,
         CoverageBeforeKnown, _CoverageAfterHeadKnown, !Info,
         AddedImpurityHead),
-
-    % Transform the tail of the disjunction.
-    coverage_prof_second_pass_disj_2(DPInfo,
-        coverage_before_unknown, Goals0, Goals, !Info,
-        AddedImpurityTail),
+    coverage_prof_second_pass_disj_2(DPInfo, coverage_before_unknown,
+        TailDisjuncts0, TailDisjuncts, !Info, AddedImpurityTail),
 
     % Insert the coverage point if we decided to above.
     (
-        InsertCP = yes
-    ->
-        DisjPath = goal_info_get_goal_path(Goal0 ^ hlds_goal_info),
-        insert_coverage_point_before(coverage_point_info(DisjPath,
-            cp_type_branch_arm), Goal1, Goal, !Info),
+        InsertCP = yes,
+        DisjPath = goal_info_get_goal_path(HeadDisjunct0 ^ hlds_goal_info),
+        HeadCoveragePoint = coverage_point_info(DisjPath, cp_type_branch_arm),
+        insert_coverage_point_before(CPOptions, HeadCoveragePoint,
+            HeadDisjunct1, HeadDisjunct, !Info),
         AddedImpurity = yes
     ;
-        Goal = Goal1,
+        InsertCP = no,
+        HeadDisjunct = HeadDisjunct1,
         AddedImpurity = bool.or(AddedImpurityHead, AddedImpurityTail)
     ).
 
@@ -2352,7 +2369,8 @@
     proc_coverage_info::in, proc_coverage_info::out, bool::out) is det.
 
 coverage_prof_second_pass_switchcase(DPInfo, CanFail, Cases0, Cases,
-        CoverageBeforeSwitchKnown, CoverageAfterSwitchKnown, !Info, AddedImpurity) :-
+        CoverageBeforeSwitchKnown, CoverageAfterSwitchKnown, !Info,
+        AddedImpurity) :-
     % If the switch can fail then the coverage after it will be unknown.
     (
         CanFail = can_fail,
@@ -2361,8 +2379,9 @@
         CanFail = cannot_fail,
         CoverageAfterSwitchKnown0 = coverage_before_known
     ),
+    CoverageBeforeEveryCaseKnown = coverage_before_known,
     coverage_prof_second_pass_switchcase_2(DPInfo, CanFail, Cases0, Cases,
-        CoverageBeforeSwitchKnown, coverage_before_known,
+        CoverageBeforeSwitchKnown, CoverageBeforeEveryCaseKnown,
         CoverageAfterSwitchKnown0, CoverageAfterSwitchKnown, !Info,
         AddedImpurity).
 
@@ -2374,59 +2393,61 @@
 
 coverage_prof_second_pass_switchcase_2(_, _, [], [], _, _,
         !CoverageAfterSwitchKnown, !Info, no). 
-
 coverage_prof_second_pass_switchcase_2(DPInfo, SwitchCanFail,
         [Case0 | Cases0], [Case | Cases], CoverageBeforeSwitchKnown,
         CoverageBeforeEveryCaseKnown, !CoverageAfterSwitchKnown, !Info,
         AddedImpurity) :-
-    Goal0 = Case0 ^ case_goal,
+    Case0 = case(MainConsId, OtherConsIds, Goal0),
 
-    % If the switch cannot fail and this is the last case then the coverage
-    % at the beginning of this case can be computed from the coverage before the
-    % entire switch and coverage information for the tail of the switch such as
-    % branch coverage points.
+    % If the switch cannot fail and this is the last case, then the coverage
+    % at the beginning of this case can be computed from the coverage before
+    % the entire switch and coverage information for the tail of the switch
+    % such as branch coverage points.
+    % XXX The last part of that sentence does not make sense.
     (
         Cases0 = [],
         (
             SwitchCanFail = cannot_fail,
-            CoverageBeforeHeadKnown0 = coverage_before_known_and(
+            CoverageBeforeCaseKnown0 = coverage_before_known_and(
                 CoverageBeforeSwitchKnown, CoverageBeforeEveryCaseKnown)
         ;
             SwitchCanFail = can_fail,
-            CoverageBeforeHeadKnown0 = coverage_before_unknown
+            CoverageBeforeCaseKnown0 = coverage_before_unknown
         )
     ;
         Cases0 = [_ | _],
-        CoverageBeforeHeadKnown0 = coverage_before_unknown
+        CoverageBeforeCaseKnown0 = coverage_before_unknown
     ),
 
     % Decide whether to insert a coverage point here.
-    CPOBranchSwitch = !.Info ^ ci_coverage_profiling_opts ^
-        cpo_branch_switch,
+    CPOptions = !.Info ^ ci_coverage_profiling_opts,
+    CPOBranchSwitch = CPOptions ^ cpo_branch_switch,
     DPInfo = dp_goal_info(IsMDProfInst, _),
     (
         CPOBranchSwitch = yes,
-        CoverageBeforeHeadKnown0 = coverage_before_unknown,
+        CoverageBeforeCaseKnown0 = coverage_before_unknown,
         IsMDProfInst = goal_is_not_mdprof_inst
     ->
         InsertCP = yes,
-        CoverageBeforeHeadKnown = coverage_before_known
+        CoverageBeforeCaseKnown = coverage_before_known
     ;      
         InsertCP = no,
-        CoverageBeforeHeadKnown = CoverageBeforeHeadKnown0
+        CoverageBeforeCaseKnown = CoverageBeforeCaseKnown0
     ),
         
-    coverage_prof_second_pass_goal(Goal0, Goal1, CoverageBeforeHeadKnown,
-        CoverageAfterCaseKnown, !Info, AddedImpurityHead0),
+    coverage_prof_second_pass_goal(Goal0, Goal1,
+        CoverageBeforeCaseKnown, CoverageAfterCaseKnown, !Info,
+        AddedImpurityHead0),
     !:CoverageAfterSwitchKnown = coverage_before_known_and(
         CoverageAfterCaseKnown, !.CoverageAfterSwitchKnown),
 
-    % Possibly insert coverage point.
+    % Possibly insert coverage point at the start of the case.
     (   
         InsertCP = yes,
         CasePath = goal_info_get_goal_path(Goal0 ^ hlds_goal_info),
-        insert_coverage_point_before(coverage_point_info(CasePath,
-            cp_type_branch_arm), Goal1, Goal, !Info),
+        CoveragePoint = coverage_point_info(CasePath, cp_type_branch_arm),
+        insert_coverage_point_before(CPOptions, CoveragePoint, Goal1, Goal,
+            !Info),
         AddedImpurityHead = yes
     ;
         InsertCP = no,
@@ -2435,22 +2456,25 @@
     ),
 
     % Handle recursive case and prepare output variables.
+    % We cannot optimize away the coverage point at the start of the last case
+    % if one of the previous cases does not have coverage information at its
+    % start.
     NextCoverageBeforeEveryCaseKnown = coverage_before_known_and(
-        CoverageBeforeEveryCaseKnown, CoverageBeforeHeadKnown),
-    coverage_prof_second_pass_switchcase_2(DPInfo, SwitchCanFail, Cases0, Cases,
+        CoverageBeforeEveryCaseKnown, CoverageBeforeCaseKnown),
+    coverage_prof_second_pass_switchcase_2(DPInfo, SwitchCanFail,
+        Cases0, Cases,
         CoverageBeforeSwitchKnown, NextCoverageBeforeEveryCaseKnown, 
         !CoverageAfterSwitchKnown, !Info, AddedImpurityTail),
-    Case = Case0 ^ case_goal := Goal,
+    Case = case(MainConsId, OtherConsIds, Goal),
     bool.or(AddedImpurityHead, AddedImpurityTail, AddedImpurity).
 
     % Determine if branch coverage points should be inserted in either or
     % both of the then and else branches, insert them and transform the
-    % sub-goals.
+    % subgoals.
     %
-    % This is performed by first transforming the condition goal,
-    % then making decisions about coverage points and inserting them, then
-    % transforming the then and else branches and constructing the new ITE
-    % goal_expr.
+    % This is performed by first transforming the condition, then making
+    % decisions about coverage points and inserting them, then transforming
+    % the then and else branches and constructing the new ITE goal_expr.
     %
 :- pred coverage_prof_second_pass_ite(dp_goal_info::in, list(prog_var)::in,
     hlds_goal::in, hlds_goal::in, hlds_goal::in, hlds_goal_expr::out,
@@ -2460,7 +2484,7 @@
 coverage_prof_second_pass_ite(DPInfo, ITEExistVars, Cond0, Then0, Else0,
         GoalExpr, CoverageBeforeITEKnown, NextCoverageBeforeKnown,
         !Info, AddedImpurity) :-
-    % Transform Cond branch.
+    % Transform the condition.
     coverage_prof_second_pass_goal(Cond0, Cond,
         CoverageBeforeITEKnown, CoverageKnownAfterCond, !Info,
         AddedImpurityCond),
@@ -2475,8 +2499,9 @@
     % may be as different variables may be used in different branches.
     %
     % Whatever we do we will ensure that the coverage will be known at the
-    % beginning of each branch,
-    CPOBranchIf = !.Info ^ ci_coverage_profiling_opts ^ cpo_branch_ite,
+    % beginning of each branch.
+    CPOptions = !.Info ^ ci_coverage_profiling_opts,
+    CPOBranchIf = CPOptions ^ cpo_branch_ite,
     DPInfo = dp_goal_info(IsMDProfInst, _),
     (
         CPOBranchIf = yes,
@@ -2493,12 +2518,11 @@
         ),
         % Always insert a coverage point for the else branch.
         ElsePath = goal_info_get_goal_path(Else0 ^ hlds_goal_info),
-        InsertCPElse = yes(coverage_point_info(ElsePath,
-            cp_type_branch_arm)),
+        InsertCPElse = yes(coverage_point_info(ElsePath, cp_type_branch_arm)),
         CoverageKnownBeforeThen = coverage_before_known,
         CoverageKnownBeforeElse = coverage_before_known
     ;
-        % Don't insert any coverage points,
+        % Don't insert any coverage points.
         InsertCPThen = no,
         InsertCPElse = no,
         CoverageKnownBeforeThen = CoverageKnownBeforeThen0,
@@ -2516,7 +2540,8 @@
     % Insert any coverage points.
     (
         InsertCPThen = yes(CPInfoThen),
-        insert_coverage_point_before(CPInfoThen, Then1, Then, !Info),
+        insert_coverage_point_before(CPOptions, CPInfoThen, Then1, Then,
+            !Info),
         AddedImpurityThen = yes
     ;
         InsertCPThen = no,
@@ -2525,7 +2550,8 @@
     ),
     (
         InsertCPElse = yes(CPInfoElse),
-        insert_coverage_point_before(CPInfoElse, Else1, Else, !Info),
+        insert_coverage_point_before(CPOptions, CPInfoElse, Else1, Else,
+            !Info),
         AddedImpurityElse = yes
     ;
         InsertCPElse = no,
@@ -2533,96 +2559,26 @@
         AddedImpurityElse = AddedImpurityElseGoal
     ),
 
-    % Build goal experession and tidy up.
+    % Build goal expression and tidy up.
     AddedImpurity = bool.or(AddedImpurityCond,
         bool.or(AddedImpurityThen, AddedImpurityElse)),
     GoalExpr = if_then_else(ITEExistVars, Cond, Then, Else),
     NextCoverageBeforeKnown = coverage_before_known_and(
         NextCoverageKnownThen, NextCoverageKnownElse).
 
-:- func goal_info_get_dp_info(hlds_goal_info) = dp_goal_info.
-
-goal_info_get_dp_info(GoalInfo) = DPInfo :-
-    MaybeDPInfo = goal_info_get_maybe_dp_info(GoalInfo),
-    (
-        MaybeDPInfo = yes(DPInfo)
-    ;
-        MaybeDPInfo = no,
-        error("goal_info_get_dp_info: MaybeDPInfo = no")
-    ).
-
-:- func goal_info_get_maybe_dp_coverage_info(hlds_goal_info) =
-    maybe(dp_coverage_goal_info).
-
-goal_info_get_maybe_dp_coverage_info(GoalInfo) = MaybeCoverageInfo :-
-    MaybeDPInfo = goal_info_get_maybe_dp_info(GoalInfo),
-    (
-        MaybeDPInfo = yes(DPInfo),
-        DPInfo = dp_goal_info(_, MaybeCoverageInfo)
-    ;
-        MaybeDPInfo = no,
-        MaybeCoverageInfo = no
-    ).
-
-:- func goal_get_maybe_dp_port_counts_coverage(hlds_goal) =
-    port_counts_give_coverage_after.
-
-goal_get_maybe_dp_port_counts_coverage(Goal) = PortCountsGiveCoverageAfter :-
-    Goal = hlds_goal(_, GoalInfo),
-    MaybeCoverageInfo = goal_info_get_maybe_dp_coverage_info(GoalInfo),
-    (
-        MaybeCoverageInfo = 
-            yes(dp_coverage_goal_info(_, PortCountsGiveCoverageAfter))
-    ;
-        MaybeCoverageInfo = no,
-        PortCountsGiveCoverageAfter = no_port_counts_give_coverage_after
-    ).
-
-    % Set the 'goal_is_mdprof_inst' field in the goal_dp_info structure
-    % in the given goal info structure.
-    %
-:- pred goal_info_set_mdprof_inst(goal_is_mdprof_inst::in,
-    hlds_goal_info::in, hlds_goal_info::out) is det.
-
-goal_info_set_mdprof_inst(IsMDProfInst, !GoalInfo) :-
-    goal_info_get_maybe_dp_info(!.GoalInfo) = MaybeDPInfo0,
-    (
-        MaybeDPInfo0 = yes(dp_goal_info(_, DPCoverageInfo)),
-        MaybeDPInfo = yes(dp_goal_info(IsMDProfInst, DPCoverageInfo))
-    ;
-        MaybeDPInfo0 = no,
-        MaybeDPInfo = yes(dp_goal_info(IsMDProfInst, no))
-    ),
-    goal_info_set_maybe_dp_info(MaybeDPInfo, !GoalInfo).
-
-    % Insert a coverage point in a conjunction before the current goal if
-    % the coverage point info has been provided.
-    %
-:- pred maybe_insert_coverage_point_before(maybe(coverage_point_info)::in,
-    hlds_goal::in, hlds_goal::out,
-    coverage_before_known::in, coverage_before_known::out,
-    proc_coverage_info::in, proc_coverage_info::out, bool::out) is det.
-
-maybe_insert_coverage_point_before(no, !Goal, !CoverageBeforeKnown, !Info, no).
-maybe_insert_coverage_point_before(yes(CPInfo), !Goal,
-        _, coverage_before_known, !Info, yes) :-
-    insert_coverage_point_before(CPInfo, !Goal, !Info).
+%-----------------------------------------------------------------------------%
 
-    % Insert a coverage point before the given goal. This returns a flat
-    % conjunction consisting of a coverage point followed by the input goal.
+    % Create a coverage info struture, initializing some values to sensible
+    % defaults.
     %
-:- pred insert_coverage_point_before(coverage_point_info::in,
-    hlds_goal::in, hlds_goal::out,
-    proc_coverage_info::in, proc_coverage_info::out) is det.
+:- func init_proc_coverage_info(var_info, module_info, pred_proc_id,
+    maybe(deep_recursion_info), coverage_profiling_options) =
+    proc_coverage_info.
 
-insert_coverage_point_before(CPInfo, !Goal, !Info) :-
-    make_coverage_point(CPInfo, CPGoals, !Info),
-    ( !.Goal = hlds_goal(conj(plain_conj, InnerGoals), _) ->
-        Goals = CPGoals ++ InnerGoals
-    ;
-        Goals = CPGoals ++ [!.Goal]
-    ),
-    create_conj_from_list(Goals, plain_conj, !:Goal).
+init_proc_coverage_info(VarInfo, ModuleInfo, PredProcId, MaybeRecInfo,
+        CoverageProfilingOptions) = CoverageInfo :-
+    CoverageInfo = proc_coverage_info(map.init, counter.init(0), VarInfo,
+        ModuleInfo, PredProcId, MaybeRecInfo, CoverageProfilingOptions).
 
     % Used to describe if coverage information is known at a partiular point
     % within a procedure.
@@ -2642,18 +2598,6 @@
 coverage_before_known_and(coverage_before_unknown, _) =
     coverage_before_unknown.
 
-    % Create a coverage info struture, initializing some values to sensible
-    % defaults.
-    %
-:- func init_proc_coverage_info(var_info, module_info, pred_proc_id,
-        maybe(deep_recursion_info), coverage_profiling_options) =
-    proc_coverage_info.
-
-init_proc_coverage_info(VarInfo, ModuleInfo, PredProcId, MaybeRecInfo,
-        CoverageProfilingOptions) = CoverageInfo :-
-    CoverageInfo = proc_coverage_info(map.init, counter.init(0), VarInfo,
-        ModuleInfo, PredProcId, MaybeRecInfo, CoverageProfilingOptions).
-
     % Boolean AND for the goal_trivial data type.
     %
 :- pred goal_trivial_and(goal_trivial::in, goal_trivial::in,
@@ -2689,9 +2633,8 @@
     % immediately after the goal.
     %
 :- pred has_port_counts_after(hlds_goal::in,
-    port_counts_give_coverage_after::in,
-    port_counts_give_coverage_after::in, port_counts_give_coverage_after::out) 
-    is det.
+    port_counts_give_coverage_after::in, port_counts_give_coverage_after::in,
+    port_counts_give_coverage_after::out) is det.
 
 has_port_counts_after(Goal, PCDirect, PCBefore, PC) :-
     (
@@ -2735,9 +2678,12 @@
     % transformation.
     %
     % This pass gathers the information in the dp_coverage_goal_info structure,
-    % namely whether the goal is trivial (if it and none of it's subgoals are
-    % calls),  And whether a port count is available from the deep profiler
-    % from which the coverage _after_ this goal can be computed.
+    % namely
+    %
+    % - whether the goal is trivial (a goal is trivial if neither it
+    %   nor any of its subgoals are calls), and
+    % - whether a port count is available from the deep profiler from which
+    %   the coverage _after_ this goal can be computed.
     %
     % XXX: Currently the first pass is unsupported. The second pass does not
     % use the information it generates.
@@ -2763,12 +2709,41 @@
         % that are assumed if the information from the first pass is not
         % available.).
         %
-        ( GoalExpr0 = plain_call(_, _, _, _, _, _)
-        ; GoalExpr0 = generic_call(_, _, _, _)
-        ; GoalExpr0 = call_foreign_proc(_, _, _, _, _, _, _)
+        GoalExpr0 = plain_call(_, _, _, BuiltinState, _, _),
+        (
+            ( BuiltinState = out_of_line_builtin
+            ; BuiltinState = not_builtin
+            ),
+            Trivial0 = goal_is_nontrivial,
+            PortCountsCoverageAfterDirect = port_counts_give_coverage_after
+        ;
+            BuiltinState = inline_builtin,
+            Trivial0 = goal_is_trivial,
+            PortCountsCoverageAfterDirect = no_port_counts_give_coverage_after
+        ),
+        GoalExpr = GoalExpr0
+    ;
+        GoalExpr0 = generic_call(GenericCall, _, _, _),
+        (
+            ( GenericCall = higher_order(_, _, _, _)
+            ; GenericCall = class_method(_, _, _, _)
+            ),
+            Trivial0 = goal_is_nontrivial,
+            PortCountsCoverageAfterDirect = port_counts_give_coverage_after
+        ;
+            ( GenericCall = cast(_)
+            ; GenericCall = event_call(_)
+            ),
+            Trivial0 = goal_is_trivial,
+            PortCountsCoverageAfterDirect = no_port_counts_give_coverage_after
         ),
+        GoalExpr = GoalExpr0
+    ;
+        GoalExpr0 = call_foreign_proc(_, _, _, _, _, _, _),
+        % Some foreign proc goals may be trivial., but there is no clear
+        % criteria by which we can conclude that here.
         Trivial0 = goal_is_nontrivial,
-        PortCountsCoverageAfterDirect = port_counts_give_coverage_after,
+        PortCountsCoverageAfterDirect = no_port_counts_give_coverage_after,
         GoalExpr = GoalExpr0
     ;
         GoalExpr0 = unify(_, _, _, _, _),
@@ -2778,18 +2753,19 @@
     ;
         GoalExpr0 = conj(ConjType, Goals0),
         map_foldl2(coverage_prof_first_pass_conj(CPOptions), Goals0, Goals,
-            goal_is_trivial, Trivial0, PortCountsCoverageAfterBefore,
-            PortCountsCoverageAfterDirect),
+            goal_is_trivial, Trivial0,
+            PortCountsCoverageAfterBefore, PortCountsCoverageAfterDirect),
         GoalExpr = conj(ConjType, Goals)
     ;
         GoalExpr0 = disj(Goals0),
-        coverage_prof_first_pass_disj(CPOptions, Goals0, Goals,
-            Trivial0, PortCountsCoverageAfterBefore,
-            PortCountsCoverageAfterDirect0),
+        coverage_prof_first_pass_disj(CPOptions, Goals0, Goals, Trivial0,
+            PortCountsCoverageAfterBefore, PortCountsCoverageAfterDirect0),
         GoalExpr = disj(Goals),
 
         % Only if the disjunction cannot fail can we know the coverage after
         % the disjunction if port counts are available from every disjunct.
+        % XXX What port counts? A disjunct need not contain a call.
+        % do you mean coverage information from the end of each disjunct?
         Detism = goal_info_get_determinism(GoalInfo0),
         determinism_components(Detism, CanFail, _),
         (
@@ -2797,7 +2773,7 @@
             PortCountsCoverageAfterDirect = no_port_counts_give_coverage_after
         ;
             CanFail = cannot_fail,
-            PortCountsCoverageAfterDirect0 = PortCountsCoverageAfterDirect
+            PortCountsCoverageAfterDirect = PortCountsCoverageAfterDirect0
         )
     ;
         GoalExpr0 = switch(Var, CanFail, Cases0),
@@ -2844,11 +2820,11 @@
 
         % An ITE is trivial iff all of its inner goals are trivial,
 
-        goal_trivial_and(TrivialCond, TrivialThen, Trivial1),
-        goal_trivial_and(Trivial1, TrivialElse, Trivial0),
+        goal_trivial_and(TrivialCond, TrivialThen, TrivialCondThen),
+        goal_trivial_and(TrivialCondThen, TrivialElse, Trivial0),
 
         % And it has port counts iff it will end in a goal with a port count
-        % regardless of how the condition evaluates.
+        % regardless of which of the then and the else branch is taken.
 
         port_counts_give_coverage_after_and(PortCountsCoverageAfterThen,
             PortCountsCoverageAfterElse, PortCountsCoverageAfterDirect)
@@ -2916,6 +2892,7 @@
     !PortCountsCoverageAfter).
 coverage_prof_first_pass_disj(CPOptions, [Goal0 | Goals0], [Goal | Goals],
         Trivial, PortCountsCoverageAfterBefore, PortCountsCoverageAfter) :-
+    % XXX What kind of name is PortCountsCoverageAfterBefore?
     coverage_prof_first_pass(CPOptions, Goal0, Goal,
         PortCountsCoverageAfterBefore,
         dp_coverage_goal_info(TrivialGoal, PortCountsCoverageAfterGoal)),
@@ -2929,6 +2906,10 @@
     % how often execution will enter the first case, so this also cannot use
     % the portcount of a goal before it.
     %
+    % XXX This is wrong: there should be more differences than that, since you
+    % a failure in one switch arm doesn't cause a jump to the start of the next
+    % arm.
+    %
 :- pred coverage_prof_first_pass_switchcase(coverage_profiling_options::in,
     list(case)::in, list(case)::out, goal_trivial::out,
     port_counts_give_coverage_after::out) is det.
@@ -2950,13 +2931,32 @@
 
     Case = case(FirstFunctor, LaterFunctors, Goal).
 
-    % Builds a list of goals that will form a conjunction) for a coverage
-    % point.
+%-----------------------------------------------------------------------------%
+
+    % Insert a coverage point before the given goal. This returns a flat
+    % conjunction consisting of a coverage point followed by the goal.
     %
-:- pred make_coverage_point(coverage_point_info::in, list(hlds_goal)::out,
+:- pred insert_coverage_point_before(coverage_profiling_options::in,
+    coverage_point_info::in, hlds_goal::in, hlds_goal::out,
     proc_coverage_info::in, proc_coverage_info::out) is det.
 
-make_coverage_point(CoveragePointInfo, Goals, !CoverageInfo) :-
+insert_coverage_point_before(CPOptions, CPInfo, !Goal, !Info) :-
+    make_coverage_point(CPOptions, CPInfo, CPGoals, !Info),
+    ( !.Goal = hlds_goal(conj(plain_conj, InnerGoals), _) ->
+        Goals = CPGoals ++ InnerGoals
+    ;
+        Goals = CPGoals ++ [!.Goal]
+    ),
+    create_conj_from_list(Goals, plain_conj, !:Goal).
+
+    % Builds a list of goals (that will form part of a conjunction)
+    % for a coverage point.
+    %
+:- pred make_coverage_point(coverage_profiling_options::in,
+    coverage_point_info::in, list(hlds_goal)::out,
+    proc_coverage_info::in, proc_coverage_info::out) is det.
+
+make_coverage_point(CPOptions, CoveragePointInfo, Goals, !CoverageInfo) :-
     CoveragePointInfos0 = !.CoverageInfo ^ ci_coverage_points,
     CPIndexCounter0 = !.CoverageInfo ^ ci_cp_index_counter,
 
@@ -2977,14 +2977,26 @@
         proc_static_cons_id(!.CoverageInfo, ProcStaticConsId),
         generate_unify(ProcStaticConsId, ProcLayoutVar, GoalUnifyProcLayout),
 
-        !:CoverageInfo = !.CoverageInfo ^ ci_var_info := !.VarInfo
+        !CoverageInfo ^ ci_var_info := !.VarInfo
     ),
 
     % Build a call to the instrumentation code.
 
+    UseCalls = CPOptions ^ cpo_use_calls,
     ModuleInfo = !.CoverageInfo ^ ci_module_info,
-    get_deep_profile_builtin_ppid(ModuleInfo, "increment_coverage_point_count",
-        2, PredId, ProcId),
+    PredName = "increment_coverage_point_count",
+    PredArity = 2,
+    ArgVars = [ProcLayoutVar, CPIndexVar],
+    % XXX The body of increment_coverage_point_count includes several
+    % assertions. If these are enabled, then bodily including the C code
+    % at EVERY coverage point will cause significant code bloat. Generating
+    % a call to a predicate with the same code in library/profiling_builtin.m
+    % should then yield smaller code, and due to cache effects, it will
+    % probably yield faster code as well.
+    (
+        UseCalls = no,
+        get_deep_profile_builtin_ppid(ModuleInfo, PredName, PredArity,
+            PredId, ProcId),
     Ground = ground(shared, none),
     make_foreign_args([ProcLayoutVar, CPIndexVar],
         [(yes("ProcLayout" - (Ground -> Ground)) - native_if_possible),
@@ -2993,26 +3005,29 @@
     coverage_point_ll_code(ForeignCallAttrs, ForeignCode),
     CallGoalExpr = call_foreign_proc(ForeignCallAttrs, PredId, ProcId,
         ForeignArgVars, [], no, ForeignCode),
-
-    Vars = [ProcLayoutVar, CPIndexVar],
-    NonLocals = list_to_set(Vars),
-    instmap_delta_from_assoc_list(assoc_list.from_corresponding_lists(Vars,
-        [Ground, Ground]), InstMapDelta),
-    CallGoalInfo = impure_init_goal_info(NonLocals, InstMapDelta, detism_det),
-    CallGoal = hlds_goal(CallGoalExpr, CallGoalInfo),
+        NonLocals = list_to_set(ArgVars),
+        instmap_delta_from_assoc_list([], InstMapDelta),
+        CallGoalInfo = impure_init_goal_info(NonLocals, InstMapDelta,
+            detism_det),
+        CallGoal = hlds_goal(CallGoalExpr, CallGoalInfo)
+    ;
+        UseCalls = yes,
+        generate_deep_call(ModuleInfo, PredName, PredArity, ArgVars,
+            yes([]), detism_det, CallGoal)
+    ),
 
     % Construct complete goal list.
     Goals = [GoalUnifyIndex, GoalUnifyProcLayout, CallGoal].
 
-    % Turn a map of coverage points and their indexs into a list in sorted
+    % Turn a map of coverage points and their indexes into a list in sorted
     % order.
     %
 :- pred coverage_points_map_list(map(int, coverage_point_info)::in,
     list(coverage_point_info)::out) is det.
 
 coverage_points_map_list(Map, List) :-
-    to_sorted_assoc_list(Map, AssocList),
-    values(AssocList, List).
+    map.to_sorted_assoc_list(Map, AssocList),
+    assoc_list.values(AssocList, List).
 
     % Retrieve the pred and proc ids from either the deep_maybe_rec_info or
     % deep_pred_proc_id fields of a deep_info structure.
@@ -3050,6 +3065,11 @@
     some [!ForeignProcAttrs] (
         % I don't think this would be thread safe but the cost of the mutexes
         % may be too high.
+        % XXX Correctness is more important than cost. The right solution
+        % would be to add a mechanism that, in par grades, allows us to replace
+        % the general foreign code mutex with one that guards only the data
+        % coverage data structure we are updating, since that would yield
+        % a LOT less contention.
         !:ForeignProcAttrs = default_attributes(lang_c),
         set_may_call_mercury(proc_will_not_call_mercury, !ForeignProcAttrs),
         set_purity(purity_impure, !ForeignProcAttrs),
@@ -3060,7 +3080,9 @@
     ),
     ForeignCodeImpl = fc_impl_ordinary(Code, no),
     Code =
-"{
+    % The code of this predicate is duplicated bodily in profiling_builtin.m
+    % in the library directory, so any changes here should also be made there.
+"
 #ifdef MR_DEEP_PROFILING
     const MR_ProcLayout *pl;
     MR_ProcStatic       *ps;
@@ -3080,23 +3102,74 @@
     ps = pl->MR_sle_proc_static;
     MR_deep_assert(NULL, pl, NULL, ps != NULL);
 
-    MR_deep_assert(NULL, pl, ps,
-        CPIndex >= ps->MR_ps_num_coverage_points);
+    MR_deep_assert(NULL, pl, ps, CPIndex >= ps->MR_ps_num_coverage_points);
     MR_deep_assert(NULL, pl, ps, ps->MR_ps_coverage_points != NULL);
 
     ps->MR_ps_coverage_points[CPIndex]++;
 
-    /*
-    ** This procedure doesn't collect statistics about the deep profiler,
-    ** as they can be generated by the profiling data itself.
-    */
-
     MR_leave_instrumentation();
 #else
     MR_fatal_error(
         ""increment_coverage_point_count: deep profiling not enabled"");
 #endif /* MR_DEEP_PROFILING */
-}".
+".
+
+%-----------------------------------------------------------------------------%
+
+:- func goal_info_get_dp_info(hlds_goal_info) = dp_goal_info.
+
+goal_info_get_dp_info(GoalInfo) = DPInfo :-
+    MaybeDPInfo = goal_info_get_maybe_dp_info(GoalInfo),
+    (
+        MaybeDPInfo = yes(DPInfo)
+    ;
+        MaybeDPInfo = no,
+        error("goal_info_get_dp_info: MaybeDPInfo = no")
+    ).
+
+:- func goal_info_get_maybe_dp_coverage_info(hlds_goal_info) =
+    maybe(dp_coverage_goal_info).
+
+goal_info_get_maybe_dp_coverage_info(GoalInfo) = MaybeCoverageInfo :-
+    MaybeDPInfo = goal_info_get_maybe_dp_info(GoalInfo),
+    (
+        MaybeDPInfo = yes(DPInfo),
+        DPInfo = dp_goal_info(_, MaybeCoverageInfo)
+    ;
+        MaybeDPInfo = no,
+        MaybeCoverageInfo = no
+    ).
+
+:- func goal_get_maybe_dp_port_counts_coverage(hlds_goal) =
+    port_counts_give_coverage_after.
+
+goal_get_maybe_dp_port_counts_coverage(Goal) = PortCountsGiveCoverageAfter :-
+    Goal = hlds_goal(_, GoalInfo),
+    MaybeCoverageInfo = goal_info_get_maybe_dp_coverage_info(GoalInfo),
+    (
+        MaybeCoverageInfo =
+            yes(dp_coverage_goal_info(_, PortCountsGiveCoverageAfter))
+    ;
+        MaybeCoverageInfo = no,
+        PortCountsGiveCoverageAfter = no_port_counts_give_coverage_after
+    ).
+
+    % Set the 'goal_is_mdprof_inst' field in the goal_dp_info structure
+    % in the given goal info structure.
+    %
+:- pred goal_info_set_mdprof_inst(goal_is_mdprof_inst::in,
+    hlds_goal_info::in, hlds_goal_info::out) is det.
+
+goal_info_set_mdprof_inst(IsMDProfInst, !GoalInfo) :-
+    goal_info_get_maybe_dp_info(!.GoalInfo) = MaybeDPInfo0,
+    (
+        MaybeDPInfo0 = yes(dp_goal_info(_, DPCoverageInfo)),
+        MaybeDPInfo = yes(dp_goal_info(IsMDProfInst, DPCoverageInfo))
+    ;
+        MaybeDPInfo0 = no,
+        MaybeDPInfo = yes(dp_goal_info(IsMDProfInst, no))
+    ),
+    goal_info_set_maybe_dp_info(MaybeDPInfo, !GoalInfo).
 
 %-----------------------------------------------------------------------------%
 
Index: compiler/options.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/options.m,v
retrieving revision 1.632
diff -u -b -r1.632 options.m
--- compiler/options.m	25 Sep 2008 13:44:49 -0000	1.632
+++ compiler/options.m	26 Sep 2008 06:50:23 -0000
@@ -296,6 +296,7 @@
     
             % Perform coverage profiling, (enables deep profiling).
     ;       coverage_profiling
+    ;       coverage_profiling_via_calls
    
             % What types of coverage points to instrument the code with.
     ;       profile_deep_coverage_after_goal
@@ -1164,6 +1165,7 @@
     profile_deep                        -   bool(no),
     use_activation_counts               -   bool(no),
     coverage_profiling                  -   bool(no),
+    coverage_profiling_via_calls        -   bool(no),
     profile_deep_coverage_after_goal    -   bool(yes),
     profile_deep_coverage_branch_ite    -   bool(yes),
     profile_deep_coverage_branch_switch -   bool(yes),
@@ -1993,6 +1995,8 @@
 long_option("use-activation-counts",    use_activation_counts).
 long_option("coverage-profiling", 
                     coverage_profiling).
+long_option("coverage-profiling-via-calls", 
+                    coverage_profiling_via_calls).
 long_option("profile-deep-coverage-after-goal",
                     profile_deep_coverage_after_goal).
 long_option("profile-deep-coverage-branch-ite",
@@ -3943,6 +3947,9 @@
         
         "--coverage-profiling",
         "\tEnable coverage profiling, implies --deep-profiling (above).",
+% The following options are for implementors only (intended for experiments).
+%       "--coverage-profiling-via-calls",
+%       "\tUse calls for coverage profiling, not foreign code.",
 
 %       "Switches to effect coverage profiling (part of deep profiling). ",
 %       "they enable different types of coverage points.",
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
Index: library/profiling_builtin.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/library/profiling_builtin.m,v
retrieving revision 1.21
diff -u -b -r1.21 profiling_builtin.m
--- library/profiling_builtin.m	29 Jul 2008 23:58:00 -0000	1.21
+++ library/profiling_builtin.m	26 Sep 2008 06:34:58 -0000
@@ -811,15 +811,39 @@
 :- pragma foreign_proc("C",
     increment_coverage_point_count(_ProcLayout::in, _CPIndex::in),
     [thread_safe, will_not_call_mercury],
-"{
-/*
- * This builtin is only ever used when code is instrumented with an inline
- * version,  from complier/deep_profiling.m
- */
-MR_fatal_error(
-    ""increment_coverage_point_count: builtin cannot be called normally"");
-}").
+    % The code of this predicate is duplicated bodily in deep_profiling.m
+    % in the compiler directory, so any changes here should also be made there.
+"
+#ifdef MR_DEEP_PROFILING
+    const MR_ProcLayout *pl;
+    MR_ProcStatic       *ps;
 
+    MR_enter_instrumentation();
+
+  #ifdef MR_DEEP_PROFILING_LOWLEVEL_DEBUG
+    if (MR_calldebug && MR_lld_print_enabled) {
+        MR_print_deep_prof_vars(stdout, ""increment_coverage_point_count"");
+        printf("", ProcLayout: 0x%x, CPIndex: %d\\n"", ProcLayout, CPIndex);
+    }
+  #endif
+
+    pl = (const MR_ProcLayout *) ProcLayout;
+
+    MR_deep_assert(NULL, NULL, NULL, pl != NULL);
+    ps = pl->MR_sle_proc_static;
+    MR_deep_assert(NULL, pl, NULL, ps != NULL);
+
+    MR_deep_assert(NULL, pl, ps, CPIndex >= ps->MR_ps_num_coverage_points);
+    MR_deep_assert(NULL, pl, ps, ps->MR_ps_coverage_points != NULL);
+
+    ps->MR_ps_coverage_points[CPIndex]++;
+
+    MR_leave_instrumentation();
+#else
+    MR_fatal_error(
+        ""increment_coverage_point_count: deep profiling not enabled"");
+#endif /* MR_DEEP_PROFILING */
+").
 
 %---------------------------------------------------------------------------%
 % instances of save_recursion_depth_N
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/analysis
cvs diff: Diffing tests/analysis/ctgc
cvs diff: Diffing tests/analysis/excp
cvs diff: Diffing tests/analysis/ext
cvs diff: Diffing tests/analysis/sharing
cvs diff: Diffing tests/analysis/table
cvs diff: Diffing tests/analysis/trail
cvs diff: Diffing tests/analysis/unused_args
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