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

[Fergus Henderson]

I agree this should be reviewed by someone other than me, e.g. Peter Ross.
But I'll add my own review comments for this one too.

On 04-Sep-2000, Tyson Dowd <trd at cs.mu.OZ.AU> wrote:
> compiler/ml_call_gen.m:
> compiler/ml_code_gen.m:
> compiler/ml_code_util.m:
> compiler/ml_unify_gen.m:
>         Use the `--det-copy-out' option to decide whether to pass output
>         arguments by reference, or whether to just return multiple values.
>         Use the `--nondet-copy-out' option to decide whether to pass output
>         arguments by reference, or whether to just pass them to the
>         continuation.

I think my original log message could be improved a little:

	 Use the `--det-copy-out' option to decide whether to pass
	 output arguments for model_det/model_semi procedures by
	 reference, or whether to just return multiple values.
	 Use the `--nondet-copy-out' option to decide whether to pass
	 output arguments for model_non procedures by reference, or
	 whether to just pass them to the continuation.

> +++ compiler/globals.m	2000/09/04 03:17:06
> @@ -21,6 +21,13 @@
>  
>  :- type globals.
>  
> +:- type compilation_target
> +	--->	c	% Generate C code
> +	;	il	% Generate IL assembler code
> +			% IL is the Microsoft COM+ 2.0 Intermediate Language

s/COM+ 2.0/.NET/

> --- compiler/mercury_compile.m	2000/08/31 03:00:18	1.172
> +++ compiler/mercury_compile.m	2000/09/04 05:00:59
> @@ -477,6 +478,21 @@
>  	    []
>  	).
>  
> +	% return `yes' iff this module defines the main/2 entry point.
> +:- func mercury_compile__mlds_has_main(mlds) = bool.
> +mercury_compile__mlds_has_main(MLDS) =
> +	(
> +		MLDS = mlds(_, _, _, Defns),
> +		list__member(Defn, Defns),
> +		Defn = mlds__defn(Name, _, _, _),
> +		Name = function(FuncName, _, _, _), 
> +		FuncName = pred(predicate, _, "main", 2)
> +	->
> +		yes
> +	;
> +		no
> +	).
> +

That bit should probably be committed separately, with the IL stuff that
uses it, since as far as I can tell, in this diff that function is not used.

> +++ compiler/ml_call_gen.m	2000/09/04 00:41:56
> @@ -83,6 +83,12 @@
>  :- mode ml_gen_box_or_unbox_lval(in, in, in, in, in, out, out, out, out,
>  		in, out) is det.
>  
> +	% XXX document this
> +:- pred ml_gen_cont_params(list(mlds__type), mlds__func_params,
> +		ml_gen_info, ml_gen_info).
> +:- mode ml_gen_cont_params(in, out, in, out) is det.

The comment there needs fixing; I suggest the following:

	% Generate the appropriate MLDS type for a continuation function
	% for a nondet procedure whose output arguments have the
	% specified types.

> Index: compiler/ml_code_gen.m
...
> @@ -1717,8 +1871,19 @@
>  			raw_target_code("\t\tif (MR_succeeded) {\n")],
>  			AssignOutputsList
>  	]) },
> +	=(MLDSGenInfo),
> +	{ ml_gen_info_get_module_info(MLDSGenInfo, ModuleInfo) },
> +	{ module_info_globals(ModuleInfo, Globals) },
> +	{ globals__lookup_string_option(Globals, target, Target) },
>  	( { CodeModel = model_non } ->
> -		ml_gen_call_current_success_cont(Context, CallCont)
> +		(
> +			{ Target = "il" }
> +		->
> +			ml_gen_call_current_success_cont_indirectly(Context,
> +				CallCont)
> +		;
> +			ml_gen_call_current_success_cont(Context, CallCont)
> +		)

I don't understand that if-then-else.

I also don't recall writing that one.  Ah, looking at the log messages
in the p7 repository I see that that code was added later, by Tyson.

A comment here would help a lot.
What's the difference between calling the continuation directly
and indirectly?  And Why does it have to be done different for IL?

> Index: compiler/ml_code_util.m
> @@ -326,9 +337,11 @@
>  	% of local variables in the containing procedure) for the continuation
>  	% function.  (If we're using gcc nested function, the `cont_env'
>  	% is not used.)
> +	% XXX need to update this documentation
>  	%
> -:- pred ml_initial_cont(success_cont, ml_gen_info, ml_gen_info).
> -:- mode ml_initial_cont(out, in, out) is det.
> +:- pred ml_initial_cont(list(mlds__lval), list(prog_type), success_cont,
> +			ml_gen_info, ml_gen_info).
> +:- mode ml_initial_cont(in, in, out, in, out) is det.

That XXX should be fixed.
The comment should explain the role of the two new parameters.

> +	% Generate code to call the current success continuation, using
> +	% a local function as a proxy.
> +	% This is used for generating success when in a model_non context
> +	% from within pragma C code (currently only in IL).
> +	%
> +:- pred ml_gen_call_current_success_cont_indirectly(prog_context, 
> +		mlds__statement, ml_gen_info, ml_gen_info).
> +:- mode ml_gen_call_current_success_cont_indirectly(in, out, in, out) is det.

That documentation doesn't explain why we need to use a local function
as a proxy when in calling something in a model_non context from
within pragma C code in IL.

Ah, it's all coming back to me now...
MS Managed C++ won't let you call via a function pointer, and
with `--target il' we generate managed C++ for `pragma c_code' fragments,
which we put in a different file, so if we want to make a call to
a continuation, we need to put the call in a separate function
so that it gets compiled as C rather than managed C++.
But this should all be documented.

> @@ -1039,6 +1120,7 @@
>  		% The variable won't have been declared, so
>  		% we need to generate a dummy lval for this variable.
>  		%
> +		{ dummy_call },
>  		{ mercury_private_builtin_module(PrivateBuiltin) },
>  		{ MLDS_Module = mercury_module_name_to_mlds(PrivateBuiltin) },
>  		{ Lval = var(qual(MLDS_Module, "dummy_var")) }
> @@ -1059,6 +1141,9 @@
>  		)
>  	).
>  
> +:- pred dummy_call is det.
> +dummy_call.

What's the dummy_call for here?

I think that was just debugging code, which should be deleted.

> @@ -1305,6 +1414,87 @@
>  	{ MLDS_Statement = mlds__statement(MLDS_Stmt,
>  			mlds__make_context(Context)) }.
>  
> +	% 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) -->

It would be a good idea to do what the XXX comment says...

-- 
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
--------------------------------------------------------------------------