[m-dev.] for review: --target option, IL grades and copy-in/copy-out for MLDS

[Fergus Henderson]

Hi,

Some more review comments on this change.

On 04-Sep-2000, Tyson Dowd <trd at cs.mu.OZ.AU> wrote:
> diff -u -r1.19 ml_code_util.m
> +	% XXX this code is quite similar to some of the existing code
> +	% for calling continuations when doing copy-in/copy-out.
> +	% Sharing code should be investigated.
> +
> +ml_gen_call_current_success_cont_indirectly(Context, MLDS_Statement) -->
...
> +		% We generate a nested function that does the real call
> +		% to the continuation.
> +		%
> +		% All we do is change the call rvals to be the input
> +		% variables, and the func rval to be the input variable
> +		% for the continuation.
> +	ml_gen_cont_params(ArgTypes0, InnerFuncParams0),
> +	{ InnerFuncParams0 = func_params(InnerArgs0, Rets) },
> +	{ InnerArgRvals = list__filter_map(
> +		(func(data(var(VarName)) - _Type) 
> +		= lval(var(qual(MLDS_Module, VarName))) is semidet), 
> +			InnerArgs0) },

Here I think it would be better to make that a call to list__map
rather than list__filter_map, and to call error/1 for any arguments
that don't match the pattern `data(var(_)) - _' rather than just ignoring
them.

> +		% XXX is the ArgTypes0 right?
> +	{ InnerFuncArgType = mlds__cont_type(ArgTypes0) },

I think the answer to that question is yes, in which case
you can delete that XXX.
(Why wouldn't it be right?)

> +	{ Defn = mlds__defn(function(PredLabel, ProcId, yes(SeqNum), _), _, _, 
> +		function(_, _, yes(_)))
> +	->

The condition of that if-then-else should be indented.

> @@ -1333,9 +1523,10 @@
>  % The `ml_gen_info' type holds information used during MLDS code generation
>  % for a given procedure.
>  %
> -% Only the `func_label', `commit_label', `cond_var', `success_cont_stack',
> -% and `extra_defns' fields are mutable; the others are set when the 
> -% `ml_gen_info' is created and then never modified.
> +% Only the `func_label', `commit_label', `cond_var', `conv_var',
> +% `var_lvals', `success_cont_stack', and `extra_defns' fields are mutable;
> +% the others are set when the % ml_gen_info' is created and then never
> +% modified.

s/% ml_gen_info'/`ml_gen_info'/
  ^^             ^

> @@ -2149,11 +2173,6 @@
>  		%	...
>  		%	new_argN_value = argN_tmp_copy;
>  		%
> -		% The temporaries are needed for the case where
> -		% we are e.g. assigning v1, v2 to v2, v1;
> -		% they ensure that we don't try to reference the old value of
> -		% a parameter after it has already been clobbered by the
> -		% new value.

Why did that comment get deleted?

> +:- pred options_help_mlds_mlds_optimization(io__state::di, io__state::uo) is det.
> +
> +options_help_mlds_mlds_optimization -->
> +	io__write_string("\n    MLDS -> MLDS optimizations:\n"),
> +	write_tabbed_lines([
> +		"--no-llds-optimize",
> +		"\tDisable the MLDS->MLDS optimization passes."
> +	]).

s/llds-optimize/mlds-optimize/
  ^             ^

> + at item @code{--target c} (grades: none, reg, jump, fast, asm_jump, asm_fast, hlc)
> + at itemx @code{--il}, @code{--target il} (grades: ilc)
> +Specify the target language used for compilation: C or IL.
> +C means ANSI/ISO C, optionally with GNU C extensions (see below).
> +IL means the Microsoft COM+ 2.0 Intermediate Language.
> + at samp{--target il} implies @samp{--high-level-code}.

s/COM+ 2.0/.NET/

The --no-mlds-optimize option should be documented in user_guide.texi.

Apart from that, this looks fine.

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