[m-dev.] for review: handle `:- external' in MLDS back-end

[Tyson Dowd]

On 10-Dec-2000, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> ----------
> 
> Estimated hours taken: 3
> 
> Fix the handling of procedures declared `:- external' for the MLDS back-end.
> This change fixes some problems that broke many of the test cases in
> tests/valid in the hl*prof* grades.
> 
> compiler/hlds_pred.m:
> 	Add `external' as a new import_status, rather than using
> 	`imported'.
> 
> compiler/assertion.m:
> compiler/higher_order.m:
> compiler/hlds_out.m:
> compiler/hlds_pred.m:
> compiler/intermod.m:
> compiler/make_hlds.m:
> 	Minor changes to handle the new import_status.
> 
> compiler/mlds.m:
> 	Add a comment about the handling of procedures declared
> 	`:- external'.
> 
> compiler/ml_code_gen.m:
> 	Generate MLDS function definitions, with no function body,
> 	for procedures declared `external'.
> 
> compiler/mlds_to_c.m:
> 	Declare private functions with no function body as `extern'
> 	rather than `static'.
> 

Thanks Fergus, this is good.  A few comments:

> Index: compiler/hlds_pred.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/hlds_pred.m,v
> retrieving revision 1.88
> diff -u -d -r1.88 hlds_pred.m
> --- compiler/hlds_pred.m	2000/12/06 06:05:04	1.88
> +++ compiler/hlds_pred.m	2000/12/09 10:41:16
> @@ -258,9 +258,11 @@
>  	% Only types can have status abstract_exported or abstract_imported.
>  
>  :- type import_status
> -	--->	imported(section)
> +	--->	external(section)
> +				% declared `external',
> +				% i.e. defined in some other language
> +	;	imported(section)
>  				% defined in the interface of some other module
> -				% or `external' (in some other language)
>  	;	opt_imported	% defined in the optimization
>  				% interface of another module
>  	;	abstract_imported % describes a type with only an abstract

IMO external doesn't really mean defined in some other language.

It means an implementation for this predicate/function will be provided
by an external source (and specifically, not through Mercury clauses).

This could be through the use of another language, or it could be
through automatic generation by the compiler, or some other method we
haven't thought of yet.


