[m-rev.] for review: MLDS back-end: generate closure layouts

[Peter Ross]

On Sat, Mar 02, 2002 at 12:50:59AM +1100, Fergus Henderson wrote:
> Here's the new full diff.
> 
> Estimated hours taken: 20
> Branches: main
> 
> Generate closure layouts for the MLDS back-end.
> 
> compiler/ml_unify_gen.m:
> 	Add code to generate closure layouts.
> 	XXX Note that we still don't fill in the MR_closure_id field yet.
> 
Is this a major limitation?  What requires this field?

I was unable to review the changes to this module well, as I don't
really understand the data structures being used, but the code looks
fine to me.

> compiler/rtti_to_mlds.m:
> 	Mark RTTI definitions as `final', and document why.
> 
I would add the rationale to why here, seeing that it is short.

> Index: compiler/ml_unify_gen.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/ml_unify_gen.m,v
> retrieving revision 1.52
> diff -u -d -r1.52 ml_unify_gen.m
> --- compiler/ml_unify_gen.m	20 Feb 2002 03:14:13 -0000	1.52
> +++ compiler/ml_unify_gen.m	1 Mar 2002 13:35:07 -0000
> @@ -88,12 +88,18 @@
>  :- import_module hlds_out, builtin_ops.
>  :- import_module ml_code_gen, ml_call_gen, ml_type_gen.
>  :- import_module prog_util, type_util, mode_util.
> -:- import_module rtti, error_util.
> -:- import_module code_util. % XXX needed for `code_util__cons_id_to_tag'.
> +:- import_module pseudo_type_info, rtti, rtti_to_mlds, error_util.
>  :- import_module globals, options.
>  
> +% XXX The following modules depend on the LLDS,
> +% so ideally they should not be used here.
> +:- import_module code_util. 	    % needed for `cons_id_to_tag'.
> +:- import_module continuation_info. % needed for `generate_closure_layout'
> +:- import_module stack_layout.      % needed for `represent_locn_as_int'
> +:- import_module llds.       	    % needed for `layout_locn'
> +

Is it possible to abstract these out?  It seems that we are getting more
on more dependencies between the MLDS and LLDS as each day goes by.

>  :- import_module bool, int, string, list, map, require, std_util, term, varset.
> -:- import_module assoc_list.
> +:- import_module assoc_list, set.
>  
>  %-----------------------------------------------------------------------------%
>  

> Index: compiler/stack_layout.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/stack_layout.m,v
> retrieving revision 1.61
> diff -u -d -r1.61 stack_layout.m
> --- compiler/stack_layout.m	20 Feb 2002 03:14:19 -0000	1.61
> +++ compiler/stack_layout.m	27 Feb 2002 14:40:23 -0000
> @@ -42,6 +42,8 @@
>  	create_arg_types::out, comp_gen_c_data::out,
>  	counter::in, counter::out) is det.
>  
> +:- pred stack_layout__represent_locn_as_int(layout_locn::in, int::out) is det.
> +

Seeing that this predicate is now in the interface it should be
documented what it does.

>  :- implementation.
>  
>  :- import_module globals, options, llds_out, trace_params, trace.
> @@ -1214,8 +1216,6 @@
>  stack_layout__represent_locn_as_int_rval(Locn, Rval) :-
>  	stack_layout__represent_locn_as_int(Locn, Word),
>  	Rval = const(int_const(Word)).
> -
> -:- pred stack_layout__represent_locn_as_int(layout_locn::in, int::out) is det.
>  
>  stack_layout__represent_locn_as_int(direct(Lval), Word) :-
>  	stack_layout__represent_lval(Lval, Word).


Apart from the fact I can't really comment on the correctness of
ml_unify_gen.m this change looks fine to me.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------