[m-rev.] for review: il as a foreign language

[Fergus Henderson]

On 11-Jul-2001, Tyson Dowd <trd at cs.mu.OZ.AU> wrote:
> Add support for foreign_proc("il", ....)

This should be documented in the language reference manual.

If you don't want to make the documentation official yet,
then leave it commented out.

> compiler/mlds.m:
> 	Add a field in blocks, which is set to yes iff the block contains
> 	handwritten foreign code.

Why?

> compiler/ml_code_util.m:
> compiler/ml_elim_nested.m:
> compiler/ml_optimize.m:
> compiler/ml_simplify_switch.m:
> compiler/ml_string_switch.m:
> compiler/ml_tailcall.m:
> compiler/ml_util.m:
> 	Handle the new field in blocks indicating whether the block contains
> 	handwritten code.

I'm not sure these changes are a good idea, because I'm not convinced that
don't think the field is necessary.  Could you elaborate on why you think
it is needed?

> Index: compiler/mlds.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/mlds.m,v
> retrieving revision 1.58
> diff -u -r1.58 mlds.m
> --- compiler/mlds.m	2001/07/10 12:51:03	1.58
> +++ compiler/mlds.m	2001/07/10 16:05:53
> @@ -726,7 +726,13 @@
>  	%
>  	% sequence
>  	%
> -		block(mlds__defns, list(mlds__statement))
> +		block(mlds__defns, list(mlds__statement), bool)	
> +			% the `bool' is yes iff the block contains
> +			% handwritten foreign language code.  
> +			% If so, this block can be considered a boundary for 
> +			% optimization purposes -- the code within the block
> +			% should not be optimized as the optimizer will not
> +			% (in general) understand the foreign code.

When you say "contains", do you mean contains directly, or is the bool supposed
to be yes even if it is only contained indirectly, i.e. if you have a block
containing another block containing the foreign code?

If the latter, then I think there is lots of code which doesn't respect that
invariant (and in general this invariant would be hard to preserve).

If the former, then what's the advantage?
Why does putting the marker on the block help?
Can't code which does optimization check for foreign_code
just as easily as it can check this marker?

In your diff, which code checks the marker?

> Index: compiler/ml_code_gen.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/ml_code_gen.m,v
> retrieving revision 1.89
> diff -u -r1.89 ml_code_gen.m
> --- compiler/ml_code_gen.m	2001/06/22 09:14:30	1.89
> +++ compiler/ml_code_gen.m	2001/07/10 16:05:51
> @@ -1051,11 +1051,11 @@
>  		% them from the byref_output_vars field in the ml_gen_info.
>  		( CodeModel = model_non ->
>  		
> - 			ml_set_up_initial_succ_cont(ModuleInfo, 
> +			ml_set_up_initial_succ_cont(ModuleInfo, 

> @@ -1989,20 +1989,20 @@
>  		MLDS_Decls, MLDS_Statements).
>  
>  ml_gen_goal_expr(foreign_proc(Attributes,
> -                PredId, ProcId, ArgVars, ArgDatas, OrigArgTypes, PragmaImpl),
> +		PredId, ProcId, ArgVars, ArgDatas, OrigArgTypes, PragmaImpl),
>  		CodeModel, OuterContext, MLDS_Decls, MLDS_Statements) -->
> -        (
> -                { PragmaImpl = ordinary(C_Code, _MaybeContext) },
> -                ml_gen_ordinary_pragma_foreign_proc(CodeModel, Attributes,
> -                        PredId, ProcId, ArgVars, ArgDatas, OrigArgTypes,
> -                        C_Code, OuterContext, MLDS_Decls, MLDS_Statements)
> -        ;
> -                { PragmaImpl = nondet(
> -                        LocalVarsDecls, LocalVarsContext,
> +	(
> +		{ PragmaImpl = ordinary(C_Code, _MaybeContext) },
> +		ml_gen_ordinary_pragma_foreign_proc(CodeModel, Attributes,
> +			PredId, ProcId, ArgVars, ArgDatas, OrigArgTypes,
> +			C_Code, OuterContext, MLDS_Decls, MLDS_Statements)
> +	;
> +		{ PragmaImpl = nondet(
> +			LocalVarsDecls, LocalVarsContext,
>  			FirstCode, FirstContext, LaterCode, LaterContext,
>  			_Treatment, SharedCode, SharedContext) },
> -                ml_gen_nondet_pragma_foreign_proc(CodeModel, Attributes,
> -                        PredId, ProcId, ArgVars, ArgDatas, OrigArgTypes,
> +		ml_gen_nondet_pragma_foreign_proc(CodeModel, Attributes,
> +			PredId, ProcId, ArgVars, ArgDatas, OrigArgTypes,
>  			OuterContext, LocalVarsDecls, LocalVarsContext,
>  			FirstCode, FirstContext, LaterCode, LaterContext,
>  			SharedCode, SharedContext, MLDS_Decls, MLDS_Statements)
>
> @@ -2010,10 +2010,10 @@
>  		{ PragmaImpl = import(Name, HandleReturn, Vars, _Context) },
>  		{ C_Code = string__append_list([HandleReturn, " ",
>  				Name, "(", Vars, ");"]) },
> -                ml_gen_ordinary_pragma_foreign_proc(CodeModel, Attributes,
> -                        PredId, ProcId, ArgVars, ArgDatas, OrigArgTypes,
> -                        C_Code, OuterContext, MLDS_Decls, MLDS_Statements)
> -        ).
> +		ml_gen_ordinary_pragma_foreign_proc(CodeModel, Attributes,
> +			PredId, ProcId, ArgVars, ArgDatas, OrigArgTypes,
> +			C_Code, OuterContext, MLDS_Decls, MLDS_Statements)
> +	).
>  
>  ml_gen_goal_expr(shorthand(_), _, _, _, _) -->
>  	% these should have been expanded out by now

Are those all just whitespace changes?

Next time please do those separately, or use the `-b' option to `diff'.

