[m-rev.] for review: Fix some problems with static terms in LLDS grades.

Peter Wang novalazy at gmail.com
Mon Apr 30 09:52:29 AEST 2018


compiler/unify_gen.m:
    Add type_info, typeclass_info and ground term constants to the var
    state map using assign_const_to_var instead of assign_expr_to_var.
    expr_is_constant will fail if a constant expression is added with
    the more general assign_expr_to_var, causing missed opportunities
    to construct terms statically in var_locn.m.

    Prevents a compiler abort (bug #457) where mark_static_terms.m
    determines that a term can be constructed statically but var_locn.m
    decides otherwise.

compiler/var_locn.m:
    Do not abort if var_locn_assign_dynamic_cell_to_var is passed
    `construct_statically'. Even after the previous change, it is
    possible for mark_static_terms.m to determine that a cell with a
    dummy type argument can be constructed statically but var_locn.m
    currently does not. This was preventing bootchecks at
    -O5 --intermodule-optimization.

compiler/hlds_goal.m:
    Fix misleading documentation for how_to_construct.

tests/valid/Mercury.options:
tests/valid/bug457.m:
    Add a test case.
---
 compiler/hlds_goal.m        |  5 +++--
 compiler/unify_gen.m        |  3 ++-
 compiler/var_locn.m         | 15 ++++++++++++---
 tests/valid/Mercury.options |  1 +
 tests/valid/Mmakefile       |  1 +
 tests/valid/bug457.m        | 29 +++++++++++++++++++++++++++++
 6 files changed, 48 insertions(+), 6 deletions(-)
 create mode 100644 tests/valid/bug457.m

diff --git a/compiler/hlds_goal.m b/compiler/hlds_goal.m
index f9ab763ed..394217361 100644
--- a/compiler/hlds_goal.m
+++ b/compiler/hlds_goal.m
@@ -1014,8 +1014,9 @@
 
     % Information on how to construct the cell for a construction unification.
     % The `construct_statically' alternative is set by the mark_static_terms.m
-    % pass, and is currently only used for the MLDS back-end (for the LLDS
-    % back-end, the same optimization is handled by var_locn.m).
+    % pass, and is used for the MLDS back-end. For the LLDS back-end, the same
+    % optimization is handled by var_locn.m but mark_static_terms.m may be run
+    % to support the loop invariant hoisting pass.
     %
 :- type how_to_construct
     --->    construct_statically
diff --git a/compiler/unify_gen.m b/compiler/unify_gen.m
index d82dc4247..70fd8e138 100644
--- a/compiler/unify_gen.m
+++ b/compiler/unify_gen.m
@@ -647,7 +647,8 @@ generate_construction(LHSVar, ConsId, RHSVarsWidths, ArgModes,
         ),
         get_const_struct_map(!.CI, ConstStructMap),
         map.lookup(ConstStructMap, ConstNum, typed_rval(Rval, _Type)),
-        assign_expr_to_var(LHSVar, Rval, Code, !CLD)
+        assign_const_to_var(LHSVar, Rval, !.CI, !CLD),
+        Code = empty
     ;
         ConsTag = tabling_info_tag(PredId, ProcId),
         expect(unify(RHSVarsWidths, []), $pred,
diff --git a/compiler/var_locn.m b/compiler/var_locn.m
index 507b30cdd..54b662500 100644
--- a/compiler/var_locn.m
+++ b/compiler/var_locn.m
@@ -2,6 +2,7 @@
 % vim: ft=mercury ts=4 sw=4 et
 %---------------------------------------------------------------------------%
 % Copyright (C) 2000-2012 The University of Melbourne.
+% Copyright (C) 2013-2018 The Mercury team.
 % This file may only be copied under the terms of the GNU General
 % Public License - see the file COPYING in the Mercury distribution.
 %---------------------------------------------------------------------------%
@@ -844,14 +845,22 @@ var_locn_assign_dynamic_cell_to_var(ModuleInfo, Var, ReserveWordAtStart, Ptag,
             LldsComment = "Allocating heap for ",
             RegionVarCode = empty,
             MaybeRegionRval = no
+        ;
+            HowToConstruct = construct_statically,
+            % construct_statically is not normally used in LLDS grades, but
+            % it may be set by mark_static_terms.m to support loop invariant
+            % hoisting. If we get here then have we missed an opportunity to
+            % construct a cell statically?
+            % Currently this is reachable when cell_is_constant fails on a
+            % dummy type argument (cell_arg_skip_one_word).
+            LldsComment = "Allocating heap (unnecessarily?) for ",
+            RegionVarCode = empty,
+            MaybeRegionRval = no
         ),
         assign_all_cell_args(ModuleInfo, Vector, yes(Ptag), lval(Lval),
             StartOffset, ArgsCode, !VLI),
         SetupReuseCode = empty,
         MaybeReuse = no_llds_reuse
-    ;
-        HowToConstruct = construct_statically,
-        unexpected($pred, "construct_statically")
     ;
         HowToConstruct = reuse_cell(CellToReuse),
         CellToReuse = cell_to_reuse(ReuseVar, _ReuseConsId, _NeedsUpdates0),
diff --git a/tests/valid/Mercury.options b/tests/valid/Mercury.options
index 35c0290cc..06da03453 100644
--- a/tests/valid/Mercury.options
+++ b/tests/valid/Mercury.options
@@ -39,6 +39,7 @@ MCFLAGS-bug159			= -w
 MCFLAGS-bug180			= --profile-optimized --allow-stubs --no-warn-stubs
 MCFLAGS-bug271			= --allow-stubs --no-warn-stubs
 MCFLAGS-bug300			= --optimize-constructor-last-call
+MCFLAGS-bug457			= --loop-invariants --intermodule-optimization
 MCFLAGS-compl_unify_bug		= -O3
 MCFLAGS-constraint_prop_bug	= -O0 --common-struct --local-constraint-propagation
 MCFLAGS-csharp_hello		= --no-intermodule-optimization
diff --git a/tests/valid/Mmakefile b/tests/valid/Mmakefile
index 3c3d1aee8..2a232c6cd 100644
--- a/tests/valid/Mmakefile
+++ b/tests/valid/Mmakefile
@@ -84,6 +84,7 @@ OTHER_PROGS = \
 	bug402 \
 	bug414 \
 	bug429 \
+	bug457 \
 	bug51 \
 	bug85 \
 	builtin_false \
diff --git a/tests/valid/bug457.m b/tests/valid/bug457.m
new file mode 100644
index 000000000..d937e6554
--- /dev/null
+++ b/tests/valid/bug457.m
@@ -0,0 +1,29 @@
+%---------------------------------------------------------------------------%
+% vim: ts=4 sw=4 et ft=mercury
+%---------------------------------------------------------------------------%
+%
+% A type_info cell was not constructed statically by var_locn.m.
+% It caused a compiler abort because that differed from the
+% construct_statically hint set by mark_static_terms.m.
+%
+%---------------------------------------------------------------------------%
+
+:- module bug457.
+:- interface.
+
+:- import_module io.
+
+:- pred wrapper(pred(io, io), io, io).
+:- mode wrapper(in(pred(di, uo) is cc_multi), di, uo) is cc_multi.
+
+:- implementation.
+
+:- import_module exception.
+
+wrapper(Pred, !IO) :-
+    Closure = (pred({}::out, IO0::di, IO::uo) is cc_multi :-
+        Pred(IO0, IO)
+    ),
+    try_io(Closure, _TryResult, !IO).
+
+%---------------------------------------------------------------------------%
-- 
2.17.0



More information about the reviews mailing list