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

[Fergus Henderson]

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'.

(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.

> +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.

> +     %
> +     % 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.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.