[m-rev.] for post-commit review: fix deep profiling problem
zs at csse.unimelb.edu.au
Mon Nov 3 17:10:16 AEDT 2008
A temporary fix for Mantis bug #87, which caused all programs compiled with
deep profiling to get a sanity check violation when writing out the deep
My detective work tracked the first reported call site that got this violation,
which is in integer.m in the library, where the predicate integer.to_string
calls the function string.++.
(a) String.++ is marked (correctly) as opt_imported.
(b) The test predicate pred_info_is_imported FAILS for opt_imported predicates.
(c) This causes make_rtti_proc_label to put "no" into the pred_is_imported
field of the rtti_proc_label for string.++.
(d) This rtti_proc_label is put into the element of the call_site_static_data
list for integer.to_string that corresponds to the call to string.++
as the rtti_proc_label of the callee.
(e) When the time comes to write out the call_site_static structures,
layout_out.m invokes make_proc_label_from_rtti to convert this
rtti_proc_label in the call_site_static_data to a plain, non-RTTI
proc_label. However, since the current module name ("integer") does not
match the name of the module that defines the function ("string") AND
the function is not marked as imported, make_user_proc_label (invoked
from make_proc_label_from_rtti) believes that this means that this module
generates code for the callee (as we do for some opt_imported procedures).
(f) For procedures for which we generate target code in a different module
than their defining module, we generate different labels for them
than the label for the code for the procedure in its own module.
We derive the name of a procedure's proc_static structure directly from
its label, so this difference holds for the names of proc_static structures
as well as for labels.
(g) This lead to the problem, which was that the call_site_static for this call
site referred to the proc_static for string.++ defined in integer.c
(as opposed to string.++ defined in string.c), since integer.c did not
actually generate code for string.++, there was no such proc_static.
This should be a link error, but it isn't, due to the requirement on unix
linkers of being compatible with Fortran's ancient semantics (specifically,
Fortran's common areas). Instead, the C compiler happily allocates memory
for the undefined symbol and fills it with zeros as if it were a
(h) The assertion violation in mercury_deep_profiling.c occurs because the
call_site_static refers to this bogus proc_static, but of course no module
writes out this proc_static.
It turns out that deep_profiling.m and the code generator both use the same
algorithm to turn the pred_proc_id of string.++ into a label, but the relevant
input to this algorithm, the import_status of string.++, changes between
the invocation of the deep_profiling transformation and the code generator.
The import_status is changed by the dead procedure elimination pass;
it shouldn't be, but it is. The reason why the bug started showing up recently
is my recent diff that moved the dead_procedure elimination pass after the deep
The temporary fix is to make sure that the deep profiling pass sees the same
status for each procedure as the code generator, by making it invoke the dead
procedure elimination pass itself.
The right fix is to change the representation of import_status, designing it
properly for the first time, but that will take more time. This redesign should
eliminate the need for the dead procedure eliminate pass to change any
Make the fix described above.
Print progress message only with -V, not -v, since this is what
all the other passes do.
When writing out a procedure's deep profiling information, include
the data structures from which we later construct the procedure's
proc_static and call_site_static structures.
Replace some if-then-elses with switches.
Don't export a function that is not used anywhere else.
Delete a predicate to avoid an ambiguity.
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
RCS file: /home/mercury/mercury1/repository/mercury/compiler/code_util.m,v
retrieving revision 1.184
diff -u -b -r1.184 code_util.m
--- compiler/code_util.m 3 Sep 2008 03:32:18 -0000 1.184
+++ compiler/code_util.m 2 Nov 2008 00:44:46 -0000
@@ -108,10 +108,13 @@
ProcAddr = make_entry_label_from_rtti(RttiProcLabel, Immed).
make_entry_label_from_rtti(RttiProcLabel, Immed) = ProcAddr :-
- ( RttiProcLabel ^ proc_is_imported = yes ->
+ ProcIsImported = RttiProcLabel ^ proc_is_imported,
+ ProcIsImported = yes,
ProcLabel = make_proc_label_from_rtti(RttiProcLabel),
ProcAddr = code_imported_proc(ProcLabel)
+ ProcIsImported = no,
Label = make_local_entry_label_from_rtti(RttiProcLabel, Immed),
ProcAddr = code_label(Label)
@@ -129,9 +132,12 @@
% If we want to define the label or use it to put it into a data
% structure, a label that is usable only within the current C module
% won't do.
- ( RttiProcLabel ^ proc_is_exported = yes ->
+ ProcIsExported = RttiProcLabel ^ proc_is_exported,
+ ProcIsExported = yes,
EntryType = entry_label_exported
+ ProcIsExported = no,
EntryType = entry_label_local
Label = entry_label(EntryType, ProcLabel)
RCS file: /home/mercury/mercury1/repository/mercury/compiler/deep_profiling.m,v
retrieving revision 1.91
diff -u -b -r1.91 deep_profiling.m
--- compiler/deep_profiling.m 11 Oct 2008 02:44:13 -0000 1.91
+++ compiler/deep_profiling.m 2 Nov 2008 01:31:14 -0000
@@ -52,6 +52,7 @@
:- import_module parse_tree.prog_data.
:- import_module parse_tree.prog_type.
:- import_module transform_hlds.
+:- import_module transform_hlds.dead_proc_elim.
:- import_module transform_hlds.dependency_graph.
:- import_module assoc_list.
@@ -74,6 +75,11 @@
apply_deep_profiling_transformation(!ModuleInfo, !IO) :-
+ % XXX The dead proc elimination pass changes the status of opt_imported
+ % predicates, which changes what labels they get generated. The
+ % call_site_static structures we generate must match the labels created
+ % during code generation.
+ dead_proc_elim(elim_opt_imported, !ModuleInfo, _ElimSpecs),
@@ -482,10 +488,10 @@
- globals.lookup_bool_option(Globals, verbose, Verbose),
+ globals.lookup_bool_option(Globals, very_verbose, VeryVerbose),
ProcName = pred_proc_id_pair_to_string(ModuleInfo, PredId, ProcId),
- string.format("Deep profiling: %s\n", [s(ProcName)]),
+ string.format("%% Deep profiling: %s\n", [s(ProcName)]),
deep_prof_transform_proc(ModuleInfo, proc(PredId, ProcId),
RCS file: /home/mercury/mercury1/repository/mercury/compiler/hlds_out.m,v
retrieving revision 1.458
diff -u -b -r1.458 hlds_out.m
--- compiler/hlds_out.m 18 Sep 2008 12:41:54 -0000 1.458
+++ compiler/hlds_out.m 30 Oct 2008 13:46:30 -0000
@@ -4144,7 +4144,8 @@
MaybeDeepLayout = yes(DeepLayout),
- DeepLayout = hlds_deep_layout(_, ExcpVars),
+ DeepLayout = hlds_deep_layout(ProcStatic, ExcpVars),
+ write_hlds_proc_static(ProcStatic, !IO),
ExcpVars = hlds_deep_excp_vars(TopCSD, MiddleCSD,
io.write_string("% deep layout info: ", !IO),
@@ -4241,6 +4242,74 @@
+:- pred write_hlds_proc_static(hlds_proc_static::in, io::di, io::uo) is det.
+write_hlds_proc_static(ProcStatic, !IO) :-
+ ProcStatic = hlds_proc_static(FileName, LineNumber,
+ InInterface, CallSiteStatics, CoveragePoints),
+ io.write_string("% proc static filename: ", !IO),
+ io.write_string(FileName, !IO),
+ io.write_string("% proc static line number: ", !IO),
+ io.write_int(LineNumber, !IO),
+ io.write_string("% proc static is interface: ", !IO),
+ io.write(InInterface, !IO),
+ list.foldl2(write_hlds_ps_call_site, CallSiteStatics, 0, _, !IO),
+ list.foldl2(write_hlds_ps_coverage_point, CoveragePoints, 0, _, !IO).
+:- pred write_hlds_ps_call_site(call_site_static_data::in, int::in, int::out,
+ io::di, io::uo) is det.
+write_hlds_ps_call_site(CallSiteStaticData, !SlotNum, !IO) :-
+ io.format("%% call site static slot %d\n", [i(!.SlotNum)], !IO),
+ CallSiteStaticData = normal_call(CalleeRttiProcLabel, TypeSubst,
+ FileName, LineNumber, GoalPath),
+ io.write_string("% normal call to ", !IO),
+ io.write(CalleeRttiProcLabel, !IO),
+ io.format("%% type subst <%s>, goal path <%s>\n",
+ [s(TypeSubst), s(goal_path_to_string(GoalPath))], !IO),
+ io.format("%% filename <%s>, line number <%d>\n",
+ [s(FileName), i(LineNumber)], !IO)
+ CallSiteStaticData = special_call(FileName, LineNumber, GoalPath),
+ io.write_string("% special call\n", !IO)
+ CallSiteStaticData = higher_order_call(FileName, LineNumber,
+ io.write_string("% higher order call\n", !IO)
+ CallSiteStaticData = method_call(FileName, LineNumber, GoalPath),
+ io.write_string("% method call\n", !IO)
+ CallSiteStaticData = callback(FileName, LineNumber, GoalPath),
+ io.write_string("% callback\n", !IO)
+ io.format("%% filename <%s>, line number <%d>, goal path <%s>\n",
+ [s(FileName), i(LineNumber), s(goal_path_to_string(GoalPath))],
+ !:SlotNum = !.SlotNum + 1.
+:- pred write_hlds_ps_coverage_point(coverage_point_info::in,
+ int::in, int::out, io::di, io::uo) is det.
+write_hlds_ps_coverage_point(CoveragePointInfo, !SlotNum, !IO) :-
+ CoveragePointInfo = coverage_point_info(GoalPath, PointType),
+ io.format("%% coverage point slot %d: goal path <%s>, type %s\n",
+ [i(!.SlotNum), s(goal_path_to_string(GoalPath)),
+ s(coverage_point_to_string(PointType))], !IO),
+ !:SlotNum = !.SlotNum + 1.
+:- func coverage_point_to_string(cp_type) = string.
+coverage_point_to_string(cp_type_coverage_after) = "after".
+coverage_point_to_string(cp_type_branch_arm) = "branch arm".
determinism_to_string(detism_det) = "det".
determinism_to_string(detism_semi) = "semidet".
determinism_to_string(detism_non) = "nondet".
RCS file: /home/mercury/mercury1/repository/mercury/compiler/proc_label.m,v
retrieving revision 1.24
diff -u -b -r1.24 proc_label.m
--- compiler/proc_label.m 13 Aug 2007 03:01:43 -0000 1.24
+++ compiler/proc_label.m 2 Nov 2008 01:16:44 -0000
@@ -28,8 +28,6 @@
:- import_module parse_tree.
:- import_module parse_tree.prog_data.
-:- import_module bool.
% Return the id of the procedure specified by the rtti_proc_label.
:- func make_proc_label_from_rtti(rtti_proc_label) = proc_label.
@@ -38,17 +36,6 @@
:- func make_proc_label(module_info, pred_id, proc_id) = proc_label.
- % make_user_proc_label(ModuleName, PredIsImported,
- % PredOrFunc, ModuleName, PredName, Arity, ProcId) = Label:
- % Make a proc_label for a user-defined procedure.
- % The PredIsImported argument should be the result of
- % calling pred_info_is_imported.
-:- func make_user_proc_label(module_name, bool, pred_or_func, module_name,
- string, arity, proc_id) = proc_label.
% Return the id of the specified mode of the unification procedure
% for the given type.
@@ -64,6 +51,7 @@
:- import_module libs.compiler_util.
:- import_module parse_tree.prog_type.
+:- import_module bool.
:- import_module list.
:- import_module pair.
:- import_module string.
@@ -114,6 +102,17 @@
RttiProcLabel = make_rtti_proc_label(ModuleInfo, PredId, ProcId),
make_proc_label_from_rtti(RttiProcLabel) = ProcLabel.
+ % make_user_proc_label(ModuleName, PredIsImported,
+ % PredOrFunc, ModuleName, PredName, Arity, ProcId) = Label:
+ % Make a proc_label for a user-defined procedure.
+ % The PredIsImported argument should be the result of
+ % calling pred_info_is_imported.
+:- func make_user_proc_label(module_name, bool, pred_or_func,
+ module_name, string, arity, proc_id) = proc_label.
make_user_proc_label(ThisModule, PredIsImported, PredOrFunc, PredModule,
PredName, PredArity, ProcId) = ProcLabel :-
RCS file: /home/mercury/mercury1/repository/mercury/compiler/type_ctor_info.m,v
retrieving revision 1.96
diff -u -b -r1.96 type_ctor_info.m
--- compiler/type_ctor_info.m 11 Feb 2008 21:26:11 -0000 1.96
+++ compiler/type_ctor_info.m 2 Nov 2008 00:31:52 -0000
@@ -290,8 +290,12 @@
construct_type_ctor_info(TypeCtorGenInfo, ModuleInfo, RttiData) :-
TypeCtorGenInfo = type_ctor_gen_info(_TypeCtor, ModuleName, TypeName,
TypeArity, _Status, HldsDefn, UnifyPredProcId, ComparePredProcId),
- make_rtti_proc_label(UnifyPredProcId, ModuleInfo, UnifyProcLabel),
- make_rtti_proc_label(ComparePredProcId, ModuleInfo, CompareProcLabel),
+ UnifyPredProcId = proc(UnifyPredId, UnifyProcId),
+ UnifyProcLabel = make_rtti_proc_label(ModuleInfo,
+ UnifyPredId, UnifyProcId),
+ ComparePredProcId = proc(ComparePredId, CompareProcId),
+ CompareProcLabel = make_rtti_proc_label(ModuleInfo,
+ ComparePredId, CompareProcId),
@@ -457,13 +461,6 @@
impl_type_ctor("private_builtin", "ticket", 0, impl_ctor_ticket).
impl_type_ctor("table_builtin", "ml_subgoal", 0, impl_ctor_subgoal).
-:- pred make_rtti_proc_label(pred_proc_id::in, module_info::in,
- rtti_proc_label::out) is det.
-make_rtti_proc_label(PredProcId, ModuleInfo, ProcLabel) :-
- PredProcId = proc(PredId, ProcId),
- ProcLabel = make_rtti_proc_label(ModuleInfo, PredId, ProcId).
% The version of the RTTI data structures -- useful for bootstrapping.
cvs diff: Diffing compiler/notes
cvs diff: Diffing debian
cvs diff: Diffing debian/patches
cvs diff: Diffing deep_profiler
cvs diff: Diffing deep_profiler/notes
cvs diff: Diffing doc
cvs diff: Diffing extras
cvs diff: Diffing extras/base64
cvs diff: Diffing extras/cgi
cvs diff: Diffing extras/complex_numbers
cvs diff: Diffing extras/complex_numbers/samples
cvs diff: Diffing extras/complex_numbers/tests
cvs diff: Diffing extras/concurrency
cvs diff: Diffing extras/curs
cvs diff: Diffing extras/curs/samples
cvs diff: Diffing extras/curses
cvs diff: Diffing extras/curses/sample
cvs diff: Diffing extras/dynamic_linking
cvs diff: Diffing extras/error
cvs diff: Diffing extras/fixed
cvs diff: Diffing extras/gator
cvs diff: Diffing extras/gator/generations
cvs diff: Diffing extras/gator/generations/1
cvs diff: Diffing extras/graphics
cvs diff: Diffing extras/graphics/easyx
cvs diff: Diffing extras/graphics/easyx/samples
cvs diff: Diffing extras/graphics/mercury_allegro
cvs diff: Diffing extras/graphics/mercury_allegro/examples
cvs diff: Diffing extras/graphics/mercury_allegro/samples
cvs diff: Diffing extras/graphics/mercury_allegro/samples/demo
cvs diff: Diffing extras/graphics/mercury_allegro/samples/mandel
cvs diff: Diffing extras/graphics/mercury_allegro/samples/pendulum2
cvs diff: Diffing extras/graphics/mercury_allegro/samples/speed
cvs diff: Diffing extras/graphics/mercury_glut
cvs diff: Diffing extras/graphics/mercury_opengl
cvs diff: Diffing extras/graphics/mercury_tcltk
cvs diff: Diffing extras/graphics/samples
cvs diff: Diffing extras/graphics/samples/calc
cvs diff: Diffing extras/graphics/samples/gears
cvs diff: Diffing extras/graphics/samples/maze
cvs diff: Diffing extras/graphics/samples/pent
cvs diff: Diffing extras/lazy_evaluation
cvs diff: Diffing extras/lex
cvs diff: Diffing extras/lex/samples
cvs diff: Diffing extras/lex/tests
cvs diff: Diffing extras/log4m
cvs diff: Diffing extras/logged_output
cvs diff: Diffing extras/moose
cvs diff: Diffing extras/moose/samples
cvs diff: Diffing extras/moose/tests
cvs diff: Diffing extras/mopenssl
cvs diff: Diffing extras/morphine
cvs diff: Diffing extras/morphine/non-regression-tests
cvs diff: Diffing extras/morphine/scripts
cvs diff: Diffing extras/morphine/source
cvs diff: Diffing extras/net
cvs diff: Diffing extras/odbc
cvs diff: Diffing extras/posix
cvs diff: Diffing extras/posix/samples
cvs diff: Diffing extras/quickcheck
cvs diff: Diffing extras/quickcheck/tutes
cvs diff: Diffing extras/references
cvs diff: Diffing extras/references/samples
cvs diff: Diffing extras/references/tests
cvs diff: Diffing extras/solver_types
cvs diff: Diffing extras/solver_types/library
cvs diff: Diffing extras/trailed_update
cvs diff: Diffing extras/trailed_update/samples
cvs diff: Diffing extras/trailed_update/tests
cvs diff: Diffing extras/windows_installer_generator
cvs diff: Diffing extras/windows_installer_generator/sample
cvs diff: Diffing extras/windows_installer_generator/sample/images
cvs diff: Diffing extras/xml
cvs diff: Diffing extras/xml/samples
cvs diff: Diffing extras/xml_stylesheets
cvs diff: Diffing java
cvs diff: Diffing java/runtime
cvs diff: Diffing library
cvs diff: Diffing mdbcomp
cvs diff: Diffing profiler
cvs diff: Diffing robdd
cvs diff: Diffing runtime
cvs diff: Diffing runtime/GETOPT
cvs diff: Diffing runtime/machdeps
cvs diff: Diffing samples
cvs diff: Diffing samples/c_interface
cvs diff: Diffing samples/c_interface/c_calls_mercury
cvs diff: Diffing samples/c_interface/cplusplus_calls_mercury
cvs diff: Diffing samples/c_interface/mercury_calls_c
cvs diff: Diffing samples/c_interface/mercury_calls_cplusplus
cvs diff: Diffing samples/c_interface/mercury_calls_fortran
cvs diff: Diffing samples/c_interface/simpler_c_calls_mercury
cvs diff: Diffing samples/c_interface/simpler_cplusplus_calls_mercury
cvs diff: Diffing samples/c_interface/standalone_c
cvs diff: Diffing samples/diff
cvs diff: Diffing samples/muz
cvs diff: Diffing samples/rot13
cvs diff: Diffing samples/solutions
cvs diff: Diffing samples/solver_types
cvs diff: Diffing samples/tests
cvs diff: Diffing samples/tests/c_interface
cvs diff: Diffing samples/tests/c_interface/c_calls_mercury
cvs diff: Diffing samples/tests/c_interface/cplusplus_calls_mercury
cvs diff: Diffing samples/tests/c_interface/mercury_calls_c
cvs diff: Diffing samples/tests/c_interface/mercury_calls_cplusplus
cvs diff: Diffing samples/tests/c_interface/mercury_calls_fortran
cvs diff: Diffing samples/tests/c_interface/simpler_c_calls_mercury
cvs diff: Diffing samples/tests/c_interface/simpler_cplusplus_calls_mercury
cvs diff: Diffing samples/tests/diff
cvs diff: Diffing samples/tests/muz
cvs diff: Diffing samples/tests/rot13
cvs diff: Diffing samples/tests/solutions
cvs diff: Diffing samples/tests/toplevel
cvs diff: Diffing scripts
cvs diff: Diffing slice
cvs diff: Diffing ssdb
cvs diff: Diffing tests
cvs diff: Diffing tests/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