[m-rev.] for preliminary review: bug 493 & 532 alterative fix

Peter Wang novalazy at gmail.com
Thu May 13 17:10:58 AEST 2021


Hi Zoltan,

My idea in bug 532 was to:

 1. allow common.m to optimize construction unifications
    marked "static" in both LLDS and MLDS backends

 2. if any static constructions were optimised away then,
    after simplification, fix up any 'construct_statically'
    annotations on any remaining constructions in the procedure

 3. modify mark_static_terms.m to do the fixing up

I implemented that, but then I discovered that
mercury_compile_mlds_back_end.m will alway run the mark_static_terms
pass anyway (except when --no-static-ground-terms is used),
so there is no need to do it straight after simplification.

I think this diff is complete except for updating the (very extensive)
comments. I will do that if you think this is the right way to go.

Peter


diff --git a/compiler/common.m b/compiler/common.m
index a23bb8461..d4494f7b7 100644
--- a/compiler/common.m
+++ b/compiler/common.m
@@ -342,6 +342,8 @@ common_optimise_unification(Unification0, UnifyMode, !GoalExpr, !GoalInfo,
             UnifyMode = unify_modes_li_lf_ri_rf(_, LVarFinalInst, _, _),
             inst_is_ground(ModuleInfo, LVarFinalInst),
 
+            % XXX update this comment
+            %
             % The third test, applied specifically to the MLDS backend,
             % is that mark_static_terms.m should not have already decided
             % that we construct Var statically. This is because if it has,
@@ -401,9 +403,7 @@ common_optimise_unification(Unification0, UnifyMode, !GoalExpr, !GoalInfo,
             (
                 How = construct_dynamically
             ;
-                How = construct_statically(_),
-                simplify_info_get_ignore_marked_static(!.Info,
-                    ignore_marked_static)
+                How = construct_statically(_)
             )
         then
             common_optimise_construct(Var, ConsId, ArgVars, LVarFinalInst,
diff --git a/compiler/hlds_goal.m b/compiler/hlds_goal.m
index 9a7586a8c..5b3091221 100644
--- a/compiler/hlds_goal.m
+++ b/compiler/hlds_goal.m
@@ -1064,7 +1064,9 @@
             % unification is set to construct_dynamically(marked_static),
             % that information remains valid *only* until the next compiler
             % pass modifies any of the information that mark_static_terms.m
-            % used to arrive at that decision. When targeting the MLDS backend,
+            % used to arrive at that decision.
+            % XXX update this comment
+            % When targeting the MLDS backend,
             % the simplification pass, and common.m in particular, carefully
             % refrain from making invalidating changes. When targeting the LLDS
             % backend, they can and sometimes do make such invalidating
diff --git a/compiler/mark_static_terms.m b/compiler/mark_static_terms.m
index ed2a8601b..f7e7402af 100644
--- a/compiler/mark_static_terms.m
+++ b/compiler/mark_static_terms.m
@@ -204,8 +204,28 @@ unification_mark_static_terms(Unification0, Unification, !StaticVars) :-
                     HowToConstruct, Unique, SubInfo)
             )
         else
+            (
+                HowToConstruct0 = construct_statically(StaticHow0),
+                (
+                    StaticHow0 = born_static,
+                    unexpected($pred, "born_static has non-static args")
+                ;
+                    StaticHow0 = marked_static,
+                    % The variable being constructed was determined to be
+                    % static in a previous pass, but at least one argument is
+                    % not static any more, so the variable must be constructed
+                    % dynamically.
+                    Unification = construct(Var, ConsId, ArgVars, ArgModes,
+                        construct_dynamically, Unique, SubInfo)
+                )
+            ;
+                ( HowToConstruct0 = construct_dynamically
+                ; HowToConstruct0 = construct_in_region(_)
+                ; HowToConstruct0 = reuse_cell(_)
+                ),
                 Unification = Unification0
             )
+        )
     ;
         Unification0 = deconstruct(_Var, _ConsId, _ArgVars, _UniModes,
             _CanFail, _CanCGC),
