[m-dev.] diff: Compiler support for stack_layouts

[Tyson Richard DOWD]

Zoltan Somogyi wrote:
> 
> > However, in some other modules, it's called "CellNo" or "CNum", and
> > there is a code_info predicate called code_info__get_next_cell_number.
> > 
> > I think cell number is a better name, shall we use that? Then I can
> > clean up the code.
> 
> Yes, cell number is a better name.
> 
> However, my main concern was that the counter you were using is also
> in other data structures, and that the two copies of the counter can
> be used to allocate the same number for two different purposes.
> This could lead to bugs, such as the bug I suspect is lurking in
> space.m.

I concur. I'll fix it properly.

> 
> >>Please give an example how two different computation paths can leave different
> >>information alive and yet still end up at the same label. Offhand, I can't
> >>think of any.
> > 
> > If the code that follows the label is "succeed()", then it can happen.
> > 
> > This happens in tree234__member/3. The continuations of the recursive calls 
> > are all redirected to the single succeed() at the end of the code.
> 
> In such cases, multiple paths can end up at the same label, but I don't see
> why the livenesses should be different. In all paths, the live variables
> just before succeed should be the outputs of the procedure.

The outputs of the procedure + any framevars that may be live upon
backtracking. And perhaps the live framevars are different for different 
cases in the code for member. I'll have to take a closer look.

You can see this by looking at the C code with --auto-comments using
the installed compiler in normal asm_fast.gc grade.

> > (This actually reminds me of another point - any label that the redoip is
> > set to is also a continuation point).
> 
> An important point. This can happen via an assignment to redoip(_), or via
> mkframe/modframe.

I'm not going to handle it just yet, but it will have to be done
eventually.

> 
> > > > +	map__init(InternalMap),
> > > > +	EntryLayout = entry_layout_info(ProcLabel, StackSize, CodeModel,
> > > > +		SuccipLocation, InternalMap),
> > > > +	continuation_info__add_proc_layout(PredProcId, EntryLayout,
> > > > +		ContInfo0, ContInfo).
> > 
> > > Why is the internal map empty here?
> > 
> > Because you can't process the instructions to find information about
> > the internal labels until you have finished generating code and
> > optimizing the code. 
> 
> Yes, I knew that, but I don't know why you need the partial information.
> 
> > There should be some comments describing the two-stages of the
> > continuation_info process.
> 
> Yes.
> 
> > > Why do you need to initialize the second field independently of the first?
> > 
> > The first field is map that we build up after processing each procedure.
> > The second field is a map that is built up during processing each procedure.
> > It is reset at the start of each new procedure.
> > 
> > Do you think this should this be documented? (Since you had to ask, I
> > assume the answer is yes).
> 
> Yes.
> 
> > > > --- llds.m	1997/10/12 13:32:31	1.211
> > > > +++ llds.m	1997/10/21 01:54:18
> > > > @@ -425,7 +426,8 @@
> > > >  	;	float_const(float)
> > > >  	;	string_const(string)
> > > >  	;	code_addr_const(code_addr)
> > > > -	;	data_addr_const(data_addr).
> > > > +	;	data_addr_const(data_addr)
> > > > +	;	label_entry(label).
> > > 
> > > The argument here provides no information that is associated with the label.
> > > Why?
> > > 
> > 
> > I'm not sure what you mean. The label_entry is to facilitate outputting
> > ENTRY(mercury__some_old_predicate) references. It's a type of rval
> > constant.
> 
> You should document what its semantics are.
> 
> > > > @@ -2665,6 +2696,10 @@
> > > >  	% we need to cast them here to avoid type errors
> > > >  	io__write_string("(const Word *) &"),
> > > >  	output_data_addr(BaseName, VarName).
> > > > +output_rval_const(label_entry(Label)) -->
> > > > +	io__write_string("ENTRY("),
> > > > +	output_label(Label),
> > > > +	io__write_string(")").
> > > 
> > > Have you considered forwarding this case to the existing code that writes
> > > out labels e.g. for return addresses in calls?
> >  
> > It's a bit inconsistent with the existing code. The existing code
> > assumes that ENTRY is only used with proc_labels, not with internal
> > labels. But the accurate GC mechanism uses ENTRY on all labels.
> > 
> > I'd like to improve this somewaht, but the old design of how labels
> > work is pretty deeply ingrained, and seems a little fragile. I wasn't
> > keen to modify it (== rewrite it).
> 
> Then note this as something to clean up in the future.
> 
> > > Is there a special value meaning no succip in stack frame? Or is any out
> > > of range integer supposed to mean this?
> > 
> > Yes, this should be documented.
> 
> I think having a specific code (e.g. -1) for no succip would be more robust.

Sorry, -1 is how it is implemented already. (The "yes" was ambiguous).

> 
> > > > % For each internal label in a procedure (that is a continuation point)
> > > 
> > > Not all internal labels are continuation points. In addition, for tracing
> > > you want info about the entry and exit points, which are in general not
> > > continuation points.
> > 
> > The comment is incorrect.
> 
> What would be the correct comment?
> 

It should be:
	% For each continuation label in a procedure

for the code at the moment. In future it should be something different,
because of the entry and exit points.

-- 
       Tyson Dowd           # To fix this, edit BLAH\BlahKey\Blah\Whatever 
                            # in the registry.
     trd at cs.mu.oz.au        # *WARNING* Editing the registry can DESTROY
http://www.cs.mu.oz.au/~trd # your machine forever. Do not do it.