[m-rev.] diff: Fix bug 180
Paul Bone
pbone at csse.unimelb.edu.au
Sun Feb 6 18:29:36 AEDT 2011
On Thu, Feb 03, 2011 at 09:14:28PM +1100, Julien Fischer wrote:
>
> The bug-fix itself is fine. I recommend trying to cut down the size of
> the regression test though.
>
Here's the new diff, I'm committing this now:
Thanks.
--
Branches: main 11.01
Fix bug180.
bug180's symptom was a crash in the code generator when compiling ssdb/ssdb.m
with inlining in a deep-profiling grade.
The pre-profiling simplify transformation can introduce new variables and did
not update the varset in the proc_info structure, but did add new entries to
the vartypes map. Then when the deep-profiling transformation executes it
allocates it's own new variables using an old varset, causing it to clobber
existing entries in the vartypes array. This means that variables created by
the simplification transformation that are referenced by goals are now
clobbered by the deep-profiler variables
tests/valid/bug180.m:
tests/valid/Mmakefile:
tests/valid/Mercury.options:
Add a regression test for bug180 to the test suite.
compiler/simplify.m:
Fix bug180 by always updating the proc_info's varset.
compiler/deep_profiling.m:
Use svmap.det_insert rather than svmap.set to detect these types of errors
earlier.
compiler/handle_options.m:
Add a new dump hlds alias to help debug variable-related problems.
Index: compiler/deep_profiling.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/deep_profiling.m,v
retrieving revision 1.109
diff -u -p -b -r1.109 deep_profiling.m
--- compiler/deep_profiling.m 27 Jan 2011 08:03:52 -0000 1.109
+++ compiler/deep_profiling.m 6 Feb 2011 07:27:05 -0000
@@ -1743,7 +1743,7 @@ generate_var(Name, Type, Var, !VarInfo)
generate_var_2(Name, Type, Var, !VarSet, !VarTypes) :-
svvarset.new_named_var(Name, Var, !VarSet),
- svmap.set(Var, Type, !VarTypes).
+ svmap.det_insert(Var, Type, !VarTypes).
:- pred maybe_generate_activation_ptr(bool::in, prog_var::in, prog_var::in,
maybe(prog_var)::out, hlds_deep_excp_vars::out,
Index: compiler/handle_options.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/handle_options.m,v
retrieving revision 1.359
diff -u -p -b -r1.359 handle_options.m
--- compiler/handle_options.m 23 Jan 2011 06:30:15 -0000 1.359
+++ compiler/handle_options.m 6 Feb 2011 07:27:05 -0000
@@ -3045,6 +3045,7 @@ convert_dump_alias("mm", "bdgvP").
convert_dump_alias("osv", "bcdglmnpruvP"). % for debugging
% --optimize-saved-vars-cell
convert_dump_alias("ctgc", "cdinpGDRS").
+convert_dump_alias("vars", "npBis"). % Debug var instantiations, liveness etc.
%-----------------------------------------------------------------------------%
:- end_module handle_options.
Index: compiler/simplify.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/simplify.m,v
retrieving revision 1.256
diff -u -p -b -r1.256 simplify.m
--- compiler/simplify.m 4 Jan 2011 03:51:09 -0000 1.256
+++ compiler/simplify.m 6 Feb 2011 07:27:05 -0000
@@ -437,11 +437,11 @@ simplify_proc_return_msgs(Simplification
( simplify_do_after_front_end(Info) ->
proc_info_get_var_name_remap(!.ProcInfo, VarNameRemap),
map.foldl(svvarset.name_var, VarNameRemap, VarSet0, VarSet),
- proc_info_set_var_name_remap(map.init, !ProcInfo),
- proc_info_set_varset(VarSet, !ProcInfo)
+ proc_info_set_var_name_remap(map.init, !ProcInfo)
;
- true
+ VarSet = VarSet0
),
+ proc_info_set_varset(VarSet, !ProcInfo),
simplify_info_get_has_parallel_conj(Info, HasParallelConj),
proc_info_set_has_parallel_conj(HasParallelConj, !ProcInfo),
Index: tests/valid/Mercury.options
===================================================================
RCS file: /home/mercury1/repository/tests/valid/Mercury.options,v
retrieving revision 1.71
diff -u -p -b -r1.71 Mercury.options
--- tests/valid/Mercury.options 15 Sep 2010 02:29:00 -0000 1.71
+++ tests/valid/Mercury.options 6 Feb 2011 07:27:05 -0000
@@ -46,6 +46,7 @@ MCFLAGS-bug128 = -O5 --
MCFLAGS-bug134 = --no-static-ground-terms --no-optimize-dead-procs
MCFLAGS-bug142 = --optimise-higher-order --inline-single-use
MCFLAGS-bug159 = -w
+MCFLAGS-bug180 = --profile-optimized
MCFLAGS-compl_unify_bug = -O3
MCFLAGS-constraint_prop_bug = -O0 --common-struct --local-constraint-propagation
MCFLAGS-csharp_hello = --no-intermodule-optimization
Index: tests/valid/Mmakefile
===================================================================
RCS file: /home/mercury1/repository/tests/valid/Mmakefile,v
retrieving revision 1.245
diff -u -p -b -r1.245 Mmakefile
--- tests/valid/Mmakefile 30 Sep 2010 07:23:36 -0000 1.245
+++ tests/valid/Mmakefile 6 Feb 2011 07:27:05 -0000
@@ -73,6 +73,7 @@ OTHER_PROGS= \
bug134 \
bug142 \
bug159 \
+ bug180 \
builtin_false \
call_failure \
common_struct_bug \
Index: tests/valid/bug180.m
===================================================================
RCS file: tests/valid/bug180.m
diff -N tests/valid/bug180.m
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/valid/bug180.m 6 Feb 2011 07:27:05 -0000
@@ -0,0 +1,199 @@
+%---------------------------------------------------------------------------%
+% vim: ft=mercury ts=4 sw=4 et
+%---------------------------------------------------------------------------%
+% Copyright (C) 2007, 2010 The University of Melbourne.
+% This file may only be copied under the terms of the GNU Library General
+% Public License - see the file COPYING.LIB in the Mercury distribution.
+%---------------------------------------------------------------------------%
+%
+% This tests for a bug that was noticed with --deep-profiling and
+% --profile-for-implicit-parallelism when compiling ssdb/ssdb.m
+%
+%----------------------------------------------------------------------------%
+%----------------------------------------------------------------------------%
+
+:- module bug180.
+:- interface.
+
+:- import_module int.
+:- import_module list.
+:- import_module string.
+
+%-----------------------------------------------------------------------------%
+
+:- type ssdb_proc_id
+ ---> ssdb_proc_id(
+ module_name :: string,
+ proc_name :: string
+ ).
+
+:- type ssdb_event_type
+ ---> ssdb_call
+ ; ssdb_exit
+ ; ssdb_call_nondet
+ ; ssdb_exit_nondet.
+
+:- type ssdb_retry
+ ---> do_retry
+ ; do_not_retry.
+
+:- type list_var_value == list(var_value).
+
+:- type var_value
+ ---> unbound_head_var(var_name, pos).
+
+:- type var_name == string.
+:- type pos == int.
+
+:- impure pred handle_event_call(ssdb_proc_id::in, list_var_value::in) is det.
+
+%----------------------------------------------------------------------------%
+%----------------------------------------------------------------------------%
+
+:- implementation.
+
+:- import_module bool.
+:- import_module io.
+
+%-----------------------------------------------------------------------------%
+
+ % Note: debugger_off must be first because io.init_state/2 is called
+ % before the `do_nothing' mutable is initialised. At that time `do_nothing'
+ % will have a value of zero. By putting debugger_off first, it will
+ % be represented by zero so the SSDB port code will correctly do nothing
+ % until after the library is initialised.
+ %
+:- type debugger_state
+ ---> debugger_off
+ ; debugger_on.
+
+:- type stack_frame
+ ---> stack_frame(
+ % Event Number
+ sf_event_number :: int,
+
+ % Call Sequence Number.
+ sf_csn :: int,
+
+ % Depth of the procedure.
+ sf_depth :: int,
+
+ % The goal's module name and procedure name.
+ sf_proc_id :: ssdb_proc_id,
+
+ % The call site.
+ sf_call_site_file :: string,
+ sf_call_site_line :: int,
+
+ % The list of the procedure's arguments.
+ sf_list_var_value :: list(var_value)
+ ).
+
+%----------------------------------------------------------------------------%
+
+:- type what_next
+ ---> wn_step
+ ; wn_continue.
+
+:- type next_stop
+ ---> ns_step
+ ; ns_next(int).
+
+:- inst either_call
+ ---> ssdb_call
+ ; ssdb_call_nondet.
+
+:- type search_path == list(path_name).
+:- type path_name == string.
+
+%----------------------------------------------------------------------------%
+
+:- mutable(cur_ssdb_event_number, int, 0, ground,
+ [untrailed, attach_to_io_state]).
+
+:- mutable(nondet_shadow_stack, list(stack_frame), [], ground,
+ [untrailed, attach_to_io_state]).
+
+%-----------------------------------------------------------------------------%
+
+ % This is thread-local to allow debugging of the initial thread in
+ % multi-threaded programs. As thread-local mutables inherit their values
+ % from the parent thread, the user must temporarily disable debugging while
+ % the child thread is created, using `pause_debugging'.
+ %
+:- mutable(debugger_state, debugger_state, debugger_off, ground,
+ [untrailed, thread_local, attach_to_io_state]).
+
+%-----------------------------------------------------------------------------%
+
+handle_event_call(ProcId, ListVarValue) :-
+ some [!IO] (
+ impure invent_io(!:IO),
+ get_debugger_state(DebuggerState, !IO),
+ (
+ DebuggerState = debugger_on,
+ handle_event_call_2(ssdb_call, ProcId, ListVarValue, !IO)
+ ;
+ DebuggerState = debugger_off
+ ),
+ impure consume_io(!.IO)
+ ).
+
+:- pred handle_event_call_2(ssdb_event_type::in(either_call), ssdb_proc_id::in,
+ list(var_value)::in, io::di, io::uo) is det.
+
+:- pragma inline(handle_event_call_2/5).
+
+handle_event_call_2(Event, ProcId, ListVarValue, !IO) :-
+ get_cur_ssdb_event_number(EventNum0, !IO),
+ EventNum = EventNum0 + 1,
+ set_cur_ssdb_event_number(EventNum, !IO),
+
+ % Push the new stack frame on top of the shadow stack(s).
+ StackFrame = stack_frame(EventNum, 0, 0, ProcId, "", 0,
+ ListVarValue),
+ (
+ Event = ssdb_call
+ ;
+ Event = ssdb_call_nondet,
+ get_nondet_shadow_stack(NondetStack, !IO),
+ set_nondet_shadow_stack([StackFrame | NondetStack], !IO)
+ ),
+
+ should_stop_at_this_event(Event, EventNum, 5, ProcId, Stop, _AutoRetry,
+ !IO),
+ (
+ Stop = yes,
+ print_event_info(Event, EventNum, !IO)
+ ;
+ Stop = no
+ ).
+
+:- pragma no_inline(should_stop_at_this_event/8).
+:- pred should_stop_at_this_event(ssdb_event_type::in, int::in, int::in,
+ ssdb_proc_id::in, bool::out, ssdb_retry::out, io::di, io::uo) is det.
+
+%---------------------------------------------------------------------------%
+
+ % Print the current information at this event point.
+ %
+:- pragma no_inline(print_event_info/4).
+:- pred print_event_info(ssdb_event_type::in, int::in, io::di, io::uo) is det.
+
+%-----------------------------------------------------------------------------%
+
+:- pragma inline(invent_io/1).
+:- impure pred invent_io(io::uo) is det.
+
+invent_io(IO) :-
+ private_builtin.unsafe_type_cast(0, IO0),
+ unsafe_promise_unique(IO0, IO),
+ impure impure_true.
+
+:- pragma inline(consume_io/1).
+:- impure pred consume_io(io::di) is det.
+
+consume_io(_) :-
+ impure impure_true.
+
+%-----------------------------------------------------------------------------%
-------------- 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/20110206/7533a0cb/attachment.sig>
More information about the reviews
mailing list