[m-rev.] for review: MLDS back-end: hoist static constants

Fergus Henderson fjh at cs.mu.OZ.AU
Fri Jul 13 20:27:24 AEST 2001


On 09-Jul-2001, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> Estimated hours taken: 3
> Branches: main
> 
> Change the MLDS back-end so that it hoists *all* static constants out
> to the top level.

I've finally managed to merge this back with the current CVS head.
There were some additional changes required as a result of both syntactic
and semantic clashes during the merge.  I tried using interdiff, but
interdiff got very confused, so the output wasn't useful.
So here's a new full diff instead.
I'll go ahead and commit this now.

Estimated hours taken: 30
Branches: main

Change the MLDS back-end so that it hoists *all* static constants out
to the top level.

This is required for the IL and Java targets since they don't support
function-local static variables.  It doesn't cost anything for the
C or GCC targets, so for consistency we may as well do it for all targets.

Also some related bug fixes in the IL back-end.
With these changes, string switches now work.

compiler/ml_elim_nested.m:
	Hoist all static constants, rather than just hoisting those that
	might be referenced from nested functions or from other static
	constants that need to be hoisted.

compiler/mlds.m:
	Document that ml_elim_nested.m does not change the access for
	hoisted static constants from `local' to `private'.

compiler/ml_util.m:
	Fix a bug where the behaviour of defn_is_public did not match
	its documentation.  This caused wrong behaviour in mlds_to_c.m
	when handling hoisted static constants.

compiler/mlds_to_il.m:
	Don't assume that variables which are not local vars or arguments
	are defined in foreign code (cpp_code).  Instead, assume that they
	are static fields, unless they come from private_builtin.m.

	Don't assume that all fields have type `il_array_type', i.e.
	`object[]'.  Instead, use the type specified in the MLDS.

Workspace: /home/mars/fjh/ws1/mercury
Index: compiler/ml_elim_nested.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/ml_elim_nested.m,v
retrieving revision 1.34
diff -u -d -b -r1.34 ml_elim_nested.m
--- compiler/ml_elim_nested.m	2001/07/12 15:44:52	1.34
+++ compiler/ml_elim_nested.m	2001/07/12 22:45:11
@@ -191,6 +191,12 @@
 		%
 		list__filter(ml_decl_is_static_const, Locals,
 			LocalStatics, LocalVars),
+		%
+		% Fix up access flags on the statics that we're going to hoist:
+		% convert "local" to "private"
+		%
+		HoistedStatics = list__map(convert_local_to_global, LocalStatics),
+
 
 		%
 		% if there were no nested functions, then we just
@@ -198,7 +204,7 @@
 		%
 		( NestedFuncs0 = [] ->
 			FuncBody = FuncBody1,
-			HoistedDefns = LocalStatics
+			HoistedDefns = HoistedStatics
 		;
 			%
 			% Create a struct to hold the local variables,
@@ -215,7 +221,7 @@
 					no, InsertedEnv),
 
 			% Hoist out the local statics and the nested functions
-			HoistedDefns0 = list__append(LocalStatics, NestedFuncs),
+			HoistedDefns0 = list__append(HoistedStatics, NestedFuncs),
 
 			% 
 			% It's possible that none of the nested
@@ -266,6 +272,7 @@
 				HoistedDefns = HoistedDefns0
 			)
 		),
+
 		DefnBody = mlds__function(PredProcId, Params,
 			defined_here(FuncBody)),
 		Defn = mlds__defn(Name, Context, Flags, DefnBody),
@@ -463,6 +470,18 @@
 		Flags = Flags0
 	).
 
+	% Similarly, when converting local statics into
+	% global statics, we need to change `local' access
+	% to something else -- we use `private'.
+:- func convert_local_to_global(mlds__defn) = mlds__defn.
+convert_local_to_global(mlds__defn(Name, Context, Flags0, Body)) =
+		mlds__defn(Name, Context, Flags, Body) :-
+	( access(Flags0) = local ->
+		Flags = set_access(Flags0, private)
+	;
+		Flags = Flags0
+	).
+
 	% ml_insert_init_env:
 	%	If the definition is a nested function definition, and it's
 	%	body makes use of the environment pointer (`env_ptr'), then
