[m-rev.] diff: fix structure reuse problems
Peter Wang
novalazy at gmail.com
Tue May 3 14:36:46 AEST 2011
Branches: main
Fix some problems uncovered when trying to build the compiler with
--structure-reuse enabled, including bug #188.
compiler/ctgc.selector.m:
Prevent a compiler abort when compiling hash_table.m with structure
reuse enabled. The sharing annotation for array.unsafe_svset somehow
causes selector_subsumed_by_2 to try to retrieve the nth subtype of a
type variable. I believe the annotation is not to blame, by the test
case below.
compiler/structure_reuse.versions.m:
Don't try to reuse cells when constructing terms within
from_ground_term_construct scopes.
Add a sanity check to unification_set_reuse.
library/array.m:
Fix problematic sharing annotations.
tests/hard_coded/Mmakefile:
tests/hard_coded/reuse_array.exp:
tests/hard_coded/reuse_array.exp2:
tests/hard_coded/reuse_array.m:
Add test case for array.unsafe_svset sharing annotation.
diff --git a/compiler/ctgc.selector.m b/compiler/ctgc.selector.m
index 8b3bc5e..542d7ee 100644
--- a/compiler/ctgc.selector.m
+++ b/compiler/ctgc.selector.m
@@ -190,12 +190,21 @@ selector_subsumed_by_2(ModuleInfo, A, B, Type, Extension) :-
AH = termsel(ConsIdA, IndexA),
BH = termsel(ConsIdB, IndexB)
->
- % If both selectors begin with term selectors, clearly they must
- % agree on the node to select for the selectors to be comparable.
ConsIdA = ConsIdB,
IndexA = IndexB,
- SubType = det_select_subtype(ModuleInfo, Type, ConsIdA, IndexA),
- selector_subsumed_by_2(ModuleInfo, AT, BT, SubType, Extension)
+ (
+ Type = type_variable(_, _),
+ AT = BT
+ ->
+ % XXX Avoid an assertion failure when trying to index arguments
+ % of a type variable. This is probably a hack.
+ Extension = []
+ ;
+ % If both selectors begin with term selectors, clearly they must
+ % agree on the node to select for the selectors to be comparable.
+ SubType = det_select_subtype(ModuleInfo, Type, ConsIdA, IndexA),
+ selector_subsumed_by_2(ModuleInfo, AT, BT, SubType, Extension)
+ )
;
% If one selector has a term selector at the current position but
% the other has a type selector, we select the node dictated by the
diff --git a/compiler/structure_reuse.versions.m b/compiler/structure_reuse.versions.m
index 36dbb70..f321df3 100644
--- a/compiler/structure_reuse.versions.m
+++ b/compiler/structure_reuse.versions.m
@@ -415,12 +415,14 @@ process_goal(ConvertPotentialReuse, ReuseTable, ModuleInfo, !Goal) :-
GoalExpr0 = negation(_Goal)
;
GoalExpr0 = scope(Reason, SubGoal0),
- % XXX We should special-case the handling of from_ground_term_construct
- % scopes.
- process_goal(ConvertPotentialReuse, ReuseTable, ModuleInfo,
- SubGoal0, SubGoal),
- GoalExpr = scope(Reason, SubGoal),
- !:Goal = hlds_goal(GoalExpr, GoalInfo0)
+ ( Reason = from_ground_term(_, from_ground_term_construct) ->
+ true
+ ;
+ process_goal(ConvertPotentialReuse, ReuseTable, ModuleInfo,
+ SubGoal0, SubGoal),
+ GoalExpr = scope(Reason, SubGoal),
+ !:Goal = hlds_goal(GoalExpr, GoalInfo0)
+ )
;
GoalExpr0 = if_then_else(Vars, IfGoal0, ThenGoal0, ElseGoal0),
process_goal(ConvertPotentialReuse, ReuseTable, ModuleInfo,
@@ -445,12 +447,21 @@ process_goal(ConvertPotentialReuse, ReuseTable, ModuleInfo, !Goal) :-
unification_set_reuse(ShortReuseDescription, !Unification) :-
(
- !.Unification = construct(A, B, C, D, _HowToConstruct, F, G),
+ !.Unification = construct(A, B, C, D, HowToConstruct0, F, G),
ShortReuseDescription = cell_reused(DeadVar, _, PossibleConsIds,
CellsToUpdate)
->
- CellToReuse = cell_to_reuse(DeadVar, PossibleConsIds,
- CellsToUpdate),
+ (
+ HowToConstruct0 = construct_statically,
+ unexpected(this_file, "unification_set_reuse: construct_statically")
+ ;
+ HowToConstruct0 = construct_dynamically
+ ;
+ HowToConstruct0 = construct_in_region(_)
+ ;
+ HowToConstruct0 = reuse_cell(_)
+ ),
+ CellToReuse = cell_to_reuse(DeadVar, PossibleConsIds, CellsToUpdate),
HowToConstruct = reuse_cell(CellToReuse),
!:Unification = construct(A, B, C, D, HowToConstruct, F, G)
;
diff --git a/library/array.m b/library/array.m
index e309122..2140ac6 100644
--- a/library/array.m
+++ b/library/array.m
@@ -1281,7 +1281,7 @@ array.svset(Index, Item, Array0, Array) :-
array.unsafe_svset(Index::in, Item::in, Array0::array_di, Array::array_uo),
[will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
does_not_affect_liveness,
- sharing(yes(array(T), int, T, array(T)), [
+ sharing(yes(int, T, array(T), array(T)), [
cel(Array0, []) - cel(Array, []),
cel(Item, []) - cel(Array, [T])
])
@@ -1911,8 +1911,7 @@ array.append(A, B) = C :-
array.append(ArrayA::in, ArrayB::in) = (ArrayC::array_uo),
[will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
does_not_affect_liveness,
- % XXX zs: I am not sure about the sharing annotation.
- sharing(yes(array(T), array(T)), [
+ sharing(yes(array(T), array(T), array(T)), [
cel(ArrayA, [T]) - cel(ArrayC, [T]),
cel(ArrayB, [T]) - cel(ArrayC, [T])
])
diff --git a/tests/hard_coded/CVS/Entries b/tests/hard_coded/CVS/Entries
index 80a7c85..f54b025 100644
--- a/tests/hard_coded/CVS/Entries
+++ b/tests/hard_coded/CVS/Entries
@@ -873,4 +873,7 @@ D/typeclasses////
/utf8_io.m/1.1/Mon Apr 4 07:10:41 2011//
/words_separator.exp/1.1/Mon Apr 4 07:10:41 2011//
/words_separator.m/1.1/Mon Apr 4 07:10:41 2011//
-/Mmakefile/1.401/Fri Apr 29 00:04:20 2011//
+/Mmakefile/1.402/Tue May 3 04:32:35 2011//
+/reuse_array.exp/1.1/Tue May 3 04:31:19 2011//
+/reuse_array.exp2/1.1/Tue May 3 04:31:19 2011//
+/reuse_array.m/1.1/Tue May 3 04:31:19 2011//
diff --git a/tests/hard_coded/Mmakefile b/tests/hard_coded/Mmakefile
index 1a4daf6..6e0e604 100644
--- a/tests/hard_coded/Mmakefile
+++ b/tests/hard_coded/Mmakefile
@@ -414,6 +414,7 @@ ifeq "$(findstring debug,$(GRADE))" ""
bad_indirect_reuse2 \
bad_indirect_reuse2b \
bad_indirect_reuse3 \
+ reuse_array \
reuse_ho \
sharing_comb \
uncond_reuse \
diff --git a/tests/hard_coded/reuse_array.exp b/tests/hard_coded/reuse_array.exp
new file mode 100644
index 0000000..36c223b
--- /dev/null
+++ b/tests/hard_coded/reuse_array.exp
@@ -0,0 +1,2 @@
+[struct(1, 999), struct(1, 999), struct(1, 999), struct(3, 999), struct(1, 999)]
+4 distinct addresses - no reuse detected
diff --git a/tests/hard_coded/reuse_array.exp2 b/tests/hard_coded/reuse_array.exp2
new file mode 100644
index 0000000..4fb0d32
--- /dev/null
+++ b/tests/hard_coded/reuse_array.exp2
@@ -0,0 +1,2 @@
+[struct(1, 999), struct(1, 999), struct(1, 999), struct(3, 999), struct(1, 999)]
+3 distinct addresses - reuse detected
diff --git a/tests/hard_coded/reuse_array.m b/tests/hard_coded/reuse_array.m
new file mode 100644
index 0000000..c1ef606
--- /dev/null
+++ b/tests/hard_coded/reuse_array.m
@@ -0,0 +1,87 @@
+%-----------------------------------------------------------------------------%
+% Test structure sharing annotations on array operations.
+
+:- module reuse_array.
+:- interface.
+
+:- import_module io.
+
+:- impure pred main(io::di, io::uo) is cc_multi.
+
+%-----------------------------------------------------------------------------%
+%-----------------------------------------------------------------------------%
+
+:- implementation.
+
+:- import_module array.
+:- import_module list.
+
+:- type struct
+ ---> struct(int, f :: int).
+
+%-----------------------------------------------------------------------------%
+
+main(!IO) :-
+ Struct1 = struct(1, dummy),
+ impure addr(Struct1, Addr1),
+ array.init(5, Struct1, Array0),
+ % Structure reuse requires a deconstruction which it can consider the death
+ % of a structure, so we oblige.
+ unused(Struct1 ^ f),
+
+ Struct2 = struct(2, dummy),
+ impure addr(Struct2, Addr2),
+ unused(Struct2 ^ f),
+
+ % Struct1 should NOT be reused here, as it shares with Array0.
+ % Struct2 can and should be reused instead.
+ Struct3 = struct(3, dummy),
+ impure addr(Struct3, Addr3),
+ array.unsafe_set(Array0, 3, Struct3, Array),
+ unused(Struct3 ^ f),
+
+ % Struct3 should NOT be reused for Struct4, as it is shared with Array.
+ Struct4 = struct(4, dummy),
+ impure addr(Struct4, Addr4),
+
+ array.to_list(Array, List),
+ io.write(List, !IO),
+ io.nl(!IO),
+
+ list.sort_and_remove_dups([Addr1, Addr2, Addr3, Addr4], Addrs),
+ list.length(Addrs, NumAddrs),
+ (
+ NumAddrs = 4
+ ->
+ io.write_string("4 distinct addresses - no reuse detected\n", !IO)
+ ;
+ NumAddrs = 3,
+ Addr2 = Addr3
+ ->
+ io.write_string("3 distinct addresses - reuse detected\n", !IO)
+ ;
+ io.write_string("unexpected addresses: ", !IO),
+ io.write(Addrs, !IO),
+ io.nl(!IO)
+ ).
+
+:- impure pred addr(T::ui, int::uo) is cc_multi.
+:- pragma foreign_proc("C",
+ addr(T::ui, Ptr::uo),
+ [will_not_call_mercury, thread_safe, no_sharing],
+"
+ Ptr = (MR_Integer) T;
+").
+
+ % This is used to force structures to be constructed dynamically.
+:- func dummy = (int::uo).
+:- pragma no_inline(dummy/0).
+
+dummy = 999.
+
+:- pred unused(int::unused) is det.
+
+unused(_).
+
+%-----------------------------------------------------------------------------%
+% vim: ft=mercury ts=4 sts=4 sw=4 et
--------------------------------------------------------------------------
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