diff --git a/compiler/mercury_compile_mlds_back_end.m b/compiler/mercury_compile_mlds_back_end.m
index d63e30748..ee32df49a 100644
--- a/compiler/mercury_compile_mlds_back_end.m
+++ b/compiler/mercury_compile_mlds_back_end.m
@@ -100,6 +100,12 @@ mlds_backend(!HLDS, !:MLDS, !:Specs, !DumpInfo, !IO) :-
     maybe_add_heap_ops(Verbose, Stats, !HLDS, !IO),
     maybe_dump_hlds(!.HLDS, 415, "add_heap_ops", !DumpInfo, !IO),
 
+    % The mark_static_terms pass may already have been run before the loop
+    % invariant hoisting pass, but it is necessary to run it again in case
+    % any code changes have invalidated the construct_statically annotations.
+    % In particular, common.m may replace a "static" construction of a variable
+    % with an assignment, causing another "static" construction to have a
+    % non-static variable as an argument.
     maybe_mark_static_terms(Verbose, Stats, !HLDS, !IO),
     maybe_dump_hlds(!.HLDS, 420, "mark_static", !DumpInfo, !IO),
 
diff --git a/compiler/simplify_info.m b/compiler/simplify_info.m
index 4a037bd68..ec1dd567b 100644
--- a/compiler/simplify_info.m
+++ b/compiler/simplify_info.m
@@ -142,10 +142,6 @@
     --->    not_trace_optimized
     ;       trace_optimized.
 
-:- type maybe_ignore_marked_static
-    --->    do_not_ignore_marked_static
-    ;       ignore_marked_static.
-
 :- pred simplify_info_get_simplify_tasks(simplify_info::in,
     simplify_tasks::out) is det.
 :- pred simplify_info_get_module_info(simplify_info::in, module_info::out)
@@ -165,8 +161,6 @@
     maybe_fully_strict::out) is det.
 :- pred simplify_info_get_trace_level_optimized(simplify_info::in,
     trace_level::out, maybe_trace_optimized::out) is det.
-:- pred simplify_info_get_ignore_marked_static(simplify_info::in,
-    maybe_ignore_marked_static::out) is det.
 
 :- pred simplify_info_get_rtti_varmaps(simplify_info::in, rtti_varmaps::out)
     is det.
@@ -315,9 +309,7 @@
                 sip_trace_level             :: trace_level,
 
                 % The value of the --trace-optimized option.
-                sip_trace_optimized         :: maybe_trace_optimized,
-
-                sip_ignore_marked_static    :: maybe_ignore_marked_static
+                sip_trace_optimized         :: maybe_trace_optimized
             ).
 
 :- type simplify_sub_info
@@ -378,17 +370,9 @@ simplify_info_init(ModuleInfo, PredId, ProcId, ProcInfo, SimplifyTasks,
     ( TraceOptimized0 = no,  TraceOptimized = not_trace_optimized
     ; TraceOptimized0 = yes, TraceOptimized = trace_optimized
     ),
-    Backend = lookup_current_backend(Globals),
-    (
-        Backend = low_level_backend,
-        IgnoreMarkedStatic = ignore_marked_static
-    ;
-        Backend = high_level_backend,
-        IgnoreMarkedStatic = do_not_ignore_marked_static
-    ),
 
     Params = simplify_info_params(PredProcId, InstVarSet, FullyStrict,
-        TraceLevel, TraceOptimized, IgnoreMarkedStatic),
+        TraceLevel, TraceOptimized),
 
     proc_info_get_rtti_varmaps(ProcInfo, RttiVarMaps),
     ElimVars = [],
@@ -492,8 +476,6 @@ simplify_info_get_fully_strict(Info, X) :-
 simplify_info_get_trace_level_optimized(Info, X, Y) :-
     X = Info ^ simp_params ^ sip_trace_level,
     Y = Info ^ simp_params ^ sip_trace_optimized.
-simplify_info_get_ignore_marked_static(Info, X) :-
-    X = Info ^ simp_params ^ sip_ignore_marked_static.
 
 simplify_info_get_rtti_varmaps(Info, X) :-
     X = Info ^ simp_sub_info ^ ssimp_rtti_varmaps.


More information about the reviews mailing list