> Index: compiler/ml_code_gen.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/ml_code_gen.m,v
> retrieving revision 1.68
> diff -u -d -r1.68 ml_code_gen.m
> --- compiler/ml_code_gen.m	2000/11/23 04:32:43	1.68
> +++ compiler/ml_code_gen.m	2000/12/09 14:09:03
> @@ -876,10 +876,11 @@
>  		{ PredIds0 = [PredId|PredIds] }
>  	->
>  		{ map__lookup(PredTable, PredId, PredInfo) },
> -		( { pred_info_is_imported(PredInfo) } ->
> +		{ pred_info_import_status(PredInfo, ImportStatus) },
> +		( { ImportStatus = imported(_) } ->
>  			{ MLDS_Defns1 = MLDS_Defns0 }
>  		;
> -			ml_gen_pred(ModuleInfo, PredId, PredInfo,
> +			ml_gen_pred(ModuleInfo, PredId, PredInfo, ImportStatus,
>  				MLDS_Defns0, MLDS_Defns1)
>  		),
>  		ml_gen_preds_2(ModuleInfo, PredIds, PredTable,
> @@ -891,12 +892,17 @@
>  	% Generate MLDS definitions for all the non-imported
>  	% procedures of a given predicate (or function).
>  	%
> -:- pred ml_gen_pred(module_info, pred_id, pred_info,
> -				mlds__defns, mlds__defns, io__state, io__state).
> -:- mode ml_gen_pred(in, in, in, in, out, di, uo) is det.
> +:- pred ml_gen_pred(module_info, pred_id, pred_info, import_status,
> +			mlds__defns, mlds__defns, io__state, io__state).
> +:- mode ml_gen_pred(in, in, in, in, in, out, di, uo) is det.
>  
> -ml_gen_pred(ModuleInfo, PredId, PredInfo, MLDS_Defns0, MLDS_Defns) -->
> -	{ pred_info_non_imported_procids(PredInfo, ProcIds) },
> +ml_gen_pred(ModuleInfo, PredId, PredInfo, ImportStatus,
> +		MLDS_Defns0, MLDS_Defns) -->
> +	( { ImportStatus = external(_) } ->
> +		{ pred_info_procids(PredInfo, ProcIds) }
> +	;
> +		{ pred_info_non_imported_procids(PredInfo, ProcIds) }
> +	),
>  	( { ProcIds = [] } ->
>  		{ MLDS_Defns = MLDS_Defns0 }
>  	;
> @@ -993,6 +999,7 @@
>  ml_gen_proc_defn(ModuleInfo, PredId, ProcId, MLDS_ProcDefnBody, ExtraDefns) :-
>  	module_info_pred_proc_info(ModuleInfo, PredId, ProcId,
>  			PredInfo, ProcInfo),
> +	pred_info_import_status(PredInfo, ImportStatus),
>  	pred_info_arg_types(PredInfo, ArgTypes),
>  	proc_info_interface_code_model(ProcInfo, CodeModel),
>  	proc_info_headvars(ProcInfo, HeadVars),
> @@ -1018,65 +1025,83 @@
>  	MLDSGenInfo0 = ml_gen_info_init(ModuleInfo, PredId, ProcId),
>  	MLDS_Params = ml_gen_proc_params(ModuleInfo, PredId, ProcId),
>  
> -	% Set up the initial success continuation, if any.
> -	% Also figure out which output variables are returned by
> -	% value (rather than being passed by reference) and remove
> -	% them from the byref_output_vars field in the ml_gen_info.
> -	( CodeModel = model_non ->
> -		ml_set_up_initial_succ_cont(ModuleInfo, CopiedOutputVars,
> -			MLDSGenInfo0, MLDSGenInfo1)
> +	( ImportStatus = external(_) ->
> +		%
> +		% For Mercury procedures declared `:- external', we generate 
> +		% an MLDS definition for them with no function body.
> +		% The MLDS -> target code pass can treat this accordingly,
> +		% e.g. for C it outputs a function declaration with no
> +		% corresponding definition, making sure that the function
> +		% is declared as `extern' rather than `static'.
> +		%
> +		MaybeStatement = no,
> +		ExtraDefns = []

FYI: I plan to make MLDS -> IL turn it into a forwarding call (much like
it does for pragma foreign_code("MC++", somepred(....), ...)).  So you
can provide an implementation by writing a static method (with the right
mangling) in Managed C++. 

This will solve the problem at the moment, where :- external doesn't
work because there is no forwarding call generated from
`modulename.predname' to `modulename__c_code.predname'.

>  	;
> -		(
> -			is_output_det_function(ModuleInfo, PredId, ProcId,
> -				ResultVar)
> -		->
> -			CopiedOutputVars = [ResultVar],
> -			ml_gen_info_get_byref_output_vars(MLDSGenInfo0,
> -				ByRefOutputVars0),
> -			list__delete_all(ByRefOutputVars0,
> -				ResultVar, ByRefOutputVars),
> -			ml_gen_info_set_byref_output_vars(ByRefOutputVars,	
> -				MLDSGenInfo0, MLDSGenInfo1)
> +		% XXX FIXME wrap long lines

Should this still be here?

> +		% Set up the initial success continuation, if any.
> +		% Also figure out which output variables are returned by
> +		% value (rather than being passed by reference) and remove
> +		% them from the byref_output_vars field in the ml_gen_info.
> +		( CodeModel = model_non ->
> +			ml_set_up_initial_succ_cont(ModuleInfo,
> +				CopiedOutputVars, MLDSGenInfo0, MLDSGenInfo1)
>  		;
> -			CopiedOutputVars = [],
> -			MLDSGenInfo1 = MLDSGenInfo0
> -		)
> -	),
> +			(
> +				is_output_det_function(ModuleInfo, PredId,
> +					ProcId, ResultVar)
> +			->
> +				CopiedOutputVars = [ResultVar],
> +				ml_gen_info_get_byref_output_vars(MLDSGenInfo0,
> +					ByRefOutputVars0),
> +				list__delete_all(ByRefOutputVars0,
> +					ResultVar, ByRefOutputVars),
> +				ml_gen_info_set_byref_output_vars(
> +					ByRefOutputVars,
> +					MLDSGenInfo0, MLDSGenInfo1)
> +			;
> +				CopiedOutputVars = [],
> +				MLDSGenInfo1 = MLDSGenInfo0
> +			)
> +		),
>  
> -	% This would generate all the local variables at the top of the
> -	% function:
> -	%	MLDS_LocalVars = ml_gen_all_local_var_decls(Goal, VarSet,
> -	% 		VarTypes, HeadVars, ModuleInfo),
> -	% But instead we now generate them locally for each goal.
> -	% We just declare the `succeeded' var here,
> -	% plus locals for any output arguments that are returned by value
> -	% (e.g. if --nondet-copy-out is enabled, or for det function return
> -	% values).
> -	MLDS_Context = mlds__make_context(Context),
> -	( CopiedOutputVars = [] ->
> -		% optimize common case
> -		OutputVarLocals = []
> -	;
> -		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(HeadVars,
> -			ArgTypes),
> -		OutputVarLocals = ml_gen_local_var_decls(VarSet,
> -			map__overlay(VarTypes, HeadVarTypes),
> -			MLDS_Context, ModuleInfo, CopiedOutputVars)
> +		% This would generate all the local variables at the top of
> +		% the function:
> +		%	MLDS_LocalVars = ml_gen_all_local_var_decls(Goal,
> +		% 		VarSet, VarTypes, HeadVars, ModuleInfo),
> +		% But instead we now generate them locally for each goal.
> +		% We just declare the `succeeded' var here, plus locals
> +		% for any output arguments that are returned by value
> +		% (e.g. if --nondet-copy-out is enabled, or for det function
> +		% return values).
> +		MLDS_Context = mlds__make_context(Context),
> +		( CopiedOutputVars = [] ->
> +			% optimize common case
> +			OutputVarLocals = []
> +		;
> +			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(HeadVars,
> +				ArgTypes),
> +			OutputVarLocals = ml_gen_local_var_decls(VarSet,
> +				map__overlay(VarTypes, HeadVarTypes),
> +				MLDS_Context, ModuleInfo, CopiedOutputVars)
> +		),
> +		MLDS_LocalVars = [ml_gen_succeeded_var_decl(MLDS_Context) |
> +				OutputVarLocals],
> +		ml_gen_proc_body(CodeModel, HeadVars, ArgTypes,
> +				CopiedOutputVars, Goal,
> +				MLDS_Decls0, MLDS_Statements,
> +				MLDSGenInfo1, MLDSGenInfo),
> +		ml_gen_info_get_extra_defns(MLDSGenInfo, ExtraDefns),
> +		MLDS_Decls = list__append(MLDS_LocalVars, MLDS_Decls0),
> +		MLDS_Statement = ml_gen_block(MLDS_Decls, MLDS_Statements,
> +			Context),
> +		MaybeStatement = yes(MLDS_Statement)
>  	),
> -	MLDS_LocalVars = [ml_gen_succeeded_var_decl(MLDS_Context) |
> -			OutputVarLocals],
> -	ml_gen_proc_body(CodeModel, HeadVars, ArgTypes, CopiedOutputVars, Goal,
> -			MLDS_Decls0, MLDS_Statements,
> -			MLDSGenInfo1, MLDSGenInfo),
> -	ml_gen_info_get_extra_defns(MLDSGenInfo, ExtraDefns),
> -	MLDS_Decls = list__append(MLDS_LocalVars, MLDS_Decls0),
> -	MLDS_Statement = ml_gen_block(MLDS_Decls, MLDS_Statements, Context),
>  	MLDS_ProcDefnBody = mlds__function(yes(proc(PredId, ProcId)),
> -			MLDS_Params, yes(MLDS_Statement)).
> +			MLDS_Params, MaybeStatement).
>  
>  :- pred ml_set_up_initial_succ_cont(module_info, list(prog_var),
>  		ml_gen_info, ml_gen_info).

This appears to be just re-formatting -- it should be mentioned in the
log message.


You could also fix the error messages in make_hlds. 

Currently if we write clauses for a :- external predicate, we get error
messages that talk about declarations for imported predicates.

For example:

      (
                { pred_info_is_imported(PredInfo1) }
        ->
                { module_info_incr_errors(ModuleInfo1, ModuleInfo) },
                prog_out__write_context(Context),
                io__write_string("Error: `:- pragma foreign_code' (or `pragma c_code')\n"),
                prog_out__write_context(Context),
                io__write_string("declaration for imported "),
                hlds_out__write_simple_call_id(PredOrFunc, PredName/Arity),
                io__write_string(".\n"),
                { Info = Info0 }
        ;


Apart from that, this change is fine.  Thanks!

-- 
       Tyson Dowd           # 
                            #  Surreal humour isn't everyone's cup of fur.
     trd at cs.mu.oz.au        # 
http://www.cs.mu.oz.au/~trd #
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------