[m-rev.] for review: --eliminate-local-vars

[Peter Ross]

On Thu, Feb 07, 2002 at 04:34:16AM +1100, Fergus Henderson wrote:
> Estimated hours taken: 12
> Branches: main
> 
> Add a new MLDS->MLDS optimization option, --eliminate-local-variables.
> The aim of this pass is to improve performance in cases where local
> variables are costly -- for nondeterministic procedures,
> for MLDS->C accurate GC, and for the .NET back-end.
> 
For interest, why are local variables costly on the .NET back-end?

> compiler/ml_optimize.m:
> 	Add a pass to eliminate local variables.
> 	Also, don't optimize tail calls unless --optimize-tailcalls is set.
> 
> compiler/mercury_compile.m:
> 	Invoke the ml_optimize pass once before ml_elim_nested
> 	(with --optimize-tailcalls disabled), as well as once after it.
> 
> compiler/options.m:	
> doc/user_guide.texi:
> 	Add an option --eliminate-local-variables to enable the new
> 	optimization.
> 
> compiler/ml_elim_nested.m:
> compiler/ml_util.m:
> 	Move statement_contains_var and definition_contains_var
> 	from ml_elim_nested.m to ml_util.m, for use by ml_optimize.m.
> 
> compiler/ml_elim_nested.m:
> 	- Handle local variables with initializers; this is neccessary
> 	  now that ml_optimize gets run before ml_elim_nested.
> 	- Don't allocate a stack frame struct and link it into the chain
> 	  if there are no local variables that contain pointers.
> 	- In fixup_atomic_statement, handle foreign_proc_code more consistently.
> 	- Add some comments.
> 
> XXX remember to check that this diff includes all
>     the relevant files that have been changed!
> 
Have you checked this XXX? ;)

