for review: new method of handling failures, part 1 of 6

[Thomas Charles CONWAY]

Zoltan writes:
> compiler/options.m:
> 	Introduce a new option, allow_hijacks, which is set to "yes" by
> 	default. It is not used yet, but the idea is that when it is set to no,
> 	the code generator will not generate code that hijacks the nondet
> 	stack frame of another procedure invocation; instead, it will create
> 	a new temporary nondet stack frame. If the current procedure is
> 	model_non, it will have three fields: prevfr, redoip and redofr.
> 	If the current procedure is model_det or model_semi, it will have
> 	a fourth field that is set to the value of MR_sp. The idea is that
> 	the runtime system, which will be able to distinguish between
> 	ordinary frames (whose size is at least 5 words), 3-word and 4-word
> 	temporary frames, will now be able to use the redofr slots of
> 	all three kinds of frames and the fourth slot values of 4-word
> 	temporary frames as the addresses relative to which framevars
> 	and detstackvars respectively ought to be offset in stack layouts.

This doesn't appear to be documented anywhere (other than the log message).
Perhaps somewhere in the notes/ would be a good idea.

> runtime/mercury_imp.h:
> 	If MR_DISABLE_REDOFR is defined, undefine off MR_USE_REDOFR before
s/off/of/ ?

> 	% Create a code address which holds the address of the specified
> 	% procedure.
> 	% The fourth argument should be `no' if the the caller wants the
> 	% returned address to be valid from everywhere in the program.
> 	% If being valid from within the current procedure is enough,
> 	% this argument should be `yes' wrapped around the value of the
> 	% --procs-per-c-function option and the current procedure id.
> 	% Using an address that is only valid from within the current
> 	% procedure may make jumps more efficient.
> 	% XXX

The 4th argument isn't a maybe, so the comment doesn;t match the decl.
It might be a good idea to create a new type for this with more descriptive
names.
Is discrepancy the reason for this XXX?

>  :- pred code_info__make_entry_label(module_info, pred_id, proc_id, bool,
> 	code_addr, code_info, code_info).
>  :- mode code_info__make_entry_label(in, in, in, in, out, in, out) is det.
> 

> 	% note: we must be careful to apply deaths before births

I know I probably wrote this comment, but it doesn't say why. I assume
that it is because a variable may have two lifetimes in adjacent 
conjoined scopes or something. If you know the reason, could you
improve the comment?

>  :- type position_info
> 	--->	position_info(
> 			code_info	% The code_info at a given position
> 					% in the code of the procedure.
> 		).
> 
>  :- type branch_end_info
> 	--->	branch_end_info(
> 			code_info	% The code_info at the end of a branch.
> 		).
> 
> code_info__remember_position(position_info(C), C, C).
> 
> code_info__reset_to_position(position_info(PosCI), CurCI, NextCI) :-
> 		% The static fields in PosCI and CurCI should be identical.
> 	PosCI  = code_info(_,  _,  _,  _,  _,  _,  _,  _, 
> 		LA, LB, LC, LD, LE, LF, _,  _,  _,  _,  _,  _ ),
> 	CurCI  = code_info(SA, SB, SC, SD, SE, SF, SG, SH,
> 		_,  _,  _,  _,  _,  _,  PA, PB, PC, PD, PE, PF),
> 	NextCI = code_info(SA, SB, SC, SD, SE, SF, SG, SH,
> 		LA, LB, LC, LD, LE, LF, PA, PB, PC, PD, PE, PF).

At one stage you talked about making the position info a structure just
containing the fields of the code info that get remembered to force
maintainers who add a field to code info to work out whether or not
it should be remembered. Did you change your mind, or is that an item
for further work.

>  :- type disj_hijack_info
> 	--->	disj_no_hijack
> 	;	disj_quarter_hijack
> 	;	disj_half_hijack(
> 			lval		% the stack slot in which we saved
> 					% the value of the hijacked redoip
> 		)
> 	;	disj_full_hijack(
> 			lval,		% the stack slot in which we saved
> 					% the value of the hijacked redoip
> 			lval		% the stack slot in which we saved
> 					% the value of the hijacked redofr
> 		).

A couple of comments that describe the meaning of the constructors would
be helpful here.

> 
>  :- type ite_hijack_info
> 	--->	ite_no_hijack(
> 			resume_point_known
> 		)
> 	;	ite_quarter_hijack(
> 			resume_point_known
> 		)
> 	;	ite_half_hijack(
> 			resume_point_known,
> 			lval		% the stack slot in which we saved
> 					% the value of the hijacked redoip
> 		)
> 	;	ite_full_hijack(
> 			resume_point_known,
> 			lval,		% the stack slot in which we saved
> 					% the value of the hijacked redoip
> 			lval,		% the stack slot in which we saved
> 					% the value of the hijacked redofr
> 			lval		% the stack slot in which we saved
> 					% the value of maxfr
> 		).

Likewise.

>  :- type commit_hijack_info
> 	--->	commit_quarter_hijack
> 	;	commit_half_hijack(
> 			lval		% the stack slot in which we saved
> 					% the value of the hijacked redoip
> 		)
> 	;	commit_full_hijack(
> 			lval,		% the stack slot in which we saved
> 					% the value of the hijacked redoip
> 			lval,		% the stack slot in which we saved
> 					% the value of the hijacked redofr
> 			lval		% the stack slot in which we saved
> 					% the value of maxfr
> 		).

Likewise.


-- 
Thomas Conway <conway at cs.mu.oz.au>
Nail here [] for new monitor.  )O+