@@ -868,9 +887,20 @@
 		=(ElimInfo),
 		{ ModuleName = elim_info_get_module_name(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(ModuleName, VarName,
 				FollowingDefns, FollowingStatements) }
+			)
 		->
 			elim_info_add_local_data(Defn0),
 			{ Defns = [] }
@@ -898,15 +928,16 @@
 	%
 	% This checks for a nested function definition
 	% or static initializer that references the variable.
-	% This is conservative; we only need to hoist out
+	% This is conservative; for the MLDS->C and MLDS->GCC
+	% back-ends, we only need to hoist out
 	% static variables if they are referenced by
 	% static initializers which themselves need to be
 	% hoisted because they are referenced from a nested
 	% function.  But checking the last part of that
-	% is tricky, so currently we just hoist more
-	% of the static consts than we strictly need to.
-	% Perhaps it would be simpler to just hoist *all*
-	% static consts.
+	% is tricky, and for the Java and IL back-ends we
+	% need to hoist out all static constants anyway,
+	% so to keep things simple we do the same for the
+	% C back-end to, i.e. we always hoist all static constants.
 	%
 :- pred ml_should_add_local_data(mlds_module_name, mlds__var_name,
 		mlds__defns, mlds__statements).
Index: compiler/ml_util.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/ml_util.m,v
retrieving revision 1.12
diff -u -d -b -r1.12 ml_util.m
--- compiler/ml_util.m	2001/07/12 15:44:53	1.12
+++ compiler/ml_util.m	2001/07/12 18:37:40
@@ -317,7 +317,7 @@
 
 defn_is_public(Defn) :-
 	Defn = mlds__defn(_Name, _Context, Flags, _Body),
-	access(Flags) \= private.
+	access(Flags) = public.
 
 %-----------------------------------------------------------------------------%
 %
Index: compiler/mlds.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/mlds.m,v
retrieving revision 1.62
diff -u -d -b -r1.62 mlds.m
--- compiler/mlds.m	2001/07/12 15:44:53	1.62
+++ compiler/mlds.m	2001/07/13 10:18:16
@@ -654,8 +654,8 @@
 				% access: accessible to anything defined
 				% in the same package.
 	%
