[m-dev.] for review: lcc compile the compiler

[Peter Ross]

On 26-Oct-1998, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> On 26-Oct-1998, Peter Ross <petdr at cs.mu.OZ.AU> wrote:
> > 
> > Fergus could you please review this.
> > It seems that the change from (const Word *) to (Word *), hasn't caused
> > a huge number of problems.  Seeing that the change is so simple, I
> > thought that it would be better to try and incorporate the output of
> > consts in the correct places in a seperate change.
> 
> Fine.
> 
> > This bootchecks using lcc on kryten, and I am currently bootchecking
> > using cc on kryten.  cc on murlibobo barfs because the runtime contains
> > lines like
> > 
> > #ifdef XXX
> >     #define YYY /* causes the barf */
> > #endif
> 
> On murlibobo, `cc' defaults to K&R C.
> Try `cc -std1'.
> 
Done. Bootchecked.

> (For portability testing, you could also try `cc -std1 -migrate'
> on murlibobo -- that gives you a different C compiler.)
> 
> > +++ options.m	1998/10/23 00:04:38
> > @@ -152,6 +152,7 @@
> >  		;	unboxed_float
> >  		;	sync_term_size % in words
> >  		;	type_layout
> > +		;	max_jump_table_size
> >  	% Options for internal use only
> >  	% (the values of these options are implied by the
> >  	% settings of other options)
> > @@ -449,6 +450,8 @@
> >  					% of writing) - will usually be over-
> >  					% ridden by a value from configure.
> >  	type_layout		-	bool(yes),
> > +	max_jump_table_size	-	int(0),
> > +					% 0 indicates any size.
> 
> That comment is a bit ambiguous.
> I suggest the following instead:
> 
> 	The special value 0 indicates that there is no limit on the
> 	jump table size.
> 
> > +		"--max-jump-table-size",
> > +		"\tThe maximum number of entries a jump table can have.",
> > +		"\t0 indicates any size",
> 
> Likewise.
> 
> Also I think it would be good to explain the purpose of this option:
> 
> 	This option can be useful to avoid exceeding fixed limits
> 	imposed by some C compilers.
> 
> Also don't forget to document this option in doc/user_guide.texi too.
> 

Done. A revised diff is coming.

> > +transform_llds(LLDS0, LLDS) -->
> > +	globals__io_lookup_int_option(max_jump_table_size, Size),
> > +	(
> > +		{ Size = 0 }
> > +	->	
> > +		{ LLDS = LLDS0 }
> > +	;
> > +		globals__io_lookup_bool_option(split_c_files, SplitFiles),
> > +		( { SplitFiles = yes } ->
> > +			{ LLDS0 = c_file(ModuleName, C_HeaderInfo, 
> > +					C_Modules0) },
> > +			transform_c_modules(C_Modules0, ModuleName, 
> > +					C_HeaderInfo, C_Modules),
> > +			{ LLDS = c_file(ModuleName, C_HeaderInfo, C_Modules) }
> > +		;
> > +			transform_c_file(LLDS0, LLDS)
> > +		)
> > +	).
> 
> I don't understand why it needs to do different things depending
> on the setting of --split-c-files.  A comment here would be helpful.
> 
> > +++ mercury_trace_internal.c	1998/10/24 01:30:37
> > @@ -1393,7 +1393,7 @@
> >  		}
> >  
> >  		this_frame = MR_saved_sp(saved_regs);
> > -		MR_saved_succip(saved_regs) = MR_based_stackvar(this_frame,
> > +		MR_saved_succip(saved_regs) = (Word *) MR_based_stackvar(this_frame,
> 
> This line is >= 80 columns.
> 

Fixed. (ts=4 problem)

> > +     %
> > +     % proc_label(Is, P)
> > +     %
> > +     % Find the proc_label, P, which is to be used for local labels
> > +     % in the list of instructions, Is.
> > +     % XXX Fergus, I am sure there is a better way to do this.  Any
> > +     % suggestions?
> > +     %
> 
> Well, you could take note of the pred_proc_id stored in the c_procedure
> in transform_c_procedure, and use code_util__make_proc_label using
> this pred_proc_id.  But that requires you to pass down the HLDS module_info.
> 
> Or you could define proc_label using opt_util__get_prologue,
> e.g. as
> 
> 	proc_label(Instructions, ProcLabel) :-
> 		opt_util__get_prologue(Instructions, ProcLabel, _, _, _).
> 
> opt_util__get_prologue works in a similar manner to your implementation
> of proc_label.
> 
Thanks.

Pete.