[m-dev.] for review: --suppress-trace

[Mark Anthony BROWN]

Zoltan Somogyi writes:
> 
> Implement a general mechanism for suppressing selected aspects of execution
> tracing.
> 
> compiler/trace_params.m:
> 	This new file contains two abstract data types that define the
> 	parameters of the execution tracing system, and the operations
> 	on them.
> 
> 	The two ADTs are the trace level (moved from globals.m) and a new one,
> 	trace_suppress_items, which replaces --no-trace-internal,
> 	--no-trace-redo and --no-trace-return, and provides additional
> 	capabilities requested by Erwan.
> 
> 	Basically any given port can now be suppressed, with the exception of
> 	call (because the call event's layout structure is needed for
> 	implementing redo from any other event) and excp (because they are
> 	created on the fly by exception.m, and are not put into object
> 	code in the first place).
> 
> compiler/globals.m:
> 	Delete the stuff now migrated to trace_params.m. Make
> 	trace_suppress_items part of the globals structure.
> 
> compiler/code_gen.m:
> compiler/trace.m:
> 	Allow the suppression of individual ports.
> 
> compiler/stack_layout.m:
> 	Allow the suppression of parts of layout structures.
> 
> compiler/*.m:
> 	Minor changes, mainly importing trace_params, to conform to the main
> 	modifications.
> 
> compiler/options.m:
> 	Delete --no-trace-internal, --no-trace-redo and --no-trace-return.
> 	Add --suppress-trace, but do not document it, since it is really
> 	intended only for implementors.
> 
> doc/user_guide.texi:
> 	Delete the documentation of --no-trace-internal, --no-trace-redo and
> 	--no-trace-return. Do not document it --suppress-trace, since it is
> 	really intended only for implementors. (The only reason why other three
> 	were documented is that most consumers of the users' guide then *were*
> 	implementors.)
> 
> Zoltan.
> 

This change looks good.  I have just a few comments:


> Index: compiler/trace_params.m
> ===================================================================
> RCS file: trace_params.m
> diff -N trace_params.m
> --- /dev/null	Thu Sep  2 15:00:04 1999
> +++ trace_params.m	Fri Sep 29 13:37:25 2000
> @@ -0,0 +1,282 @@
> +%-----------------------------------------------------------------------------%
> +% Copyright (C) 2000 The University of Melbourne.
> +% This file may only be copied under the terms of the GNU General
> +% Public License - see the file COPYING in the Mercury distribution.
> +%-----------------------------------------------------------------------------%
> +%
> +% File: trace_params.m.
> +%
> +% Author: zs.
> +%
> +% This module defines the parameters of execution tracing at various trace
> +% levels and with various settings of the --suppress-trace option.
> +
> +%-----------------------------------------------------------------------------%
> +
> +:- module trace_params.
> +
> +:- interface.
> +
> +:- import_module llds.
> +:- import_module bool.
> +
> +:- type trace_level.
> +:- type trace_suppress_items.
> +
> +	% the bool should be the setting of the `require_tracing' option.
> +:- pred convert_trace_level(string::in, bool::in, trace_level::out) is semidet.
> +
> +:- pred convert_trace_suppress(string::in, trace_suppress_items::out)
> +	is semidet.
> +
> +	% These functions check for various properties of the trace level.
> +:- func trace_level_is_none(trace_level) = bool.
> +:- func trace_level_needs_fixed_slots(trace_level) = bool.
> +:- func trace_level_needs_from_full_slot(trace_level) = bool.
> +:- func trace_level_needs_decl_debug_slots(trace_level) = bool.
> +:- func trace_needs_return_info(trace_level, trace_suppress_items) = bool.
> +:- func trace_needs_all_var_names(trace_level, trace_suppress_items) = bool.
> +:- func trace_needs_proc_body_reps(trace_level, trace_suppress_items) = bool.
> +:- func trace_needs_port(trace_level, trace_suppress_items, trace_port) = bool.
> +
> +:- func trace_level_none = trace_level.
> +
> +:- implementation.
> +
> +:- import_module char, string, list, set.
> +
> +:- type trace_level
> +	--->	none
> +	;	shallow
> +	;	deep
> +	;	decl
> +	;	decl_rep.
> +
> +:- type trace_suppress_item
> +	--->	port(trace_port)
> +	;	return_info
> +	;	all_var_names
> +	;	proc_body_reps.
> +
> +:- type trace_suppress_items == set(trace_suppress_item).
> +
> +trace_level_none = none.
> +
> +convert_trace_level("minimum", no, none).
> +convert_trace_level("minimum", yes, shallow).
> +convert_trace_level("shallow", _, shallow).
> +convert_trace_level("deep", _, deep).
> +convert_trace_level("decl", _, decl).
> +convert_trace_level("rep", _, decl_rep).
> +convert_trace_level("default", no, none).
> +convert_trace_level("default", yes, deep).
> +
> +trace_level_is_none(none) = yes.
> +trace_level_is_none(shallow) = no.
> +trace_level_is_none(deep) = no.
> +trace_level_is_none(decl) = no.
> +trace_level_is_none(decl_rep) = no.
> +
> +trace_level_needs_fixed_slots(none) = no.
> +trace_level_needs_fixed_slots(shallow) = yes.
> +trace_level_needs_fixed_slots(deep) = yes.
> +trace_level_needs_fixed_slots(decl) = yes.
> +trace_level_needs_fixed_slots(decl_rep) = yes.
> +
> +trace_level_needs_from_full_slot(none) = no.
> +trace_level_needs_from_full_slot(shallow) = yes.
> +trace_level_needs_from_full_slot(deep) = no.
> +trace_level_needs_from_full_slot(decl) = no.
> +trace_level_needs_from_full_slot(decl_rep) = no.
> +
> +trace_level_needs_decl_debug_slots(none) = no.
> +trace_level_needs_decl_debug_slots(shallow) = no.
> +trace_level_needs_decl_debug_slots(deep) = no.
> +trace_level_needs_decl_debug_slots(decl) = yes.
> +trace_level_needs_decl_debug_slots(decl_rep) = yes.
> +
> +trace_needs_return_info(TraceLevel, TraceSuppressItems) = Need :-
> +	(
> +		trace_level_has_return_info(TraceLevel) = yes,
> +		\+ set__member(return_info, TraceSuppressItems)
> +	->
> +		Need = yes
> +	;
> +		Need = no
> +	).
> +
> +trace_needs_all_var_names(TraceLevel, TraceSuppressItems) = Need :-
> +	(
> +		trace_level_has_all_var_names(TraceLevel) = yes,
> +		\+ set__member(all_var_names, TraceSuppressItems)
> +	->
> +		Need = yes
> +	;
> +		Need = no
> +	).
> +
> +trace_needs_proc_body_reps(TraceLevel, TraceSuppressItems) = Need :-
> +	(
> +		trace_level_has_proc_body_reps(TraceLevel) = yes,
> +		\+ set__member(proc_body_reps, TraceSuppressItems)
> +	->
> +		Need = yes
> +	;
> +		Need = no
> +	).
> +
> +:- func trace_level_has_return_info(trace_level) = bool.
> +:- func trace_level_has_all_var_names(trace_level) = bool.
> +:- func trace_level_has_proc_body_reps(trace_level) = bool.
> +
> +trace_level_has_return_info(none) = no.
> +trace_level_has_return_info(shallow) = yes.
> +trace_level_has_return_info(deep) = yes.
> +trace_level_has_return_info(decl) = yes.
> +trace_level_has_return_info(decl_rep) = yes.
> +
> +trace_level_has_all_var_names(none) = no.
> +trace_level_has_all_var_names(shallow) = no.
> +trace_level_has_all_var_names(deep) = no.
> +trace_level_has_all_var_names(decl) = yes.
> +trace_level_has_all_var_names(decl_rep) = yes.
> +
> +trace_level_has_proc_body_reps(none) = no.
> +trace_level_has_proc_body_reps(shallow) = no.
> +trace_level_has_proc_body_reps(deep) = no.
> +trace_level_has_proc_body_reps(decl) = no.
> +trace_level_has_proc_body_reps(decl_rep) = yes.
> +
> +convert_trace_suppress(SuppressString, SuppressItemSet) :-
> +	SuppressWords = string__words(char_is_comma, SuppressString),
> +	list__map(convert_item_name, SuppressWords, SuppressItemLists),
> +	list__condense(SuppressItemLists, SuppressItems),
> +	set__list_to_set(SuppressItems, SuppressItemSet).
> +
> +:- pred char_is_comma(char::in) is semidet.
> +
> +char_is_comma(',').
> +
> +:- func convert_port_name(string) = trace_port is semidet.
> +
> +	% The call port cannot be disabled, because its layout structure is
> +	% referred to implicitly by the redo command in mdb.
> +	%
> +	% The exception port cannot be disabled, because it is never put into
> +	% compiler-generated code in the first place; such events are created
> +	% on the fly by library/exception.m.
> +% convert_port_name("call") = call.
> +convert_port_name("exit") = exit.
> +convert_port_name("fail") = fail.
> +convert_port_name("redo") = redo.
> +% convert_port_name("excp") = exception.
> +convert_port_name("exception") = exception.
> +convert_port_name("cond") = ite_cond.
> +convert_port_name("ite_cond") = ite_cond.
> +convert_port_name("then") = ite_then.
> +convert_port_name("ite_then") = ite_then.
> +convert_port_name("else") = ite_else.
> +convert_port_name("ite_else") = ite_else.
> +convert_port_name("nege") = neg_enter.
> +convert_port_name("neg_enter") = neg_enter.
> +convert_port_name("negs") = neg_success.
> +convert_port_name("neg_success") = neg_success.
> +convert_port_name("negf") = neg_failure.
> +convert_port_name("neg_failure") = neg_failure.
> +convert_port_name("swtc") = switch.
> +convert_port_name("switch") = switch.
> +convert_port_name("disj") = disj.
> +convert_port_name("frst") = nondet_pragma_first.
> +convert_port_name("nondet_pragma_first") = nondet_pragma_first.
> +convert_port_name("latr") = nondet_pragma_first.
> +convert_port_name("nondet_pragma_later") = nondet_pragma_later.
> +
> +:- func convert_port_class_name(string) = list(trace_port) is semidet.
> +
> +convert_port_class_name("interface") =
> +	[call, exit, redo, fail, exception].
> +convert_port_class_name("internal") =
> +	[ite_then, ite_else, switch, disj].
> +convert_port_class_name("context") =
> +	[ite_cond, neg_enter, neg_success, neg_failure].
> +
> +:- func convert_other_name(string) = trace_suppress_item is semidet.
> +
> +convert_other_name("return") = return_info.
> +convert_other_name("return_info") = return_info.
> +convert_other_name("names") = all_var_names.
> +convert_other_name("all_var_names") = all_var_names.
> +convert_other_name("bodies") = proc_body_reps.
> +convert_other_name("proc_body_reps") = proc_body_reps.
> +
> +:- pred convert_item_name(string::in, list(trace_suppress_item)::out)
> +	is semidet.
> +
> +convert_item_name(String, Names) :-
> +	( convert_port_name(String) = PortName ->
> +		Names = [port(PortName)]
> +	; convert_port_class_name(String) = PortNames ->
> +		list__map(wrap_port, PortNames, Names)
> +	; convert_other_name(String) = OtherName ->
> +		Names = [OtherName]
> +	;
> +		fail
> +	).
> +
> +:- pred wrap_port(trace_port::in, trace_suppress_item::out) is det.
> +
> +wrap_port(Port, port(Port)).
> +
> +%-----------------------------------------------------------------------------%
> +
> +:- type port_category
> +	--->	interface
> +	;	internal
> +	;	context.
> +
> +:- func trace_port_category(trace_port) = port_category.
> +
> +trace_port_category(call)			= interface.
> +trace_port_category(exit)			= interface.
> +trace_port_category(fail)			= interface.
> +trace_port_category(redo)			= interface.
> +trace_port_category(exception)			= interface.
> +trace_port_category(ite_cond)			= context.
> +trace_port_category(ite_then)			= internal.
> +trace_port_category(ite_else)			= internal.
> +trace_port_category(neg_enter)			= context.
> +trace_port_category(neg_success)		= context.
> +trace_port_category(neg_failure)		= context.
> +trace_port_category(switch)			= internal.
> +trace_port_category(disj)			= internal.
> +trace_port_category(nondet_pragma_first)	= internal.
> +trace_port_category(nondet_pragma_later)	= internal.
> +
> +:- func trace_level_port_categories(trace_level) = list(port_category).
> +
> +trace_level_port_categories(none) = [].
> +trace_level_port_categories(shallow) = [interface].
> +trace_level_port_categories(deep) = [interface, internal].
> +trace_level_port_categories(decl) = [interface, internal, context].
> +trace_level_port_categories(decl_rep) = [interface, internal, context].
> +
> +:- func trace_level_allows_port_suppression(trace_level) = bool.
> +
> +trace_level_allows_port_suppression(none) = no.		% no ports exist
> +trace_level_allows_port_suppression(shallow) = yes.
> +trace_level_allows_port_suppression(deep) = yes.
> +trace_level_allows_port_suppression(decl) = no.
> +trace_level_allows_port_suppression(decl_rep) = no.
> +
> +trace_needs_port(TraceLevel, TraceSuppressItems, Port) = NeedsPort :-
> +	(
> +		trace_port_category(Port) = Category,
> +		list__member(Category,
> +			trace_level_port_categories(TraceLevel)),
> +		( trace_level_allows_port_suppression(TraceLevel) = yes =>
> +			\+ set__member(port(Port), TraceSuppressItems) )

IMHO, the logic would be clearer if that last conjunct was

	\+ (
		trace_level_allows_port_suppression(Trace_Level) = yes,
		set__member(port(Port), TraceSuppressItems)
	)


> +	->
> +		NeedsPort = yes
> +	;
> +		NeedsPort = no
> +	).
> cvs diff: Diffing compiler/notes
> cvs diff: Diffing debian
> cvs diff: Diffing doc
> Index: doc/user_guide.texi
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/doc/user_guide.texi,v
> retrieving revision 1.220
> diff -u -r1.220 user_guide.texi
> --- doc/user_guide.texi	2000/09/17 09:19:12	1.220
> +++ doc/user_guide.texi	2000/09/29 02:42:49
> @@ -1440,62 +1440,63 @@
>  MCFLAGS-bar = --trace shallow
>  @end example
>  
> - at node Selecting trace events
> - at section Selecting trace events
> + at node Tracing and optimization trace events
> + at section Tracing and optimization trace events

I'd call this section "Tracing and compiler optimization", or maybe
"Tracing optimized code".

>  
> - at c XXX anybody got a better title for this section?
> -
> -In preparing a Mercury program for debugging,
> -the Mercury compiler provides two options that you can use
> -to say that you are not interested in certain kinds of events,
> -and that the compiler should therefore not generate code for those events.
> -This makes the executable smaller and faster.
> -
> -The first of these options is @samp{--no-trace-internal}.
> -If you specify this when you compile a module,
> -you will not get any internal events
> -from predicates and functions defined in that module,
> -even if the trace level is @samp{deep};
> -you will only get external events.
> -These are sufficient to tell you what a predicate or function does,
> -i.e. what outputs it computes from what inputs.
> -They do not tell you how it computed those outputs,
> -i.e. what path control took through the predicate or function,
> -but that is sometimes of no particular interest.
> -In any case, much of the time you can deduce the path
> -from the events that result from
> -calls made by the predicate or function in question.
> -
> -The second of these options is @samp{--no-trace-redo},
> -which can be specified independently of @samp{--no-trace-internal}.
> -If you specify this when you compile a module,
> -you will not get any redo events
> -from predicates and functions defined in that module.
> -If you are not interested in how backtracking arrives
> -at a program point where forward execution can resume after a failure,
> -this is an entirely reasonable thing to do.
> -In any case, with sufficient thought and a memory of previous events
> -you can reconstruct the sequence of redo events
> -that would normally be present between the fail event
> -and the event that represents the resumption of forward execution.
> -This sequence has a redo event
> -for every call to a procedure that can succeed more than once
> -that occurred after the call to the procedure
> -in which the resumption event occurs,
> -provided that that call has not failed yet,
> -and in reverse chronological order.
> -
> -Normally, when it compiles a module with a trace level other than @samp{none},
> -the compiler will include in the module's object file
> -information about all the call return sites in that module.
> -This information allows the debugger to print stack dumps,
> -as well as the values of variables in ancestors of current call.
> -However, if you specify the @samp{--no-trace-return} option,
> -the compiler will not include this information in the object file,
> -reducing its size but losing the above functionality.
> -
> - at c XXX should we document --stack-trace-higher-order
> - at c it has not really been tested yet


> + at c @c XXX anybody got a better title for this section?

There is an extra `@c' here.

> + at c 
> + at c In preparing a Mercury program for debugging,
> + at c the Mercury compiler provides two options that you can use
> + at c to say that you are not interested in certain kinds of events,
> + at c and that the compiler should therefore not generate code for those events.
> + at c This makes the executable smaller and faster.
> + at c 
> + at c The first of these options is @samp{--no-trace-internal}.
> + at c If you specify this when you compile a module,
> + at c you will not get any internal events
> + at c from predicates and functions defined in that module,
> + at c even if the trace level is @samp{deep};
> + at c you will only get external events.
> + at c These are sufficient to tell you what a predicate or function does,
> + at c i.e. what outputs it computes from what inputs.
> + at c They do not tell you how it computed those outputs,
> + at c i.e. what path control took through the predicate or function,
> + at c but that is sometimes of no particular interest.
> + at c In any case, much of the time you can deduce the path
> + at c from the events that result from
> + at c calls made by the predicate or function in question.
> + at c 
> + at c The second of these options is @samp{--no-trace-redo},
> + at c which can be specified independently of @samp{--no-trace-internal}.
> + at c If you specify this when you compile a module,
> + at c you will not get any redo events
> + at c from predicates and functions defined in that module.
> + at c If you are not interested in how backtracking arrives
> + at c at a program point where forward execution can resume after a failure,
> + at c this is an entirely reasonable thing to do.
> + at c In any case, with sufficient thought and a memory of previous events
> + at c you can reconstruct the sequence of redo events
> + at c that would normally be present between the fail event
> + at c and the event that represents the resumption of forward execution.
> + at c This sequence has a redo event
> + at c for every call to a procedure that can succeed more than once
> + at c that occurred after the call to the procedure
> + at c in which the resumption event occurs,
> + at c provided that that call has not failed yet,
> + at c and in reverse chronological order.
> + at c 
> + at c Normally, when it compiles a module
> + at c with a trace level other than @samp{none},
> + at c the compiler will include in the module's object file
> + at c information about all the call return sites in that module.
> + at c This information allows the debugger to print stack dumps,
> + at c as well as the values of variables in ancestors of current call.
> + at c However, if you specify the @samp{--no-trace-return} option,
> + at c the compiler will not include this information in the object file,
> + at c reducing its size but losing the above functionality.
> + at c 

Even if it is commented out, it would be nicer to update this so that it
refers to the new option.

> + at c @c XXX should we document --stack-trace-higher-order
> + at c @c it has not really been tested yet

Extra `@c's here too.

>  
>  By default, all trace levels other than @samp{none}
>  turn off all compiler optimizations


Cheers,
Mark.

--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------