[m-rev.] for review: explicitly free dead cells

[Peter Wang]

On 2008-08-25, Peter Wang <novalazy at gmail.com> wrote:
> On 2008-08-25, Zoltan Somogyi <zs at csse.unimelb.edu.au> wrote:
> > On 21-Aug-2008, Peter Wang <novalazy at gmail.com> wrote:
> > > Zoltan, do you think it will be hard to solve the problem noted in
> > > unify_gen.m?
> > 
> > What problem noted in unify_gen.m?
> 
> Nevermind.  I thought there was a difficulty in freeing a dead cell,
> because some of its fields might not actually be extracted yet.  But of
> course the same problem would occur if we were to reuse the dead cell,
> so I can just reuse the code that handles that.

I've updated the patch so explicit frees work in LLDS grades as well.
Interdiff follows.  Post-commit review welcome.


Branches: main

Add the ability, when structure reuse is enabled, to free dead heap cells which
can't otherwise be reused.

Fix a bug where we might try to reuse static data in MLDS grades.  As with the
the LLDS grades, check at run-time that the cell to reuse is within the heap
range.

compiler/options.m:
	Add `--structure-reuse-free-cells' option (disabled by default).

compiler/structure_reuse.direct.choose_reuse.m:
	Fix a bug which caused the code to mark deconstructions with `can_cgc'
	to never be run.  Run the code if `--structure-reuse-free-cells'
	is enabled.

compiler/unify_gen.m:
	Generate `free_heap' instructions after deconstructions marked with
	`can_cgc'.  As in the case of cell reuse, we need to ensure that all
	the needed fields are available from other locations before freeing the
	cell.

compiler/code_info.m:
compiler/var_locn.m:
	Export the code which saves cell fields before the cell is reused
	or freed.

compiler/ml_unify_gen.m:
	Make the code which implements cell reuse generate code like this:

	    MR_assign_if_in_heap(Lval, Rval);
	    if (Lval) {
		/* assign fields */
	    } else {
		Lval = MR_new_object(...);
	    }

	where `MR_assign_if_in_heap' assigns NULL to Lval if Rval doesn't look
	like a dynamically allocated address.

	Strip tag bits off the argument to `delete_object' statements.

compiler/mlds.m:
	Add an MLDS statement `assign_if_in_heap'.

	Change `delete_object' to take an rval argument instead of an lval,
	for stripping tag bits.

compiler/mlds_to_c.m:
	Generate output for `assign_if_in_heap' and `delete_object'.

compiler/ml_elim_nested.m:
compiler/ml_optimize.m:
compiler/mlds_to_gcc.m:
compiler/mlds_to_il.m:
compiler/mlds_to_java.m:
	Conform to MLDS statement changes.

