[m-dev.] for review: MLDS switch optimization

Tyson Dowd trd at cs.mu.OZ.AU
Thu Nov 9 01:21:12 AEDT 2000


On 07-Nov-2000, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> This change breaks the IL back-end, but the work-around is easy
> (compile with `--no-smart-indexing').  I'm hoping Tyson will review
> fix the following XXX in ilasm.m, which is the cause of that problem,
> 
>         % XXX we really should implement this since we will use it
> 	% eventually.
> 	output_instr(switch(_)) --> { error("output not implemented") }.
> 
> since "eventually" has now arrived ;-)
> I'm also hoping that Tyson and/or Zoltan will review this change.

I'm working on a fix for the switch instruction in IL.

> +
> +	% Generate a new label number for use in label statements.
> +	% This is used to give unique names to the case labels generated
> +	% for dense switch statements.
> +:- type label_num == int.
> +:- pred ml_gen_info_new_label(label_num, ml_gen_info, ml_gen_info).
> +:- mode ml_gen_info_new_label(out, in, out) is det.
> +
>  	% A number corresponding to an MLDS nested function which serves as a
>  	% label (i.e. a continuation function).
>  :- type ml_label_func == mlds__func_sequence_num.
> @@ -1200,6 +1213,10 @@
>  	),
>  	MLDS_Module = mercury_module_name_to_mlds(DefiningModule).
>  
> +ml_gen_new_label(Label) -->
> +	ml_gen_info_new_label(LabelNum),
> +	{ string__format("label_%d", [i(LabelNum)], Label) }.
> +
>  %-----------------------------------------------------------------------------%
>  %
>  % Code for dealing with variables
> @@ -1679,6 +1696,7 @@
>  
>  			func_label :: mlds__func_sequence_num,
>  			commit_label :: commit_sequence_num,
> +			label :: label_num,
>  			cond_var :: cond_seq,
>  			conv_var :: conv_seq,
>  			const_num :: const_seq,
> @@ -1705,6 +1723,7 @@
>  	ByRefOutputVars = select_output_vars(ModuleInfo, HeadVars, HeadModes,
>  		VarTypes),
>  
> +	CaseLabelCounter = 0,
>  	FuncLabelCounter = 0,
>  	CommitLabelCounter = 0,
>  	CondVarCounter = 0,
> @@ -1722,6 +1741,7 @@
>  			VarSet,
>  			VarTypes,
>  			ByRefOutputVars,
> +			CaseLabelCounter,
>  			FuncLabelCounter,
>  			CommitLabelCounter,
>  			CondVarCounter,
> @@ -1756,6 +1776,9 @@
>  	=(Info),
>  	{ ml_gen_info_get_module_info(Info, ModuleInfo) },
>  	{ module_info_globals(ModuleInfo, Globals) }.
> +
> +ml_gen_info_new_label(Label, Info, Info^label := Label) :-
> +	Label = Info^label + 1.
>  
>  ml_gen_info_new_func_label(Label, Info, Info^func_label := Label) :-
>  	Label = Info^func_label + 1.

We should really use the counter module in the library to implement
counters such as this.

> -/******
> -	% Is it worth including this?  We already have `computed_goto'...
> +		% This representation for switches is very general:
> +		% it allows switching on any type, and cases can match
> +		% on ranges as well as on values.
> +		% Many target languages only allow switches on ints or enums.
> +		% Some (e.g. C#) also allow switches on strings.
> +		% Most target languages only allow matching on values;
> +		% only some (e.g. GNU C) allow matching on ranges.

This comment is fine, but it doesn't say what the consequences are.
Should the backend be responsible for simplifying switches to what is
available, or should the MLDS code generator look at the backend before
generating a switch?  (your code does the latter).

> +mlds_output_case_cond(Indent, Context, match_range(Low, High)) -->
> +	% This uses the GNU C extension `case <Low> ... <High>:'.
> +	mlds_indent(Context, Indent),
> +	io__write_string("case "),
> +	mlds_output_rval(Low),
> +	io__write_string(" ... "),
> +	mlds_output_rval(High),
> +	io__write_string(":\n").

Isn't this going to cause Pete Ross to thump you when it breaks the
VC++ backend?  

Have I missed the bit where you avoid generating this for non GNU C
compilers?  Or is it not used yet?

> +
> +% This option is not yet documented because it is not yet useful -- currently
> +% we don't take advantage of GNU C's computed gotos extension.
> +%		"--no-prefer-switch",
> +%		"\tGenerate code using computed gotos rather than switches.",
> +%		"\tThis makes the generated code less readable, but potentially",
> +%		"\tslightly more efficient.",
> +%		"\tThis option has no effect unless the `--high-level-code' option",
> +%		"\tis enabled.  It also has no effect if the `--target' option is",
> +%		"\tset to `il'.",
> +

Perhaps you're over-doing it by adding commented out documentation to
describe the cases where an unimplemented option has no effect.

Other than those (sometimes cynical) comments the diff is fine.

-- 
       Tyson Dowd           # 
                            #  Surreal humour isn't everyone's cup of fur.
     trd at cs.mu.oz.au        # 
http://www.cs.mu.oz.au/~trd #
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------



More information about the developers mailing list