> +:- pred ml_gen_ordinary_pragma_il_proc(code_model, 
> +	pragma_foreign_proc_attributes, pred_id, proc_id, list(prog_var),
> +	list(maybe(pair(string, mode))), list(prog_type), string, prog_context,
> +	mlds__defns, mlds__statements, ml_gen_info, ml_gen_info).
> +:- mode ml_gen_ordinary_pragma_il_proc(in, in, in, in, in, in, in, in, in,
> +	out, out, in, out) is det.
> +
> +ml_gen_ordinary_pragma_il_proc(_CodeModel, Attributes,
> +	PredId, ProcId, ArgVars, _ArgDatas, OrigArgTypes,
> +	ForeignCode, Context, MLDS_Decls, MLDS_Statements) -->
> +
> +	{ MLDSContext = mlds__make_context(Context) },
> +
> +	=(MLDSGenInfo),
> +	{ ml_gen_info_get_module_info(MLDSGenInfo, ModuleInfo) },
> +	{ module_info_pred_proc_info(ModuleInfo, PredId, ProcId,
> +			_PredInfo, ProcInfo) },
> +	{ proc_info_varset(ProcInfo, VarSet) },
> +%	{ proc_info_vartypes(ProcInfo, VarTypes) },
> +	% note that for headvars we must use the types from
> +	% the procedure interface, not from the procedure body
> +	{ HeadVarTypes = map__from_corresponding_lists(ArgVars,
> +		OrigArgTypes) },
> +	{ ml_gen_info_get_byref_output_vars(MLDSGenInfo, ByRefOutputVars) },
> +	{ ml_gen_info_get_value_output_vars(MLDSGenInfo, CopiedOutputVars) },
> +	{ module_info_name(ModuleInfo, ModuleName) },
> +	{ MLDSModuleName = mercury_module_name_to_mlds(ModuleName) },
> +
> +	{ list__filter_map(	
> +		(pred(Var::in, Statement::out) is semidet :- 
> +			map__lookup(HeadVarTypes, Var, Type),
> +			not type_util__is_dummy_argument_type(Type),
> +			VarName = mlds__var_name(VarNameString, _MangleInt),
> +			MLDSType = mercury_type_to_mlds_type(ModuleInfo, Type),
> +
> +			VarName = ml_gen_var_name(VarSet, Var),
> +			QualVarName = qual(MLDSModuleName, VarName),
> +			OutputVarLval = mem_ref(lval(
> +				var(QualVarName, MLDSType)), MLDSType),
> +
> +			NonMangledVarName = mlds__var_name(VarNameString, no),
> +			QualLocalVarName= qual(MLDSModuleName,
> +				NonMangledVarName),
> +			LocalVarLval = var(QualLocalVarName, MLDSType),
> +
> +			Statement = ml_gen_assign(OutputVarLval,
> +				lval(LocalVarLval), Context)
> +		), ByRefOutputVars, ByRefAssignStatements) },

I think this code won't handle the case where the types in the
procedure interface are polymorphic, but the types of the
vars in the `foreign_proc' HLDS goal are concrete instances of
those types, which can happen when the procedure is inlined
or specialized.  The assignment that you generate here with
ml_gen_assign won't be type-correct.  In general you may need
to box/unbox the arguments.

> +:- func inline_code_to_il_asm(list(target_code_component)) = instr_tree.
> +inline_code_to_il_asm([]) = empty.
> +inline_code_to_il_asm([T | Ts]) = tree(Instrs, Rest) :-
> +	( 
> +		T = user_target_code(Code, MaybeContext, Attrs),
> +		( yes(max_stack_size(N)) = get_max_stack_attribute(Attrs) ->
> +			Instrs = tree__list([
> +				( MaybeContext = yes(Context) ->
> +					context_node(mlds__make_context(
> +						Context))
> +				;
> +					empty
> +				),
> +				instr_node(il_asm_code(Code, N))
> +				])
> +		;
> +			error(this_file ++ ": max_stack_size not set")
> +		)
> +	;
> +		T = raw_target_code(Code, Attrs),
> +		( yes(max_stack_size(N)) = get_max_stack_attribute(Attrs) ->
> +			Instrs = instr_node(il_asm_code(Code, N))
> +		;
> +			error(this_file ++ ": max_stack_size not set")
> +		)
> +	;
> +		T = target_code_input(_),
> +		Instrs = empty
> +	;
> +		T = target_code_output(_),
> +		Instrs = empty
> +	;
> +		T = name(_ `with_type` mlds__qualified_entity_name),

Is the explicit type qualifier really needed here?

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