[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