> Workspace: /home/earth/fjh/ws-earth4/mercury
> Index: compiler/ml_elim_nested.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/ml_elim_nested.m,v
> retrieving revision 1.49
> diff -u -d -r1.49 ml_elim_nested.m
> --- compiler/ml_elim_nested.m	4 Feb 2002 07:01:25 -0000	1.49
> +++ compiler/ml_elim_nested.m	6 Feb 2002 17:14:09 -0000
> @@ -1481,23 +1508,32 @@
>  %	Hoist out nested function definitions, and any local variables
>  %	that need to go in the environment struct (e.g. because they are
>  %	referenced by nested functions), storing them both in the elim_info.
> +%	Convert initializers for local variables that need to go in the
> +%	environment struct into assignment statements.
> +%	Return the remaining (non-hoisted) definitions,
> +% 	the list of assignment statements, and the updated elim_info.
>  %
>  
>  :- pred flatten_nested_defns(mlds__defns, mlds__statements, mlds__defns,
> -		elim_info, elim_info).
> -:- mode flatten_nested_defns(in, in, out, in, out) is det.
> +		mlds__statements, elim_info, elim_info).
> +:- mode flatten_nested_defns(in, in, out, out, in, out) is det.
>  
> -flatten_nested_defns([], _, []) --> [].
> -flatten_nested_defns([Defn0 | Defns0], FollowingStatements, Defns) -->
> -	flatten_nested_defn(Defn0, Defns0, FollowingStatements, Defns1),
> -	flatten_nested_defns(Defns0, FollowingStatements, Defns2),
> -	{ Defns = list__append(Defns1, Defns2) }.
> +flatten_nested_defns([], _, [], []) --> [].
> +flatten_nested_defns([Defn0 | Defns0], FollowingStatements, Defns,
> +		InitStatements) -->
> +	flatten_nested_defn(Defn0, Defns0, FollowingStatements,
> +		Defns1, InitStatements1),
> +	flatten_nested_defns(Defns0, FollowingStatements,
> +		Defns2, InitStatements2),
> +	{ Defns = Defns1 ++ Defns2 },
> +	{ InitStatements = InitStatements1 ++ InitStatements2 }.
>  
>  :- pred flatten_nested_defn(mlds__defn, mlds__defns, mlds__statements,
> -		mlds__defns, elim_info, elim_info).
> -:- mode flatten_nested_defn(in, in, in, out, in, out) is det.
> +		mlds__defns, mlds__statements, elim_info, elim_info).
> +:- mode flatten_nested_defn(in, in, in, out, out, in, out) is det.
>  
> -flatten_nested_defn(Defn0, FollowingDefns, FollowingStatements, Defns) -->
> +flatten_nested_defn(Defn0, FollowingDefns, FollowingStatements,
> +		Defns, InitStatements) -->
>  	{ Defn0 = mlds__defn(Name, Context, Flags0, DefnBody0) },
>  	(
>  		{ DefnBody0 = mlds__function(PredProcId, Params, FuncBody0,
> @@ -1579,9 +1615,10 @@
>  			{ Defns = [] }
>  		;
>  			{ Defns = [Defn] }
> -		)
> +		),
> +		{ InitStatements = [] }
>  	;
> -		{ DefnBody0 = mlds__data(Type, Init, MaybeGCTraceCode0) },
> +		{ DefnBody0 = mlds__data(Type, Init0, MaybeGCTraceCode0) },
>  		%
>  		% for local variable definitions, if they are
>  		% referenced by any nested functions, then
> @@ -1589,31 +1626,54 @@
>  		%
>  		=(ElimInfo),
>  		(
> -			(
> -				% For IL and Java, we need to hoist all
> -				% static constants out to the top level,
> -				% so that they can be initialized in the
> -				% class constructor.
> -				% To keep things consistent (and reduce
> -				% the testing burden), we do the same for
> -				% the other back-ends too.
> -				{ ml_decl_is_static_const(Defn0) }
> -			;
> -				{ Name = data(var(VarName)) },
> -				{ ml_should_add_local_data(ElimInfo,
> -					VarName, MaybeGCTraceCode0,
> -					FollowingDefns, FollowingStatements) }
> -			)
> +			% For IL and Java, we need to hoist all
> +			% static constants out to the top level,
> +			% so that they can be initialized in the
> +			% class constructor.
> +			% To keep things consistent (and reduce
> +			% the testing burden), we do the same for
> +			% the other back-ends too.
> +			{ ml_decl_is_static_const(Defn0) }
>  		->
>  			elim_info_add_and_flatten_local_data(Defn0),
> +			{ Defns = [] },
> +			{ InitStatements = [] }
> +		;
> +			% Hoist ordinary local variables
> +			{ Name = data(var(VarName)) },
> +			{ ml_should_add_local_data(ElimInfo,
> +				VarName, MaybeGCTraceCode0,
> +				FollowingDefns, FollowingStatements) }
> +		->
> +			% we need to strip out the initializer (if any)
> +			% and convert it into an assignment statement,
> +			% since this local variable is going to become
> +			% a field, and fields can have initializers.

I assume that can have initializers should be *cannot* have initializers.

> +			( { Init0 = init_obj(Rval) } ->
> +				{ Init1 = no_initializer },
> +				{ DefnBody1 = mlds__data(Type, Init1,
> +					MaybeGCTraceCode0) },
> +				{ Defn1 = mlds__defn(Name, Context, Flags0,
> +					DefnBody1) },
> +				{ VarLval = var(qual(ElimInfo ^ module_name,
> +					VarName), Type) },
> +				{ InitStatements = [mlds__statement(
> +					atomic(assign(VarLval, Rval)),
> +					Context)] }
> +			;
> +				{ Defn1 = Defn0 },
> +				{ InitStatements = [] }
> +			),
> +			elim_info_add_and_flatten_local_data(Defn1),
>  			{ Defns = [] }
>  		;
> +			fixup_initializer(Init0, Init),
>  			flatten_maybe_statement(MaybeGCTraceCode0,
>  				MaybeGCTraceCode),
>  			{ DefnBody = mlds__data(Type, Init, MaybeGCTraceCode) },
>  			{ Defn = mlds__defn(Name, Context, Flags0, DefnBody) },
> -
> -			{ Defns = [Defn] }
> +			{ Defns = [Defn] },
> +			{ InitStatements = [] }
>  		)
>  	;
>  		{ DefnBody0 = mlds__class(_) },


> Index: compiler/options.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/options.m,v
> retrieving revision 1.350
> diff -u -d -r1.350 options.m
> --- compiler/options.m	16 Dec 2001 08:11:09 -0000	1.350
> +++ compiler/options.m	6 Feb 2002 14:35:44 -0000
> @@ -1097,9 +1099,11 @@
>  long_option("auto-comments",		auto_comments).
>  long_option("show-dependency-graph",	show_dependency_graph).
>  long_option("dump-hlds",		dump_hlds).
> +long_option("hlds-dump",		dump_hlds).
>  long_option("dump-hlds-alias",		dump_hlds_alias).
>  long_option("dump-hlds-options",	dump_hlds_options).
>  long_option("dump-mlds",		dump_mlds).
> +long_option("mlds-dump",		dump_mlds).
>  long_option("dump-rl",			dump_rl).
>  long_option("dump-rl-bytecode",		dump_rl_bytecode).
>  long_option("sign-assembly",		sign_assembly).

This change is not mentioned, I suggest doing this change seperately.

>  % Optimization level 6: apply optimizations which may have any
> @@ -1800,6 +1808,32 @@
>  	use_macro_for_redo_fail	-	bool(yes)
>  ]).
>  
> +% The following optimization options are not enabled at any level:
> +%
> +% 	checked_nondet_tailcalls:
> +%		This is deliberate, because the transformation
> +%		might make code run slower.
> +%
> +% 	constraint_propagation:
> +%		I think this is deliberate, because the transformation
> +%		might make code run slower?
> +%
> +%	prev_code:
> +%		Not useful?
> +%
> +%	unneeded_code:
> +%	type_specialization:
> +%	optimize_rl_invariant:
> +%		XXX why not?
> +%
> +%	introduce_accumulators:
> +%		XXX Disabled until a bug in extras/trailed_update/var.m
> +%		is resolved.
> +%
I believe that this bug has been fixed.  I remember fixing it a long time ago.
I will add it to my list of things to do.

> +%	optimize_rl_cse:
> +%	optimize_constructor_last_call:
> +%		Not implemented yet.
> +
>  %-----------------------------------------------------------------------------%
>  
>  options_help -->
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------