for review: stack layouts for execution tracing

[Tyson Dowd]

> > +	% Generate the list of lval - live_value_type pairs and the
> > +	% typeinfo variable - lval pairs for any type variables in
> > +	% the types of the input variables.
> > +
> > +:- pred code_gen__generate_lvaltypes(list(var), list(lval),
> > +		assoc_list(lval, live_value_type), assoc_list(var, lval),
> > +		code_info, code_info).
> > +:- mode code_gen__generate_lvaltypes(in, in, out, out, in, out) is det.
> 
> Don't you want to use this predicate for the output args as well?

Comment is misleading. s/input/given/

> 
> > +code_gen__generate_lvaltypes(Vars, Lvals, LvalTypes, TypeInfos) -->
> > +	code_info__get_instmap(InstMap),
> > +	list__map_foldl(code_info__variable_type, Vars, Types),
> > +	{ list__map(instmap__lookup_var(InstMap), Vars, Insts) },
> > +	{ assoc_list__from_corresponding_lists(Types, Insts,
> > +		TypeInsts) },
> > +	{ list__map(lambda([TypeInst::in, LiveType::out] is det, (
> > +			TypeInst = Type - Inst,
> > +			LiveType = var(Type, Inst))), 
> > +		TypeInsts, LiveTypes) },
> 
> It may be worthwhile adding a new predicate to assoc_list.m that
> invokes a user-supplied predicate for each pair of list elements,
> instead of wrapping them up inside a pair, before putting the result
> into the zipped list.
> 

As Fergus noted in mercury-developers, it might be better to make the
compiler optimize this case away instead. 

> >  :- type proc_layout_info
> >  	--->
> >  		proc_layout_info(
> > +			maybe(proc_layout_general_info),
> > +			map(label, internal_layout_info),
> > +					% info for each internal label
> > +			maybe(continuation_label_info),  % entry
> > +			maybe(continuation_label_info)	 % exit
> > +					% live data information about
> > +					% entry and exit points
> > +		).
> > +
> > +:- type proc_layout_general_info
> > +	--->
> > +		proc_layout_general_info(
> >  			proc_label,	% the proc label
> >  			int,		% number of stack slots
> >  			code_model,	% which stack is used
> > -			maybe(int),	% location of succip on stack
> > -			map(label, internal_layout_info)
> > -					% info for each internal label
> > +			maybe(int)	% location of succip on stack
> 				 	^ % (if any)
> >  		).
> 
> Why is the proc_layout_general_info inside a maybe?
> 
> I presume that the maybe(continuation_label_info) will be yes with tracing
> and no otherwise. Please document this.

I've added some documentation about the maybes.

> 
> Similarly that the map of internal_layout_infos will be empty unless you
> have agc set.

No, because for the moment we are making them available for execution
tracing too (with just a link to the layout info for the procedure). 
They are also needed for stack traces. In fact, for the moment,
basic_stack_layouts will set them.

Having agc set will fill the internal_layout_infos with liveness
information (as well as the link to the procedure).

> > +stack_layout__construct_agc_rvals(Internal, RvalList) -->
> > +	stack_layout__get_agc_stack_layout(AgcStackLayout),
> > +	( 
> > +		{ AgcStackLayout = yes }
> > +	->
> > +		{ Internal = internal_layout_info(ContinuationLabelInfo) },
> > +		{
> > +			ContinuationLabelInfo = yes(continuation_label_info(
> > +				LiveLvalSet0, TVars0))
> > +		->
> > +			LiveLvalSet = LiveLvalSet0,
> > +			TVars = TVars0
> > +		;
> > +			% We record no live values here. This might not be
> > +			% true, however this label is not being used as a
> > +			% continuation, so it shouldn't be relied upon.
> > +			
> > +			set__init(LiveLvalSet),
> > +			set__init(TVars)
> > +		},
> > +		stack_layout__construct_livelval_rvals(LiveLvalSet, TVars,
> > +			RvalList)
> > +	;
> > +		{ RvalList = [yes(const(int_const(0))), 
> > +			yes(const(int_const(0)))] }
> > +	).
> 
> I know this is old code that was just moved around, but I don't understand it.
> The comment doesn't help, since I don't what "it" cannot be relied upon,
> and by whom.

Yep, it's a dodgy comment. How's this?

			% This label is not being used as a continuation,
                        % or we are not doing accurate GC, so we record
                        % no live values here. 
                        % This might not be a true reflection of the
                        % liveness at this point, so the values cannot
                        % be relied upon by the runtime system unless
                        % you know you are at a continuation (and doing
                        % accurate GC).

I'll send a revised diff soon (after I check Fergus' comments on the
diff).

-- 
       Tyson Dowd           # 
                            #  Surreal humour isn't eveyone's cup of
     trd at cs.mu.oz.au        #  fur.
http://www.cs.mu.oz.au/~trd #