[m-rev.] for review: use bytecode for procedure body representations

Ian MacLarty maclarty at cs.mu.OZ.AU
Wed Mar 30 20:40:03 AEST 2005


On Wed, Mar 30, 2005 at 11:39:07AM +1000, Zoltan Somogyi wrote:
> Index: compiler/layout_out.m
> ===================================================================
> @@ -831,28 +842,72 @@
>  	output_layout_decl(CallLabelLayout, !DeclSet, !IO),
>  	output_layout_decl(module_layout(ModuleName), !DeclSet, !IO),
>  	(
> -		MaybeProcBody = yes(ProcBody),
> -		output_rval_decls(ProcBody, !DeclSet, !IO)
> -	;
> -		MaybeProcBody = no
> -	),
> -	(
>  		MaybeTableInfo = yes(TableInfo),
>  		output_layout_decl(TableInfo, !DeclSet, !IO)
>  	;
>  		MaybeTableInfo = no
>  	).
>  
> +	% The job of this predicate is to minimize stack space consumption in
> +	% grades that do not allow output_bytecode to be tail recursive.

s/output_bytecode/output_bytecodes

Out of curiosity, which grades don't allow output_bytecodes to be tail
recursive?

> Index: compiler/prog_rep.m
> ===================================================================
>  
> -prog_rep__represent_goal_expr(unify(_, _, _, Uni, _), GoalInfo, InstMap0,
> -		Info, Rep) :-
> +goal_expr_to_byte_list(conj(Goals), _, InstMap0, Info) = Bytes :-
> +	Bytes = [goal_type_to_byte(goal_conj)] ++
> +		length_to_byte_list(Goals) ++
> +		conj_to_byte_list(Goals, InstMap0, Info).
> +goal_expr_to_byte_list(par_conj(_), _, _, _) = _ :-
> +	error("Sorry, not yet implemented:\n\
> +		parallel conjunctions and declarative debugging").

Shouldn't you call sorry/2 instead?

...
>  	;
>  		Uni = simple_test(Var1, Var2),
> -		term__var_to_int(Var1, Var1Rep),
> -		term__var_to_int(Var2, Var2Rep),
> -		AtomicGoalRep = unify_simple_test_rep(Var1Rep, Var2Rep)
> +		Bytes = [goal_type_to_byte(goal_simple_test)] ++
> +			var_to_byte_list(Info, Var1) ++
> +			var_to_byte_list(Info, Var2) ++
> +			AtomicBytes
>  	;
>  		Uni = complicated_unify(_, _, _),
> -		error("prog_rep__represent_goal_expr: complicated_unify")
> -	),
> -	prog_rep__represent_atomic_goal(GoalInfo, InstMap0, Info,
> -		DetismRep, FilenameRep, LinenoRep, ChangedVarsRep),
> -	Rep = atomic_goal_rep(DetismRep, FilenameRep, LinenoRep, ChangedVarsRep,
> -		AtomicGoalRep).
> -prog_rep__represent_goal_expr(conj(Goals), _, InstMap0, Info, Rep) :-
> -	prog_rep__represent_conj(Goals, InstMap0, Info, Reps),
> -	Rep = conj_rep(Reps).
> -prog_rep__represent_goal_expr(par_conj(_), _, _, _, _) :-
> -	error("Sorry, not yet implemented:\n\
> -	parallel conjunctions and declarative debugging").
> -prog_rep__represent_goal_expr(disj(Goals), _, InstMap0, Info, Rep) :-
> -	prog_rep__represent_disj(Goals, InstMap0, Info, DisjReps),
> -	Rep = disj_rep(DisjReps).
> -prog_rep__represent_goal_expr(not(Goal), _GoalInfo, InstMap0, Info, Rep)
> -		:-
> -	prog_rep__represent_goal(Goal, InstMap0, Info, InnerRep),
> -	Rep = negation_rep(InnerRep).
> -prog_rep__represent_goal_expr(if_then_else(_, Cond, Then, Else),
> -		_, InstMap0, Info, Rep) :-
> -	prog_rep__represent_goal(Cond, InstMap0, Info, CondRep),
> -	Cond = _ - CondGoalInfo,
> -	goal_info_get_instmap_delta(CondGoalInfo, InstMapDelta),
> -	instmap__apply_instmap_delta(InstMap0, InstMapDelta, InstMap1),
> -	prog_rep__represent_goal(Then, InstMap1, Info, ThenRep),
> -	prog_rep__represent_goal(Else, InstMap0, Info, ElseRep),
> -	Rep = ite_rep(CondRep, ThenRep, ElseRep).
> -prog_rep__represent_goal_expr(switch(_, _, Cases), _,
> -		InstMap0, Info, Rep) :-
> -	prog_rep__represent_cases(Cases, InstMap0, Info, CaseReps),
> -	Rep = switch_rep(CaseReps).
> -prog_rep__represent_goal_expr(scope(_, Goal), GoalInfo, InstMap0, Info, Rep) :-
> -	prog_rep__represent_goal(Goal, InstMap0, Info, InnerRep),
> +		error("goal_expr_to_byte_list: complicated_unify")

