[m-rev.] diff: Fix a coverage propagation bug.

Paul Bone pbone at csse.unimelb.edu.au
Thu Sep 23 14:32:48 AEST 2010


Fix a coverage propagation bug.

Coverage propagation only relies on coverage points for code that it cannot
infer coverage from call site port counts alone.  However since using dynamic
coverage points I had not updated this code to also use port counts from
dynamic call sites rather than static call sites.

This patch fixes this by using port counts from dynamic call sites when doing
coverage propagation with dynamic coverage points.

deep_profiler/coverage.m:
    Introduce a new type calls_and_exits which stores the number of calls and
    exits for a given call site.

    procrep_annotate_with_coverage now uses calls_and_exits rather than
    own_prof_info structures to represent call sites.

deep_profiler/create_report.m:
    Use port counts from dynamic call sites when creating dynamic coverage
    reports.

deep_profiler/mdprof_test.m:
    Add functionality that allows us to test creation of dynamic call site 

Index: deep_profiler/coverage.m
===================================================================
RCS file: /home/mercury1/repository/mercury/deep_profiler/coverage.m,v
retrieving revision 1.6
diff -u -p -b -r1.6 coverage.m
--- deep_profiler/coverage.m	21 Sep 2010 01:09:16 -0000	1.6
+++ deep_profiler/coverage.m	23 Sep 2010 00:33:45 -0000
@@ -75,10 +75,19 @@
 
 %----------------------------------------------------------------------------%
 
+    % The coverage of a call site can be expressed as the number of calls and
+    % exits at that call site.
+    %
+:- type calls_and_exits
+    --->    calls_and_exits(
+                cae_calls           :: int,
+                cae_exits           :: int
+            ).
+
     % Annotate the program representation structure with coverage information.
     %
 :- pred procrep_annotate_with_coverage(own_prof_info::in,
-    map(goal_path, own_prof_info)::in, map(goal_path, coverage_point)::in,
+    map(goal_path, calls_and_exits)::in, map(goal_path, coverage_point)::in,
     map(goal_path, coverage_point)::in, proc_rep::in,
     maybe_error(proc_rep(coverage_info))::out) is det.
 
