[m-rev.] [b.schmidt at ugrad.unimelb.edu.au: MLDS Accurate GC fixes]
Zoltan Somogyi
zs at csse.unimelb.edu.au
Thu Dec 14 19:38:17 AEDT 2006
----- Forwarded message from Ben Schmidt <b.schmidt at ugrad.unimelb.edu.au> -----
Date: Thu, 14 Dec 2006 16:52:10 +1100
From: Ben Schmidt <b.schmidt at ugrad.unimelb.edu.au>
To: Zoltan Somogyi <zs at csse.unimelb.edu.au>
Subject: MLDS Accurate GC fixes
Proposed diff and log message attached.
Ben.
Estimated hours taken: 25 (though some were spent learning build system, etc.)
Branches: <which CVS branches ("main", "release", etc.) this change
will be committed on>
Bug fixes for the MLDS accurate garbage collector.
compiler/mlds.m:
compiler/ml_code_util.m:
compiler/ml_elim_nested.m:
compiler/ml_closure_gen.m
compiler/mlds_to_c.m:
compiler/mlds_to_il.m:
Change type of mlds_maybe_gc_trace_code to a discriminated union
to allow separation of initialisation and variable tracing code,
rather than the standard yes/no maybe type. Update predicate/
function definitions accordingly.
compiler/ml_elim_nested.m:
- Update extract_gc_trace_code to provide initialisation and
variable tracing code separately.
- Change method of flattening so that pulling out of variables into
structs and changing references to those structs are
sequenced, not interleaved; this fixed a bug where references to
a variable could be processed before that variable was pulled out
into a struct, and thus the references not properly updated.
compiler/ml_closure_gen.m:
When generating definitions for 'closure_layout_ptr' and
'type_params', use 'init' for the garbage collection code, as
it is initialisation required by the tracing code for other
variables rather than variable tracing code itself, and as such
must occur first in the GC tracing function.
Index: compiler/ml_closure_gen.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/ml_closure_gen.m,v
retrieving revision 1.46
diff -u -r1.46 ml_closure_gen.m
--- compiler/ml_closure_gen.m 22 Aug 2006 05:03:53 -0000 1.46
+++ compiler/ml_closure_gen.m 11 Dec 2006 23:51:05 -0000
@@ -1073,8 +1073,11 @@
% This type is really `const MR_Closure_Layout *', but there's no easy
% way to represent that in the MLDS; using MR_Box instead works fine.
ClosureLayoutPtrType = mlds_generic_type,
+ % We use 'init' for the garbage collection code as it is code to
+ % initialise local variables used during garbage collection and must
+ % run before variables are traced.
ClosureLayoutPtrDecl = ml_gen_mlds_var_decl(var(ClosureLayoutPtrName),
- ClosureLayoutPtrType, yes(ClosureLayoutPtrGCInit), MLDS_Context),
+ ClosureLayoutPtrType, init(ClosureLayoutPtrGCInit), MLDS_Context),
ml_gen_var_lval(!.Info, ClosureLayoutPtrName, ClosureLayoutPtrType,
ClosureLayoutPtrLval),
@@ -1082,8 +1085,11 @@
% This type is really MR_TypeInfoParams, but there's no easy way to
% represent that in the MLDS; using MR_Box instead works fine.
TypeParamsType = mlds_generic_type,
+ % We use 'init' for the garbage collection code as it is code to
+ % initialise local variables used during garbage collection and must
+ % run before variables are traced.
TypeParamsDecl = ml_gen_mlds_var_decl(var(TypeParamsName),
- TypeParamsType, yes(TypeParamsGCInit), MLDS_Context),
+ TypeParamsType, init(TypeParamsGCInit), MLDS_Context),
ml_gen_var_lval(!.Info, TypeParamsName, TypeParamsType, TypeParamsLval),
(
ClosureKind = higher_order_proc_closure,
@@ -1182,7 +1188,11 @@
lval(TypeInfoLval), Context, MaybeGCTraceCode0, !Info),
(
- MaybeGCTraceCode0 = yes(CallTraceFuncCode),
+ (
+ MaybeGCTraceCode0 = yes(CallTraceFuncCode)
+ ;
+ MaybeGCTraceCode0 = init(CallTraceFuncCode)
+ ),
MakeTypeInfoCode = atomic(inline_target_code(lang_C, [
raw_target_code("{\n", []),
raw_target_code("MR_MemoryList allocated_mem = NULL;\n", []),
Index: compiler/ml_code_util.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/ml_code_util.m,v
retrieving revision 1.115
diff -u -r1.115 ml_code_util.m
--- compiler/ml_code_util.m 22 Aug 2006 05:03:54 -0000 1.115
+++ compiler/ml_code_util.m 11 Dec 2006 23:51:09 -0000
@@ -273,14 +273,14 @@
% and the code to trace it for accurate GC (if needed).
%
:- func ml_gen_mlds_var_decl(mlds_data_name, mlds_type,
- maybe(statement), mlds_context) = mlds_defn.
+ mlds_maybe_gc_trace_code, mlds_context) = mlds_defn.
% Generate a declaration for an MLDS variable, given its MLDS type
% and initializer, and given the code to trace it for accurate GC
% (if needed).
%
:- func ml_gen_mlds_var_decl(mlds_data_name, mlds_type, mlds_initializer,
- maybe(statement), mlds_context) = mlds_defn.
+ mlds_maybe_gc_trace_code, mlds_context) = mlds_defn.
% Generate declaration flags for a local variable.
%
@@ -464,7 +464,7 @@
% the variable.
%
:- pred ml_gen_maybe_gc_trace_code(mlds_var_name::in, mer_type::in,
- prog_context::in, maybe(statement)::out,
+ prog_context::in, mlds_maybe_gc_trace_code::out,
ml_gen_info::in, ml_gen_info::out) is det.
% ml_gen_maybe_gc_trace_code(Var, DeclType, ActualType, Context, Code):
@@ -486,7 +486,8 @@
% both.
%
:- pred ml_gen_maybe_gc_trace_code(mlds_var_name::in,
- mer_type::in, mer_type::in, prog_context::in, maybe(statement)::out,
+ mer_type::in, mer_type::in, prog_context::in,
+ mlds_maybe_gc_trace_code::out,
ml_gen_info::in, ml_gen_info::out) is det.
% ml_gen_maybe_gc_trace_code_with_typeinfo(Var, DeclType, TypeInfoRval,
@@ -500,7 +501,8 @@
% for the the local variables in closure wrapper functions.
%
:- pred ml_gen_maybe_gc_trace_code_with_typeinfo(mlds_var_name::in,
- mer_type::in, mlds_rval::in, prog_context::in, maybe(statement)::out,
+ mer_type::in, mlds_rval::in, prog_context::in,
+ mlds_maybe_gc_trace_code::out,
ml_gen_info::in, ml_gen_info::out) is det.
%-----------------------------------------------------------------------------%
@@ -1898,7 +1900,8 @@
; already_provided(mlds_rval).
:- pred ml_gen_maybe_gc_trace_code_2(mlds_var_name::in, mer_type::in,
- how_to_get_type_info::in, prog_context::in, maybe(statement)::out,
+ how_to_get_type_info::in, prog_context::in,
+ mlds_maybe_gc_trace_code::out,
ml_gen_info::in, ml_gen_info::out) is det.
ml_gen_maybe_gc_trace_code_2(VarName, DeclType, HowToGetTypeInfo, Context,
Index: compiler/ml_elim_nested.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/ml_elim_nested.m,v
retrieving revision 1.85
diff -u -r1.85 ml_elim_nested.m
--- compiler/ml_elim_nested.m 22 Aug 2006 05:03:54 -0000 1.85
+++ compiler/ml_elim_nested.m 11 Dec 2006 23:51:12 -0000
@@ -529,8 +529,8 @@
Params0 = mlds_func_params(Arguments0, RetValues),
ml_maybe_add_args(Arguments0, FuncBody0, ModuleName,
Context, ElimInfo0, ElimInfo1),
- flatten_arguments(Arguments0, Arguments1, ElimInfo1, ElimInfo2),
- flatten_statement(FuncBody0, FuncBody1, ElimInfo2, ElimInfo),
+ flatten_statement(FuncBody0, FuncBody1, ElimInfo1, ElimInfo2),
+ fixup_gc_trace_code(ElimInfo2, ElimInfo),
elim_info_finish(ElimInfo, NestedFuncs0, Locals),
% Split the locals that we need to process into local variables
@@ -595,7 +595,7 @@
% functions, or (for accurate GC) may contain pointers,
% then we need to copy them to local variables in the
% environment structure.
- ml_maybe_copy_args(Arguments1, FuncBody0, ElimInfo,
+ ml_maybe_copy_args(Arguments0, FuncBody0, ElimInfo,
EnvTypeName, EnvPtrTypeName, Context,
_ArgsToCopy, CodeToCopyArgs),
@@ -645,10 +645,10 @@
% annotation on the arguments anymore. We delete them here, because
% otherwise the `#if 0 ... #endif' blocks output for the
% annotations clutter up the generated C files.
- Arguments = list.map(strip_gc_trace_code, Arguments1)
+ Arguments = list.map(strip_gc_trace_code, Arguments0)
;
Action = hoist_nested_funcs,
- Arguments = Arguments1
+ Arguments = Arguments0
),
Params = mlds_func_params(Arguments, RetValues),
DefnBody = mlds_function(PredProcId, Params,
@@ -681,7 +681,7 @@
[], [FuncBody])
->
ml_conv_arg_to_var(Context, Arg, ArgToCopy),
- elim_info_add_and_flatten_local_data(ArgToCopy, !Info)
+ elim_info_add_local_data(ArgToCopy, !Info)
;
true
),
@@ -815,21 +815,23 @@
Fields0 = list.map(convert_local_to_field, LocalVars),
% Extract the GC tracing code from the fields.
- list.map2(extract_gc_trace_code, Fields0, Fields1, GC_TraceStatements0),
- GC_TraceStatements = list.condense(GC_TraceStatements0),
+ list.map3(extract_gc_trace_code, Fields0, Fields1,
+ GC_InitStatements,GC_TraceStatements),
+ list.append(GC_InitStatements,GC_TraceStatements,GC_Statements0),
+ GC_Statements = list.condense(GC_Statements1),
( Action = chain_gc_stack_frames ->
- ml_chain_stack_frames(Fields1, GC_TraceStatements, EnvTypeName,
+ ml_chain_stack_frames(Fields1, GC_Statements, EnvTypeName,
Context, FuncName, ModuleName, Globals, Fields, EnvInitializer,
LinkStackChain, GCTraceFuncDefns),
GC_TraceEnv = no
;
(
- GC_TraceStatements = [],
+ GC_Statements = [],
GC_TraceEnv = no
;
- GC_TraceStatements = [_ | _],
- GC_TraceEnv = yes(ml_block([], GC_TraceStatements, Context))
+ GC_Statements = [_ | _],
+ GC_TraceEnv = yes(ml_block([], GC_Statements, Context))
),
Fields = Fields1,
EnvInitializer = no_initializer,
@@ -1034,15 +1036,21 @@
Virtuality, Finality, Constness, Abstractness).
:- pred extract_gc_trace_code(mlds_defn::in, mlds_defn::out,
- statements::out) is det.
+ statements::out,statements::out) is det.
extract_gc_trace_code(mlds_defn(Name, Context, Flags, Body0),
- mlds_defn(Name, Context, Flags, Body), GCTraceStmts) :-
+ mlds_defn(Name, Context, Flags, Body), GCInitStmts, GCTraceStmts) :-
( Body0 = mlds_data(Type, Init, yes(GCTraceStmt)) ->
Body = mlds_data(Type, Init, no),
+ GCInitStmts = [],
GCTraceStmts = [GCTraceStmt]
+ ; Body0 = mlds_data(Type, Init, init(GCInitStmt)) ->
+ Body = mlds_data(Type, Init, no),
+ GCInitStmts = [GCInitStmt],
+ GCTraceStmts = []
;
Body = Body0,
+ GCInitStmts = [],
GCTraceStmts = []
).
@@ -1319,10 +1327,9 @@
%-----------------------------------------------------------------------------%
-% flatten_arguments:
-% flatten_argument:
% flatten_function_body:
% flatten_maybe_statement:
+% flatten_maybe_gc_trace_code:
% flatten_statements:
% flatten_statement:
% Recursively process the statement(s), calling fixup_var on every
@@ -1333,20 +1340,6 @@
% Also, for Action = chain_gc_stack_frames, add code to save and
% restore the stack chain pointer at any `try_commit' statements.
-:- pred flatten_arguments(mlds_arguments::in, mlds_arguments::out,
- elim_info::in, elim_info::out) is det.
-
-flatten_arguments(!Arguments, !Info) :-
- list.map_foldl(flatten_argument, !Arguments, !Info).
-
-:- pred flatten_argument(mlds_argument::in, mlds_argument::out,
- elim_info::in, elim_info::out) is det.
-
-flatten_argument(Argument0, Argument, !Info) :-
- Argument0 = mlds_argument(Name, Type, MaybeGCTraceCode0),
- flatten_maybe_statement(MaybeGCTraceCode0, MaybeGCTraceCode, !Info),
- Argument = mlds_argument(Name, Type, MaybeGCTraceCode).
-
:- pred flatten_function_body(mlds_function_body::in, mlds_function_body::out,
elim_info::in, elim_info::out) is det.
@@ -1363,6 +1356,16 @@
flatten_maybe_statement(yes(Statement0), yes(Statement), !Info) :-
flatten_statement(Statement0, Statement, !Info).
+:- pred flatten_maybe_gc_trace_code(mlds_maybe_gc_trace_code::in,
+ mlds_maybe_gc_trace_code::out,
+ elim_info::in, elim_info::out) is det.
+
+flatten_maybe_gc_trace_code(no, no, !Info).
+flatten_maybe_gc_trace_code(yes(Statement0), yes(Statement), !Info) :-
+ flatten_statement(Statement0, Statement, !Info).
+flatten_maybe_gc_trace_code(init(Statement0), init(Statement), !Info) :-
+ flatten_statement(Statement0, Statement, !Info).
+
:- pred flatten_statements(statements::in, statements::out,
elim_info::in, elim_info::out) is det.
@@ -1592,7 +1595,7 @@
% 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, !Info),
+ elim_info_add_local_data(Defn0, !Info),
Defns = [],
InitStatements = []
;
@@ -1621,13 +1624,11 @@
Defn1 = Defn0,
InitStatements = []
),
- elim_info_add_and_flatten_local_data(Defn1, !Info),
+ elim_info_add_local_data(Defn1, !Info),
Defns = []
;
fixup_initializer(Init0, Init, !Info),
- flatten_maybe_statement(MaybeGCTraceCode0, MaybeGCTraceCode,
- !Info),
- DefnBody = mlds_data(Type, Init, MaybeGCTraceCode),
+ DefnBody = mlds_data(Type, Init, MaybeGCTraceCode0),
Defn = mlds_defn(Name, Context, Flags0, DefnBody),
Defns = [Defn],
InitStatements = []
@@ -1658,7 +1659,11 @@
Action = Info ^ action,
(
Action = chain_gc_stack_frames,
- MaybeGCTraceCode = yes(_)
+ (
+ MaybeGCTraceCode = yes(_)
+ ;
+ MaybeGCTraceCode = init(_)
+ )
;
Action = hoist_nested_funcs,
ml_need_to_hoist(Info ^ module_name, DataName,
@@ -1698,32 +1703,6 @@
initializer_contains_var(Initializer, QualDataName)
).
- % Add the variable definition to the local_data field of the elim_info,
- % fix up any references inside the GC tracing code, and then update
- % the GC tracing code in the local_data.
- %
- % Note that we need to add the variable definition to the local_data
- % *before* we fix up the GC tracing code, otherwise references to the
- % variable itself in the GC tracing code won't get fixed.
- %
-:- pred elim_info_add_and_flatten_local_data(mlds_defn::in,
- elim_info::in, elim_info::out) is det.
-
-elim_info_add_and_flatten_local_data(Defn0, !Info) :-
- (
- Defn0 = mlds_defn(Name, Context, Flags, DefnBody0),
- DefnBody0 = mlds_data(Type, Init, MaybeGCTraceCode0)
- ->
- elim_info_add_local_data(Defn0, !Info),
- flatten_maybe_statement(MaybeGCTraceCode0, MaybeGCTraceCode, !Info),
- DefnBody = mlds_data(Type, Init, MaybeGCTraceCode),
- Defn = mlds_defn(Name, Context, Flags, DefnBody),
- elim_info_remove_local_data(Defn0, !Info),
- elim_info_add_local_data(Defn, !Info)
- ;
- elim_info_add_local_data(Defn0, !Info)
- ).
-
%-----------------------------------------------------------------------------%
% fixup_initializer:
@@ -1871,6 +1850,47 @@
fixup_lval(var(Var0, VarType), VarLval, !Info) :-
fixup_var(Var0, VarType, VarLval, !Info).
+% fixup_gc_trace_code:
+% Process the trace code in the locals that have been pulled out
+% to the stack frame structure so that the code correctly refers to any
+% variables that have been pulled out.
+% It assumes the locals don't actually change during the process.
+% I think this should be safe. (schmidt)
+
+:- pred fixup_gc_trace_code(elim_info::in, elim_info::out) is det.
+
+fixup_gc_trace_code(!Info) :-
+ Locals = elim_info_get_local_data(!.Info),
+ fixup_gc_trace_code_defns(Locals, !Info).
+
+:- pred fixup_gc_trace_code_defns(mlds_defns::in,
+ elim_info::in, elim_info::out) is det.
+
+fixup_gc_trace_code_defns([], !Info).
+fixup_gc_trace_code_defns([Defn0|Defns], !Info) :-
+ % The ordering of variables is important, and must not be disturbed,
+ % so this ordering is important. If other parts of the compiler
+ % end up changing the order variables are defined, this routine
+ % may need to change also, or flags for the important initialisations
+ % in GC trace code (closure_layout_ptr in ml_gen_closure.m, etc.)
+ % set so they are kept at the top.
+ % Ordering will become unimportant due to other modifications soon ???
+ % Wrecking the ordering for testing ???
+ %fixup_gc_trace_code_defns(Defns, !Info),
+ (
+ Defn0 = mlds_defn(Name, Context, Flags, DefnBody0),
+ DefnBody0 = mlds_data(Type, Init, MaybeGCTraceCode0)
+ ->
+ flatten_maybe_gc_trace_code(MaybeGCTraceCode0, MaybeGCTraceCode, !Info),
+ DefnBody = mlds_data(Type, Init, MaybeGCTraceCode),
+ Defn = mlds_defn(Name, Context, Flags, DefnBody),
+ elim_info_remove_local_data(Defn0, !Info),
+ elim_info_add_local_data(Defn, !Info)
+ ;
+ true
+ ), %.
+ fixup_gc_trace_code_defns(Defns, !Info).
+
%-----------------------------------------------------------------------------%
% Change up any references to local vars in the containing function
Index: compiler/mlds.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/mlds.m,v
retrieving revision 1.140
diff -u -r1.140 mlds.m
--- compiler/mlds.m 22 Aug 2006 05:03:56 -0000 1.140
+++ compiler/mlds.m 11 Dec 2006 23:51:15 -0000
@@ -588,7 +588,16 @@
% `no' here indicates that no GC tracing code is needed,
% e.g. because accurate GC isn't enabled, or because the
% variable can never contain pointers to objects on the heap.
-:- type mlds_maybe_gc_trace_code == maybe(statement).
+ % 'yes' indicates that GC tracing code is required for the
+ % variable.
+ % 'init' indicates that the variable is a compiler-generated
+ % variable that needs to be initialised before traversing the
+ % other variables for the predicate (used when generating
+ % closures).
+:- type mlds_maybe_gc_trace_code
+ ---> yes(statement)
+ ; init(statement)
+ ; no.
% It is possible for the function to be defined externally
% (i.e. the original Mercury procedure was declared `:- external').
Index: compiler/mlds_to_c.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/mlds_to_c.m,v
retrieving revision 1.197
diff -u -r1.197 mlds_to_c.m
--- compiler/mlds_to_c.m 22 Aug 2006 05:03:57 -0000 1.197
+++ compiler/mlds_to_c.m 11 Dec 2006 23:51:20 -0000
@@ -1376,7 +1376,7 @@
).
:- pred mlds_output_maybe_gc_trace_code(indent::in,
- mlds_qualified_entity_name::in, maybe(statement)::in,
+ mlds_qualified_entity_name::in, mlds_maybe_gc_trace_code::in,
string::in, io::di, io::uo) is det.
mlds_output_maybe_gc_trace_code(Indent, Name, Maybe_GC_TraceCode, MaybeNewLine,
@@ -1384,7 +1384,11 @@
(
Maybe_GC_TraceCode = no
;
- Maybe_GC_TraceCode = yes(GC_TraceCode),
+ (
+ Maybe_GC_TraceCode = yes(GC_TraceCode)
+ ;
+ Maybe_GC_TraceCode = init(GC_TraceCode)
+ ),
io.write_string(MaybeNewLine, !IO),
io.write_string("#if 0 /* GC trace code */\n", !IO),
% XXX This value for FuncInfo is bogus. However, this output is only
Index: compiler/mlds_to_il.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/mlds_to_il.m,v
retrieving revision 1.169
diff -u -r1.169 mlds_to_il.m
--- compiler/mlds_to_il.m 22 Aug 2006 05:03:58 -0000 1.169
+++ compiler/mlds_to_il.m 11 Dec 2006 23:51:25 -0000
@@ -428,7 +428,7 @@
(
Entity0 = mlds_data(Type, Initializer, GC_TraceCode),
Entity = mlds_data(Type, rename_initializer(Initializer),
- rename_maybe_statement(GC_TraceCode))
+ rename_maybe_gc_trace_code(GC_TraceCode))
;
Entity0 = mlds_function(MaybePredProcId, Params, FunctionBody0,
Attributes, EnvVarNames),
@@ -455,6 +455,13 @@
rename_maybe_statement(no) = no.
rename_maybe_statement(yes(Stmt)) = yes(rename_statement(Stmt)).
+:- func rename_maybe_gc_trace_code(mlds_maybe_gc_trace_code)
+ = mlds_maybe_gc_trace_code.
+
+rename_maybe_gc_trace_code(no) = no.
+rename_maybe_gc_trace_code(yes(Stmt)) = yes(rename_statement(Stmt)).
+rename_maybe_gc_trace_code(init(Stmt)) = init(rename_statement(Stmt)).
+
:- func rename_statement(statement) = statement.
rename_statement(statement(block(Defns, Stmts), Context))
----- End forwarded message -----
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to: mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions: mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------
More information about the reviews
mailing list