[m-dev.] for review: generating C layout structures

[Fergus Henderson]

On 08-Jan-2001, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> +output_layout_addr_storage_type_name(label_layout(Label, LabelVars),
> +		_BeingDefined) -->
> +	io__write_string("static const "),
> +	io__write_string(label_vars_to_type(LabelVars)),
> +	io__write_string(" "),
> +	output_layout_addr(label_layout(Label, LabelVars)).
> +output_layout_addr_storage_type_name(proc_layout(ProcLabel, Kind),
> +		_BeingDefined) -->
> +	io__write_string("static MR_STATIC_CODE_CONST "),
> +	io__write_string(kind_to_type(Kind)),
> +	io__write_string(" "),
> +	output_layout_addr(proc_layout(ProcLabel, Kind)).
> +output_layout_addr_storage_type_name(proc_layout_var_names(ProcLabel),
> +		_BeingDefined) -->
> +	io__write_string("static const "),
> +	io__write_string("MR_int_least16_t "),
> +	output_layout_addr(proc_layout_var_names(ProcLabel)),
> +	io__write_string("[]").
> +output_layout_addr_storage_type_name(closure_proc_id(CallerProcLabel, SeqNo,
> +		ClosureProcLabel), _BeingDefined) -->
> +	io__write_string("static const "),
> +	(
> +		{ ClosureProcLabel = proc(_, _, _, _, _, _) },
> +		io__write_string("MR_User_Closure_Id\n")
> +	;
> +		{ ClosureProcLabel = special_proc(_, _, _, _, _, _) },
> +		io__write_string("MR_Compiler_Closure_Id\n")
> +	),
> +	output_layout_addr(closure_proc_id(CallerProcLabel, SeqNo,
> +		ClosureProcLabel)).
> +output_layout_addr_storage_type_name(file_layout(ModuleName, FileNum),
> +		_BeingDefined) -->
> +	io__write_string("static const MR_Module_File_Layout "),
> +	output_layout_addr(file_layout(ModuleName, FileNum)).
> +output_layout_addr_storage_type_name(file_layout_line_number_vector(
> +		ModuleName, FileNum), _BeingDefined) -->
> +	io__write_string("static const MR_int_least16_t "),
> +	output_layout_addr(
> +		file_layout_line_number_vector(ModuleName, FileNum)),
> +	io__write_string("[]").
> +output_layout_addr_storage_type_name(file_layout_label_layout_vector(
> +		ModuleName, FileNum), _BeingDefined) -->
> +	io__write_string("static const MR_Label_Layout *"),
> +	output_layout_addr(
> +		file_layout_label_layout_vector(ModuleName, FileNum)),
> +	io__write_string("[]").
> +output_layout_addr_storage_type_name(module_layout_file_vector(ModuleName),
> +		_BeingDefined) -->
> +	io__write_string("static const MR_Module_File_Layout *"),
> +	output_layout_addr(module_layout_file_vector(ModuleName)),
> +	io__write_string("[]").
> +output_layout_addr_storage_type_name(module_layout_proc_vector(ModuleName),
> +		_BeingDefined) -->
> +	io__write_string("static const MR_Proc_Layout *"),
> +	output_layout_addr(module_layout_proc_vector(ModuleName)),
> +	io__write_string("[]").
> +output_layout_addr_storage_type_name(module_layout(ModuleName),
> +		_BeingDefined) -->
> +	io__write_string("static const MR_Module_Layout "),
> +	output_layout_addr(module_layout(ModuleName)).