@@ -203,7 +212,7 @@ procrep_annotate_with_coverage(OwnProf, 
 :- type coverage_reference_info
     --->    coverage_reference_info(
                 cri_proc                    :: string_proc_label,
-                cri_call_sites              :: map(goal_path, own_prof_info),
+                cri_call_sites              :: map(goal_path, calls_and_exits),
                 cri_solns_coverage_points   :: map(goal_path, coverage_point),
                 cri_branch_coverage_points  :: map(goal_path, coverage_point)
             ).
@@ -257,13 +266,12 @@ goal_annotate_coverage(Info, GoalPath, B
             ; AtomicGoal = higher_order_call_rep(_, _)
             ; AtomicGoal = method_call_rep(_, _, _)
             ),
-            ( map.search(Info ^ cri_call_sites, GoalPath, OwnProfInfo) ->
+            ( map.search(Info ^ cri_call_sites, GoalPath, CallsAndExits) ->
                 % Entry due to redo is not counted at the point before the
                 % goal, it is represented when the number of exists is greater
                 % than the number of calls. XXX This won't work with nondet
                 % code, which should be fixed in the future.
-                Calls = calls(OwnProfInfo),
-                Exits = exits(OwnProfInfo),
+                CallsAndExits = calls_and_exits(Calls, Exits),
                 require(unify(Before, before_coverage(Calls)),
                   "Coverage before call doesn't match calls port on call site"),
                 After0 = after_coverage(Exits)
Index: deep_profiler/create_report.m
===================================================================
RCS file: /home/mercury1/repository/mercury/deep_profiler/create_report.m,v
retrieving revision 1.24
diff -u -p -b -r1.24 create_report.m
--- deep_profiler/create_report.m	22 Sep 2010 12:56:53 -0000	1.24
+++ deep_profiler/create_report.m	23 Sep 2010 04:05:28 -0000
@@ -1143,8 +1143,18 @@ create_static_procrep_coverage_report(De
         deep_lookup_ps_coverage(Deep, PSPtr, StaticCoverage),
         MaybeCoveragePoints =
             static_coverage_maybe_get_coverage_points(StaticCoverage),
-        maybe_create_procrep_coverage_report(Deep, PSPtr, MaybeCoveragePoints,
-            MaybeReport)
+        
+        % Gather call site information.
+        deep_lookup_proc_statics(Deep, PSPtr, PS),
+        CallSitesArray = PS ^ ps_sites,
+        CallSitesMap = array.foldl(add_ps_calls_and_exits_to_map(Deep),
+            CallSitesArray, map.init),
+        
+        % Gather information about the procedure.
+        deep_lookup_ps_own(Deep, PSPtr, Own),
+
+        maybe_create_procrep_coverage_report(Deep, PSPtr, Own,
+            MaybeCoveragePoints, CallSitesMap, MaybeReport)
     ;
         PSPtr = proc_static_ptr(PSId),
         MaybeReport = error(
@@ -1156,8 +1166,21 @@ create_dynamic_procrep_coverage_report(D
         deep_lookup_proc_dynamics(Deep, PDPtr, PD),
         PSPtr = PD ^ pd_proc_static,
         MaybeCoveragePoints = PD ^ pd_maybe_coverage_points,
-        maybe_create_procrep_coverage_report(Deep, PSPtr, MaybeCoveragePoints,
-            MaybeReport)
+        
+        % Gather call site information.
+        deep_lookup_proc_statics(Deep, PSPtr, PS),
+        StaticCallSitesArray = PS ^ ps_sites,
+        DynamicCallSitesArray = PD ^ pd_sites,
+        foldl_corresponding(
+            add_pd_calls_and_exits_to_map(Deep),
+            to_list(StaticCallSitesArray), to_list(DynamicCallSitesArray), 
+            map.init, CallSitesMap),
+        
+        % Gather information about the procedure.
+        deep_lookup_pd_own(Deep, PDPtr, Own),
+
+        maybe_create_procrep_coverage_report(Deep, PSPtr, Own,
+            MaybeCoveragePoints, CallSitesMap, MaybeReport)
     ;
         PDPtr = proc_dynamic_ptr(PDId),
         MaybeReport = error(
@@ -1165,12 +1188,14 @@ create_dynamic_procrep_coverage_report(D
     ).
 
 :- pred maybe_create_procrep_coverage_report(deep::in, proc_static_ptr::in,
-    maybe(array(int))::in, maybe_error(procrep_coverage_info)::out) is det.
+    own_prof_info::in, maybe(array(int))::in, 
+    map(goal_path, calls_and_exits)::in,
+    maybe_error(procrep_coverage_info)::out) is det.
 
-maybe_create_procrep_coverage_report(_, _, no, error(Error)) :-
+maybe_create_procrep_coverage_report(_, _, _, no, _, error(Error)) :-
     Error = "No coverage information available".
-maybe_create_procrep_coverage_report(Deep, PSPtr, yes(CoveragePointsArray),
-        MaybeReport) :-
+maybe_create_procrep_coverage_report(Deep, PSPtr, Own, yes(CoveragePointsArray),
+        CallSitesMap, MaybeReport) :-
     deep_lookup_proc_statics(Deep, PSPtr, PS),
     coverage_point_arrays_to_list(PS ^ ps_coverage_point_infos,
         CoveragePointsArray, CoveragePoints),
@@ -1180,13 +1205,6 @@ maybe_create_procrep_coverage_report(Dee
         MaybeReport = error(Error)
     ;
         MaybeProcRep0 = ok(ProcRep0),
-        % Information about the procedure.
-        deep_lookup_ps_own(Deep, PSPtr, Own),
-
-        % Gather call site information.
-        CallSitesArray = PS ^ ps_sites,
-        CallSitesMap = array.foldl(add_ps_own_info_to_map(Deep),
-            CallSitesArray, map.init),
 
         foldl2(add_coverage_point_to_map, 
             CoveragePoints, map.init, SolnsCoveragePointMap,
@@ -1204,14 +1222,54 @@ maybe_create_procrep_coverage_report(Dee
         )
     ).
 
-:- func add_ps_own_info_to_map(deep, call_site_static_ptr,
-    map(goal_path, own_prof_info)) = map(goal_path, own_prof_info).
+:- func add_ps_calls_and_exits_to_map(deep, call_site_static_ptr,
+    map(goal_path, calls_and_exits)) = map(goal_path, calls_and_exits).
 
-add_ps_own_info_to_map(Deep, CSSPtr, !.Map) = !:Map :-
+add_ps_calls_and_exits_to_map(Deep, CSSPtr, !.Map) = !:Map :-
     lookup_call_site_statics(Deep ^ call_site_statics, CSSPtr, CSS),
     goal_path_from_string_det(CSS ^ css_goal_path, GoalPath),
     lookup_css_own(Deep ^ css_own, CSSPtr, Own),
-    svmap.det_insert(GoalPath, Own, !Map).
+    svmap.det_insert(GoalPath, calls_and_exits(calls(Own), exits(Own)), !Map).
+
+:- pred add_pd_calls_and_exits_to_map(deep::in, call_site_static_ptr::in,
+    call_site_array_slot::in, 
+    map(goal_path, calls_and_exits)::in, map(goal_path, calls_and_exits)::out)
+    is det.
+
+add_pd_calls_and_exits_to_map(Deep, CSSPtr, Slot, !Map) :-
+    (
+        Slot = slot_normal(CSDPtr),
+        csd_get_calls_and_exits(Deep, CSDPtr, Calls, Exits)
+    ;
+        Slot = slot_multi(_, CSDs),
+        foldl2(csd_get_calls_and_exits_accum(Deep), CSDs, 
+            0, Calls, 0, Exits)
+    ),
+    deep_lookup_call_site_statics(Deep, CSSPtr, CSS),
+    goal_path_from_string_det(CSS ^ css_goal_path, GoalPath),
+    svmap.det_insert(GoalPath, calls_and_exits(Calls, Exits), !Map).
+
+:- pred csd_get_calls_and_exits(deep::in, call_site_dynamic_ptr::in, 
+    int::out, int::out) is det.
+
+csd_get_calls_and_exits(Deep, CSDPtr, Calls, Exits) :-
+    ( valid_call_site_dynamic_ptr(Deep, CSDPtr) ->
+        deep_lookup_call_site_dynamics(Deep, CSDPtr, CSD),
+        Own = CSD ^ csd_own_prof,
+        Calls = calls(Own),
+        Exits = exits(Own)
+    ;
+        % If the CSD is invalid then this call site was never called.
+        Calls = 0,
+        Exits = 0
+    ).
+
+:- pred csd_get_calls_and_exits_accum(deep::in, call_site_dynamic_ptr::in,
+    int::in, int::out, int::in, int::out) is det.
+
+csd_get_calls_and_exits_accum(Deep, CSDPtr, Calls0, Calls0 + NewCalls, 
+        Exits0, Exits0 + NewExits) :-
+    csd_get_calls_and_exits(Deep, CSDPtr, NewCalls, NewExits).
 
 :- pred add_coverage_point_to_map(coverage_point::in,
     map(goal_path, coverage_point)::in, map(goal_path, coverage_point)::out,
Index: deep_profiler/mdprof_test.m
===================================================================
RCS file: /home/mercury1/repository/mercury/deep_profiler/mdprof_test.m,v
retrieving revision 1.23
diff -u -p -b -r1.23 mdprof_test.m
--- deep_profiler/mdprof_test.m	22 Sep 2010 12:56:54 -0000	1.23
+++ deep_profiler/mdprof_test.m	23 Sep 2010 01:07:02 -0000
@@ -236,13 +236,25 @@ test_server(Pref, Deep, Options, !IO) :-
     % test_cliques(1, NumCliques, DirName, Pref, Deep, !IO),
     % test_procs(1, NumProcStatics, DirName, Pref, Deep, !IO).
     
-    lookup_bool_option(Options, procrep_coverage, ProcrepCoverage),
+    lookup_bool_option(Options, static_procrep_coverage, StaticProcrepCoverage),
     (
-        ProcrepCoverage = yes,
+        StaticProcrepCoverage = yes,
         array.max(Deep ^ proc_statics, NumProcStatics),
-        test_procrep_coverages(1, NumProcStatics, Pref, Deep, Options, !IO)
+        test_procrep_static_coverages(1, NumProcStatics, Pref, Deep, Options, 
+            !IO)
     ;
-        ProcrepCoverage = no
+        StaticProcrepCoverage = no
+    ),
+
+    lookup_bool_option(Options, dynamic_procrep_coverage,
+        DynamicProcrepCoverage),
+    (
+        DynamicProcrepCoverage = yes,
+        array.max(Deep ^ proc_dynamics, NumProcDynamics),
+        test_procrep_dynamic_coverages(1, NumProcDynamics, Pref, Deep, Options,
+            !IO)
+    ;
+        DynamicProcrepCoverage = no
     ),
 
     lookup_bool_option(Options, recursion_types_histogram, RecTypesHistogram),
@@ -277,15 +289,28 @@ test_procs(Cur, Max, Options, Pref, Deep
         true
     ).
 
-:- pred test_procrep_coverages(int::in, int::in, preferences::in, deep::in,
-    option_table::in, io::di, io::uo) is cc_multi.
+:- pred test_procrep_static_coverages(int::in, int::in, preferences::in,
+    deep::in, option_table::in, io::di, io::uo) is cc_multi.
 
-test_procrep_coverages(Cur, Max, Pref, Deep, Options, !IO) :-
+test_procrep_static_coverages(Cur, Max, Pref, Deep, Options, !IO) :-
     ( Cur =< Max ->
         try_exec(deep_cmd_static_procrep_coverage(proc_static_ptr(Cur)), Pref, 
             Deep, HTML, !IO),
-        write_test_html(Options, "procrep_coverage", Cur, HTML, !IO),
-        test_procrep_coverages(Cur + 1, Max, Pref, Deep, Options, !IO)
+        write_test_html(Options, "procrep_dynamic_coverage", Cur, HTML, !IO),
+        test_procrep_static_coverages(Cur + 1, Max, Pref, Deep, Options, !IO)
+    ;
+        true
+    ).
+
+:- pred test_procrep_dynamic_coverages(int::in, int::in, preferences::in,
+    deep::in, option_table::in, io::di, io::uo) is cc_multi.
+
+test_procrep_dynamic_coverages(Cur, Max, Pref, Deep, Options, !IO) :-
+    ( Cur =< Max ->
+        try_exec(deep_cmd_dynamic_procrep_coverage(proc_dynamic_ptr(Cur)), Pref,
+            Deep, HTML, !IO),
+        write_test_html(Options, "procrep_static_coverage", Cur, HTML, !IO),
+        test_procrep_dynamic_coverages(Cur + 1, Max, Pref, Deep, Options, !IO)
     ;
         true
     ).
@@ -359,7 +384,8 @@ write_test_html(Options, BaseName, Num, 
     ;       verbose
     ;       version
     ;       verify_profile
-    ;       procrep_coverage
+    ;       static_procrep_coverage
+    ;       dynamic_procrep_coverage
     ;       recursion_types_histogram.
 
 :- type options ---> options.
@@ -385,7 +411,8 @@ long("test-dir",                    test
 long("verbose",                     verbose).
 long("version",                     version).
 long("verify-profile",              verify_profile).
-long("procrep-coverage",            procrep_coverage).
+long("static-procrep-coverage",     static_procrep_coverage).
+long("dynamic-procrep-coverage",    dynamic_procrep_coverage).
 long("recursion-types-histogram",   recursion_types_histogram).
 
 :- pred defaults(option::out, option_data::out) is multi.
@@ -400,7 +427,8 @@ defaults(test_dir,                  stri
 defaults(verbose,                   bool(no)).
 defaults(version,                   bool(no)).
 defaults(verify_profile,            bool(no)).
-defaults(procrep_coverage,          bool(no)).
+defaults(static_procrep_coverage,   bool(no)).
+defaults(dynamic_procrep_coverage,  bool(no)).
 defaults(recursion_types_histogram, bool(no)).
 
 %-----------------------------------------------------------------------------%
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 489 bytes
Desc: Digital signature
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20100923/b4138844/attachment.sig>


More information about the reviews mailing list