runtime/mercury_heap.h:
	Make `MR_free_heap' macro test that its argument is the heap range
	before calling `GC_FREE', so we don't try to free static data.

	Add `MR_assign_if_in_heap'.

diff --git a/compiler/code_info.m b/compiler/code_info.m
index f9ef9be..ba0eeea 100644
--- a/compiler/code_info.m
+++ b/compiler/code_info.m
@@ -3617,6 +3617,9 @@ should_add_region_ops(CodeInfo, _GoalInfo) = AddRegionOps :-
     list(int)::in, string::in, may_use_atomic_alloc::in, code_tree::out,
     code_info::in, code_info::out) is det.
 
+:- pred save_reused_cell_fields(prog_var::in, lval::in, code_tree::out,
+    list(lval)::out, code_info::in, code_info::out) is det.
+
 :- pred place_var(prog_var::in, lval::in, code_tree::out,
     code_info::in, code_info::out) is det.
 
@@ -3768,6 +3771,13 @@ assign_cell_to_var(Var, ReserveWordAtStart, Ptag, MaybeRvals, HowToConstruct,
     set_static_cell_info(StaticCellInfo, !CI),
     set_var_locn_info(VarLocnInfo, !CI).
 
+save_reused_cell_fields(Var, Lval, Code, Regs, !CI) :-
+    get_var_locn_info(!.CI, VarLocnInfo0),
+    get_module_info(!.CI, ModuleInfo),
+    var_locn_save_cell_fields(ModuleInfo, Var, Lval, Code, Regs,
+        VarLocnInfo0, VarLocnInfo),
+    set_var_locn_info(VarLocnInfo, !CI).
+
 place_var(Var, Lval, Code, !CI) :-
     get_var_locn_info(!.CI, VarLocnInfo0),
     get_module_info(!.CI, ModuleInfo),

diff --git a/compiler/unify_gen.m b/compiler/unify_gen.m
index 518dcb1..8b833b6 100644
--- a/compiler/unify_gen.m
+++ b/compiler/unify_gen.m
@@ -143,20 +143,21 @@ generate_unification(CodeModel, Uni, GoalInfo, Code, !CI) :-
         ),
         (
             CanCGC = can_cgc,
-            % XXX we can't free the cell yet.
-            % Any live variables which are only reachable via Var (or its
-            % aliases?) must be produced beforehand.
-            %
-            % XXX avoid strip_tag when we know what tag it will have
-            % VarName = variable_name(!.CI, Var),
-            % FreeVar = node([
-            %     llds_instr(free_heap(unop(strip_tag, VarRval)),
-            %         "Free " ++ VarName)
-            % ]),
-            ProduceArgs = empty,
-            ProduceVar = empty,
-            FreeVar = empty,
-            Code = tree_list([Code0, ProduceArgs, ProduceVar, FreeVar])
+            VarName = variable_name(!.CI, Var),
+            produce_variable(Var, ProduceVar, VarRval, !CI),
+            ( VarRval = lval(VarLval) ->
+                save_reused_cell_fields(Var, VarLval, SaveArgs, Regs, !CI),
+                % This seems to be fine.
+                list.foldl(release_reg, Regs, !CI),
+                % XXX avoid strip_tag when we know what tag it will have
+                FreeVar = node([
+                    llds_instr(free_heap(unop(strip_tag, VarRval)),
+                        "Free " ++ VarName)
+                ]),
+                Code = tree_list([Code0, ProduceVar, SaveArgs, FreeVar])
+            ;
+                Code = Code0
+            )
         ;
             CanCGC = cannot_cgc,
             Code = Code0
diff --git a/compiler/var_locn.m b/compiler/var_locn.m
index 18f7696..3adbb13 100644
--- a/compiler/var_locn.m
+++ b/compiler/var_locn.m
@@ -187,6 +187,16 @@
     static_cell_info::in, static_cell_info::out,
     var_locn_info::in, var_locn_info::out) is det.
 
+    % var_locn_save_cell_fields(ModuleInfo, Var, VarLval, Code,
+    %   TempRegs, !VarLocnInfo)
+    %
+    % Save any variables which depend on the ReuseLval into temporary
+    % registers so that they are available after ReuseLval is clobbered.
+    %
+:- pred var_locn_save_cell_fields(module_info::in, prog_var::in, lval::in,
+    code_tree::out, list(lval)::out, var_locn_info::in, var_locn_info::out)
+    is det.
+
     % var_locn_place_var(ModuleInfo, Var, Lval, Code, !VarLocnInfo):
     %
     % Produces Code to place the value of Var in Lval, and update !VarLocnInfo
@@ -879,8 +889,8 @@ var_locn_assign_dynamic_cell_to_var(ModuleInfo, Var, ReserveWordAtStart, Ptag,
         MaybeOffset = no,
         StartOffset = 0
     ),
-    % This must appear before the call to `save_reused_cell_fields' in the
-    % reused_cell case, otherwise `save_reused_cell_fields' won't know not to
+    % This must appear before the call to `var_locn_save_cell_fields' in the
+    % reused_cell case, otherwise `var_locn_save_cell_fields' won't know not to
     % use Lval as a temporary register (if Lval is a register).
     var_locn_set_magic_var_location(Var, Lval, !VLI),
     (
@@ -953,7 +963,7 @@ assign_reused_cell_to_var(ModuleInfo, Lval, Ptag, Vector, CellToReuse,
 
     % Save any variables which are available only in the reused cell into
     % temporary registers.
-    save_reused_cell_fields(ModuleInfo, ReuseVar, ReuseLval, SaveArgsCode,
+    var_locn_save_cell_fields(ModuleInfo, ReuseVar, ReuseLval, SaveArgsCode,
         TempRegs0, !VLI),
     SetupReuseCode = tree(ReuseVarCode, SaveArgsCode),
 
@@ -1102,27 +1112,20 @@ assign_cell_arg(ModuleInfo, Rval0, Ptag, Base, Offset, Code, !VLI) :-
     ),
     Code = tree(EvalCode, AssignCode).
 
-    % Save any variables which depend on the ReuseLval into temporary
-    % registers so that they are available after ReuseLval is clobbered.
-    %
-:- pred save_reused_cell_fields(module_info::in, prog_var::in, lval::in,
-    code_tree::out, list(lval)::out, var_locn_info::in, var_locn_info::out)
-    is det.
-
-save_reused_cell_fields(ModuleInfo, ReuseVar, ReuseLval, Code, Regs, !VLI) :-
+var_locn_save_cell_fields(ModuleInfo, ReuseVar, ReuseLval, Code, Regs, !VLI) :-
     var_locn_get_var_state_map(!.VLI, VarStateMap),
     map.lookup(VarStateMap, ReuseVar, ReuseVarState0),
     DepVarsSet = ReuseVarState0 ^ using_vars,
     DepVars = set.to_sorted_list(DepVarsSet),
-    list.map_foldl2(save_reused_cell_fields_2(ModuleInfo, ReuseLval),
+    list.map_foldl2(var_locn_save_cell_fields_2(ModuleInfo, ReuseLval),
         DepVars, SaveArgsCode, [], Regs, !VLI),
     Code = tree_list(SaveArgsCode).
 
-:- pred save_reused_cell_fields_2(module_info::in, lval::in, prog_var::in,
+:- pred var_locn_save_cell_fields_2(module_info::in, lval::in, prog_var::in,
     code_tree::out, list(lval)::in, list(lval)::out,
     var_locn_info::in, var_locn_info::out) is det.
 
-save_reused_cell_fields_2(ModuleInfo, ReuseLval, DepVar, SaveDepVarCode,
+var_locn_save_cell_fields_2(ModuleInfo, ReuseLval, DepVar, SaveDepVarCode,
         !Regs, !VLI) :-
     find_var_availability(!.VLI, DepVar, no, Avail),
     (


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