There seems to be a lot of code duplication there.
I think it would be better to factor out most of it
into separate subroutines:

	output_layout_addr_storage_type_name(Layout, _BeingDefined) -->
		( layout_name_is_exported(LayoutName) = yes ->
			io__write_string("static ")
		;
			io__write_string("extern ")
		),
		( layout_name_would_include_code_addr(LayoutName) = yes ->
			io__write_string("MR_STATIC_CODE_CONST ")
		;
			io__write_string("const ")
		),
		io__write_string(layout_type(Layout)),
		output_layout_name(Layout),
		( layout_name_has_array_type(Layout) = yes ->
			io__write_string("[]"
		;
			[]
		).

> +layout_name_would_include_code_addr(label_layout(_, _), no).
> +layout_name_would_include_code_addr(proc_layout(_, _), yes).
> +layout_name_would_include_code_addr(proc_layout_var_names(_), no).
> +layout_name_would_include_code_addr(closure_proc_id(_, _, _), no).
> +layout_name_would_include_code_addr(file_layout(_, _), no).
> +layout_name_would_include_code_addr(file_layout_line_number_vector(_, _), no).
> +layout_name_would_include_code_addr(file_layout_label_layout_vector(_, _), no).
> +layout_name_would_include_code_addr(module_layout_file_vector(_), no).
> +layout_name_would_include_code_addr(module_layout_proc_vector(_), no).
> +layout_name_would_include_code_addr(module_layout(_), no).

It would be nicer to make that a function.

> +:- pred maybe_proc_layout_and_exec_trace_kind(maybe_proc_id_and_exec_trace::in,
> +	proc_label::in, proc_layout_kind::out) is det.
> +
> +maybe_proc_layout_and_exec_trace_kind(MaybeRest, ProcLabel, Kind) :-
> +	(
> +		MaybeRest = no_proc_id,
> +		Kind = proc_layout_traversal
> +	;
> +		MaybeRest = proc_id_only,
> +		proc_label_user_or_compiler(ProcLabel, UserOrCompiler),
> +		Kind = proc_layout_proc_id(UserOrCompiler)
> +	;
> +		MaybeRest = proc_id_and_exec_trace(_),
> +		proc_label_user_or_compiler(ProcLabel, UserOrCompiler),
> +		Kind = proc_layout_exec_trace(UserOrCompiler)
> +	).
> +
> +:- pred proc_label_user_or_compiler(proc_label::in,
> +	proc_layout_user_or_compiler::out) is det.
> +
> +proc_label_user_or_compiler(proc(_, _, _, _, _, _), user).
> +proc_label_user_or_compiler(special_proc(_, _, _, _, _, _), compiler).

Those two would be nicer as functions.

> +output_layout_exec_trace_group(ProcLabel, ExecTrace) -->
> +	{ ExecTrace = proc_layout_exec_trace(CallLabelLayout, MaybeProcBody,
> +		_VarNames, MaxVarNum, MaxRegNum, MaybeFromFullSlot,
> +		MaybeIoSeqSlot, MaybeTrailSlot, MaybeMaxfrSlot, EvalMethod,
> +		MaybeCallTableSlot, MaybeDeclDebugSlot) },
> +	io__write_string("\t{\n\t(const MR_Label_Layout *) &"),
> +	output_layout_addr(CallLabelLayout),

I think `output_layout_addr' seems to be a misnomer,
since it is outputting the name, not the address, right?
If so, I suggest you rename it as output_layout_name.
You might also want to consider defining a separate procedure
output_layout_addr which outputs the address, cast to the right type.

> +:- pred output_proc_id(proc_label::in, io__state::di, io__state::uo) is det.
> +
> +output_proc_id(ProcLabel) -->
> +	(
> +		{ ProcLabel = proc(DefiningModule, PredOrFunc, DeclaringModule,
> +			Name, Arity, Mode) },
> +		{ prog_out__sym_name_to_string(DefiningModule,
> +			DefiningModuleStr) },
> +		{ prog_out__sym_name_to_string(DeclaringModule,
> +			DeclaringModuleStr) },
> +		{ proc_id_to_int(Mode, ModeInt) },
> +		(
> +			{ PredOrFunc = predicate },
> +			io__write_string("\tMR_PREDICATE,\n\t")
> +		;
> +			{ PredOrFunc = function },
> +			io__write_string("\tMR_FUNCTION,\n\t")
> +		),

I think that disjunction is worth extracting out as a separate
function.

> +++ compiler/stack_layout.m	2001/01/07 09:50:07

Some general comments explaining what this module does would be
helpful.

> +% File: stacK_layout.m.
>  % Main authors: trd, zs.
> +% This module generates label, procedure, module and closure layout structures
> +% for code in the current module for the LLDS backend.

A brief comment explaining what those layout structures are for
would be helpful here.

runtime/mercury_stack_layout.h:
> +** If the procedure has an I/O state argument, the maybe_io_seq field will
> +** contain the number of the stack slot that holds the value the I/O action
> +** counter had on entry to this procedure.

That's only true if the appropriate option is enabled.

> +** We cannot put enums into structures as bit fields. To avoid wasting space,
> +** we put MR_EvalMethodInts into structures instead of MR_EvalMethods
> +** themselves.

I suggest adding ", since standard C doesn't support that"
at the end of the first sentence.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
                                    |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------