Should complicated unification have been converted to procedure calls by this
point?

> +:- func cons_id_to_string(cons_id) = string.
> +
> +cons_id_to_string(cons(SymName, _)) =
> +	prog_rep__sym_name_to_string(SymName).
> +cons_id_to_string(int_const(Int)) =
> +	string__int_to_string(Int).
> +cons_id_to_string(float_const(Float)) =
> +	string__float_to_string(Float).
> +cons_id_to_string(string_const(String)) =
> +	string__append_list(["""", String, """"]).
> +cons_id_to_string(pred_const(_, _)) = "$pred_const".
> +cons_id_to_string(type_ctor_info_const(_, _, _)) =
> +	"$type_ctor_info_const".
> +cons_id_to_string(base_typeclass_info_const(_, _, _, _)) =
> +	"$base_typeclass_info_const".
> +cons_id_to_string(type_info_cell_constructor(_)) =
> +	"$type_info_cell_constructor".
> +cons_id_to_string(typeclass_info_cell_constructor) =
> +	"$typeclass_info_cell_constructor".
> +cons_id_to_string(tabling_pointer_const(_)) =
> +	"$tabling_pointer_const".
> +cons_id_to_string(deep_profiling_proc_layout(_)) =
> +	"$deep_profiling_procedure_data".
> +cons_id_to_string(table_io_decl(_)) =
> +	"$table_io_decl".
> +
> +:- func sym_name_to_byte_list(sym_name) = list(int).
> +
> +sym_name_to_byte_list(SymName) =
> +	string_to_byte_list(prog_rep__sym_name_to_string(SymName)).
> +

The above function doesn't appear to be used anywhere.

> +:- func sym_name_to_string(sym_name) = string.
> +
> +sym_name_to_string(unqualified(String)) = String.
> +sym_name_to_string(qualified(_, String)) = String.
> +

Are you sure it's okay to throw away any module qualifiers here?

I suggest you give sym_name_to_string a different name to more
easily distinguish is from the predicate with the same name in
mdbcomp.prim_data.  Or alternatively, since this version of 
sym_name_to_string is only used in cons_id_to_string, just inline
this code in cons_id_to_string, with a comment justifying why it's
okay to throw away the module qualifiers in that case.

> +%---------------------------------------------------------------------------%
> +
> +:- func conj_to_byte_list(hlds_goals, instmap, prog_rep__info) = list(int).
> +
> +conj_to_byte_list([], _, _) = [].
> +conj_to_byte_list([Goal | Goals], InstMap0, Info) = Bytes :-
> +	GoalBytes = goal_to_byte_list(Goal, InstMap0, Info),
>  	Goal = _ - GoalInfo,
>  	goal_info_get_instmap_delta(GoalInfo, InstMapDelta),
>  	instmap__apply_instmap_delta(InstMap0, InstMapDelta, InstMap1),
> -	prog_rep__represent_conj(Goals, InstMap1, Info, Reps).
> +	GoalsBytes = conj_to_byte_list(Goals, InstMap1, Info),
> +	Bytes = GoalBytes ++ GoalsBytes.
> +
> +%---------------------------------------------------------------------------%
> +
> +:- func disj_to_byte_list(hlds_goals, instmap, prog_rep__info) = list(int).
> +
> +disj_to_byte_list([], _, _) = [].
> +disj_to_byte_list([Goal | Goals], InstMap0, Info) = Bytes :-
> +	GoalBytes = goal_to_byte_list(Goal, InstMap0, Info),
> +	GoalsBytes = disj_to_byte_list(Goals, InstMap0, Info),
> +	Bytes = GoalBytes ++ GoalsBytes.
>  
>  %---------------------------------------------------------------------------%
>  
> -:- pred prog_rep__represent_disj(hlds_goals::in, instmap::in,
> -	prog_rep__info::in, list(goal_rep)::out) is det.
> +:- func cases_to_byte_list(list(case), instmap, prog_rep__info) = list(int).

I think "switch_arms_to_byte_list" would be a better name.

> Index: mdbcomp/program_representation.m
> ===================================================================
> @@ -364,4 +402,46 @@
>  string_from_path_step(later, "l").
>  
>  %-----------------------------------------------------------------------------%
> +
> +detism_rep(Detism) = Rep :-
> +	determinism_representation(Detism, Rep).
> +
> +determinism_representation(det_rep, 6).
> +determinism_representation(semidet_rep, 2).
> +determinism_representation(nondet_rep, 3).
> +determinism_representation(multidet_rep, 7).
> +determinism_representation(erroneous_rep, 4).

In the reference manual it says that erroneous procedures never produce any
solutions, so why is the 4 bit set here?   I think you should include a comment
about erroneous being a special case.

> +determinism_representation(failure_rep, 0).
> +determinism_representation(cc_nondet_rep, 10).
> +determinism_representation(cc_multidet_rep, 14).
> +
> +%-----------------------------------------------------------------------------%
> +
> +goal_type_to_byte(Type) = TypeInt :-
> +	goal_type_byte(TypeInt, Type).
> +
> +byte_to_goal_type(TypeInt) = Type :-
> +	goal_type_byte(TypeInt, Type).
> +
> +:- pred goal_type_byte(int, bytecode_goal_type).
> +:- mode goal_type_byte(in, out) is semidet.
> +:- mode goal_type_byte(out, in) is det.
> +
> +goal_type_byte(1, goal_conj).
> +goal_type_byte(2, goal_disj).
> +goal_type_byte(3, goal_switch).
> +goal_type_byte(4, goal_ite).
> +goal_type_byte(5, goal_neg).
> +goal_type_byte(6, goal_scope).
> +goal_type_byte(7, goal_construct).
> +goal_type_byte(8, goal_deconstruct).
> +goal_type_byte(9, goal_assign).
> +goal_type_byte(10, goal_unsafe_cast).
> +goal_type_byte(11, goal_simple_test).
> +goal_type_byte(12, goal_foreign).
> +goal_type_byte(13, goal_ho_call).
> +goal_type_byte(14, goal_method_call).
> +goal_type_byte(15, goal_plain_call).
> +goal_type_byte(16, goal_builtin_call).
> +
>  %-----------------------------------------------------------------------------%
> cvs diff: Diffing profiler
> cvs diff: Diffing robdd
> cvs diff: Diffing runtime
> Index: runtime/mercury_stack_layout.h
> ===================================================================
> @@ -697,12 +697,10 @@
>  ** containing the procedure. This allows the debugger access to the string table
>  ** stored there, as well the table associating source-file contexts with labels.
>  **
> -** The proc_rep field contains a representation of the body of the procedure
> -** as a Mercury term of type proc_rep, defined in program_representation.m.
> -** Note that the type of this field is `MR_Word *', not `MR_Word',
> -** for the same reasons that MR_mkword() has type `MR_Word *' rather
> -** than `MR_Word' (see the comment in runtime/mercury_tags.h).
> -** It will be a null pointer if no such representation is available.
> +** The body_bytes field contains a pointer to a list of bytecodes that

s/list/array ?

> +** represents the body of the procedure. It will be a null pointer if no
> +** representation is available. If it is not null pointer, then it should
> +** be interpreted by read_proc_rep in browser/declarative_execution.m.
>  #define	MR_sle_head_var_nums	MR_sle_exec_trace->MR_exec_head_var_nums
> Index: runtime/mercury_trace_base.c
> ===================================================================
>  #endif  /* MR_TRACE_HISTOGRAM */
> +
> +/*
> +** We record information about procedure representations in a hash table
> +** that is indexed by the proc layout address.
> +**
> +** This table is used by the declarative debugger. Since the declarative
> +** debugger can be required any grade, we always include this table, but

s/required any/required in any

> Index: trace/mercury_trace_internal.c
> ===================================================================
>      entry = event_info->MR_event_sll->MR_sll_entry;
> -    if (entry->MR_sle_proc_rep == NULL) {
> -        return "current procedure has no body info";
> +
> +    if (entry->MR_sle_body_bytes == NULL) {
> +        return "current procedure has no body bytecodes";
>      }

Since this return string would be displayed to the user I think the
previous version is better.  The user doesn't care that the procedure
body is represented as bytecodes.

Otherwise that looks good.

Ian.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list