-	% used for entities defined in a block/2 statement,
-	% i.e. local variables and nested functions
+	% Used for entities defined in a block/2 statement,
+	% i.e. local variables and nested functions.
 	%
 	;	local		% only accessible within the block
 				% in which the entity (variable or
Index: compiler/mlds_to_il.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/mlds_to_il.m,v
retrieving revision 1.45
diff -u -d -b -r1.45 mlds_to_il.m
--- compiler/mlds_to_il.m	2001/07/12 15:44:56	1.45
+++ compiler/mlds_to_il.m	2001/07/12 18:45:35
@@ -711,11 +711,13 @@
 		% and instructions to initialize it.
 		% See the comments about class constructors to
 		% find out why we do this.
-	data_initializer_to_instrs(DataInitializer, AllocInstrsTree,
+	data_initializer_to_instrs(DataInitializer, Type, AllocInstrsTree,
 			InitInstrTree),
 
 		% Make a field reference for the field
-	{ FieldRef = make_fieldref(il_array_type, ClassName, FieldName) },
+	DataRep =^ il_data_rep,
+	{ ILType = mlds_type_to_ilds_type(DataRep, Type) },
+	{ FieldRef = make_fieldref(ILType, ClassName, FieldName) },
 
 		% If we had to allocate memory, the code
 		% we generate looks like this:
@@ -783,12 +785,10 @@
 	il_info_add_alloc_instructions(AllocInstrs),
 	il_info_add_init_instructions(InitInstrs),
 
-	DataRep =^ il_data_rep,
-	{ IlType = mlds_type_to_ilds_type(DataRep, Type) },
 	{ MaybeOffset = no },
 	{ Initializer = none },
 
-	{ ClassDecl = field(Attrs, IlType, FieldName,
+	{ ClassDecl = field(Attrs, ILType, FieldName,
 			MaybeOffset, Initializer) }.
 
 generate_method(_, IsCons, defn(Name, Context, Flags, Entity), ClassDecl) -->
@@ -970,8 +970,8 @@
 				{ StoreLvalInstrs = node([]) },
 				{ NameString = "unknown" }
 			),
-			data_initializer_to_instrs(Initializer, AllocInstrs,
-				InitInstrs),
+			data_initializer_to_instrs(Initializer, MLDSType,
+				AllocInstrs, InitInstrs),
 			{ string__append("initializer for ", NameString, 
 				Comment) },
 			{ Tree = tree__list([
@@ -991,21 +991,37 @@
 	% initialize this value, leave it on the stack.
 	% XXX the code generator doesn't box these values
 	% we need to look ahead at them and box them appropriately.
-:- pred data_initializer_to_instrs(mlds__initializer::in,
+:- pred data_initializer_to_instrs(mlds__initializer::in, mlds__type::in,
 	instr_tree::out, instr_tree::out, il_info::in, il_info::out) is det.
-data_initializer_to_instrs(init_obj(Rval), node([]), InitInstrs) --> 
+data_initializer_to_instrs(init_obj(Rval), _Type, node([]), InitInstrs) --> 
 	load(Rval, InitInstrs).
 
 	% Currently, structs are the same as arrays.
-data_initializer_to_instrs(init_struct(InitList), AllocInstrs, InitInstrs) --> 
-	data_initializer_to_instrs(init_array(InitList), AllocInstrs, 
-		InitInstrs).
+data_initializer_to_instrs(init_struct(InitList), Type,
+		AllocInstrs, InitInstrs) --> 
+	data_initializer_to_instrs(init_array(InitList), Type,
+		AllocInstrs, InitInstrs).
 
 	% Put the array allocation in AllocInstrs.
 	% For sub-initializations, we don't worry about keeping AllocInstrs
 	% and InitInstrs apart, since we are only interested in top level
 	% allocations.
-data_initializer_to_instrs(init_array(InitList), AllocInstrs, InitInstrs) -->
+data_initializer_to_instrs(init_array(InitList), Type,
+		AllocInstrs, InitInstrs) -->
+		%
+		% figure out the array element type
+		%
+	DataRep =^ il_data_rep,
+	( { Type = mlds__array_type(ElemType0) } ->
+		{ ElemType = ElemType0 },
+		{ ILElemType = mlds_type_to_ilds_type(DataRep, ElemType) }
+	;
+		% XXX we assume struct fields have type mlds__generic_type
+		% This is probably wrong for --high-level-data
+		{ ElemType = mlds__generic_type },
+		{ ILElemType = il_generic_type }
+	),
+	{ ILElemType = ilds__type(_, ILElemSimpleType) },
 
 		% To initialize an array, we generate the following
 		% code:
@@ -1022,21 +1038,29 @@
 		%
 		% The initialization will leave the array on the stack.
 		%	
-	{ AllocInstrs = node([ldc(int32, i(list__length(InitList))), 
-		newarr(il_generic_type)]) },
+	{ AllocInstrs = node([
+		ldc(int32, i(list__length(InitList))), 
+		newarr(ILElemType)]) },
 	{ AddInitializer = 
 		(pred(Init0::in, X0 - Tree0::in, (X0 + 1) - Tree::out,
 				in, out) is det -->
-			maybe_box_initializer(Init0, Init),
-			data_initializer_to_instrs(Init, ATree1, ITree1),
+			% we may need to box the arguments
+			% XXX is this right?
+			( { ElemType = mlds__generic_type } ->
+				maybe_box_initializer(Init0, Init)
+			;
+				{ Init = Init0 }
+			),
+			data_initializer_to_instrs(Init, ElemType,
+				ATree1, ITree1),
 			{ Tree = tree(tree(Tree0, node(
 					[dup, ldc(int32, i(X0))])), 
 				tree(tree(ATree1, ITree1), 
-					node([stelem(il_generic_simple_type)]
+					node([stelem(ILElemSimpleType)]
 				))) }
 		) },
 	list__foldl2(AddInitializer, InitList, 0 - empty, _ - InitInstrs).
-data_initializer_to_instrs(no_initializer, node([]), node([])) --> [].
+data_initializer_to_instrs(no_initializer, _, node([]), node([])) --> [].
 
 	% If we are initializing an array or struct, we need to box
 	% all the things inside it.
@@ -1622,8 +1646,7 @@
 		; is_argument(MangledVarStr, Info) ->
 			Instrs = instr_node(ldarg(name(MangledVarStr)))
 		;
-			FieldRef = make_fieldref_for_handdefined_var(DataRep,
-				Var, VarType),
+			FieldRef = make_static_fieldref(DataRep, Var, VarType),
 			Instrs = instr_node(ldsfld(FieldRef))
 		}
 	; { Lval = field(_MaybeTag, Rval, FieldNum, FieldType, ClassType) },
@@ -1708,8 +1731,7 @@
 		; is_argument(MangledVarStr, Info) ->
 			Instrs = instr_node(ldarga(name(MangledVarStr)))
 		;
-			FieldRef = make_fieldref_for_handdefined_var(DataRep,
-				Var, VarType),
+			FieldRef = make_static_fieldref(DataRep, Var, VarType),
 			Instrs = instr_node(ldsfld(FieldRef))
 		}
 	; { Lval = field(_MaybeTag, Rval, FieldNum, FieldType, ClassType) },
@@ -1751,8 +1773,7 @@
 	; is_argument(MangledVarStr, Info) ->
 		Instrs = instr_node(starg(name(MangledVarStr)))
 	;
-		FieldRef = make_fieldref_for_handdefined_var(DataRep, Var,
-			VarType),
+		FieldRef = make_static_fieldref(DataRep, Var, VarType),
 		Instrs = instr_node(stsfld(FieldRef))
 	}.
 
@@ -2466,18 +2487,18 @@
 
 
 	% If an mlds__var is not an argument or a local, what is it?
-	% We assume the given variable is a handwritten RTTI reference or a
+	% We assume the given variable is a static field;
+	% either a compiler-generated static,
+	% or possibly a handwritten RTTI reference or a
 	% reference to some hand-written code in the
-	% modulename__cpp_code class.  This is OK so long as the
-	% code generator uses real 'field' lvals to reference
-	% fields in the modulename class.
+	% modulename__cpp_code class.
 
-:- func make_fieldref_for_handdefined_var(il_data_rep, mlds__var, mlds__type)
+:- func make_static_fieldref(il_data_rep, mlds__var, mlds__type)
 	 = fieldref.
-make_fieldref_for_handdefined_var(DataRep, Var, VarType) = FieldRef :-
-	Var = qual(ModuleName, _),
+make_static_fieldref(DataRep, Var, VarType) = FieldRef :-
+	Var = qual(ModuleName, VarName),
 	mangle_mlds_var(Var, MangledVarStr),
-	mangle_dataname_module(no, ModuleName, NewModuleName),
+	mangle_dataname_module(yes(var(VarName)), ModuleName, NewModuleName),
 	ClassName = mlds_module_name_to_class_name(NewModuleName),
 	FieldRef = make_fieldref(
 		mlds_type_to_ilds_type(DataRep, VarType), ClassName,
@@ -2548,6 +2569,7 @@
 		SymName = mlds_module_name_to_sym_name(ModuleName0),
 		SymName = qualified(qualified(unqualified("mercury"),
 			LibModuleName0), wrapper_class_name),
+		(
 		DataName = rtti(rtti_type_id(_, Name, Arity),
 			_RttiName),
 		( LibModuleName0 = "builtin",
@@ -2577,6 +2599,10 @@
 			; Name = "typeclass_info", Arity = 1
 			)
 		)		  
+		;
+			DataName = var(_),
+			LibModuleName0 = "private_builtin"
+		)
 	->
 		string__append(LibModuleName0, "__cpp_code",
 			LibModuleName),

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------



More information about the reviews mailing list