[m-rev.] diff: fix structure sharing bugs
Peter Wang
novalazy at gmail.com
Tue Feb 19 12:07:41 AEDT 2008
The compiler bootstraps with --ctgc --intermodule-optimisation. Well,
two modules in library (bag.m, multi_map.m) may still need inlining
disabled (an existing problem). Benchmarks later.
Branches: main
Fix some problems with the structure sharing analysis.
compiler/ctgc.selector.m:
Remove an "potential bug" comment. I have verified that the code was
correct for one case and added an assertion for the other case which
should never occur.
Change the handling of existentially typed functors when normalising a
selector. We stopped normalisation when we got to a unit selector that
selects an existentially typed functors, but we kept any selectors on
the path following that unit selector. That causes compiler aborts
later. The new behaviour is to discard any later selectors on the
path.
compiler/ctgc.util.m:
Add a predicate `type_needs_sharing_analysis'. We were trying to cover
that concept with `type_is_reusable' as well, which is wrong, e.g. for
no-tag type and type variables.
Replace `var_has_non_reusable_type' by `var_needs_sharing_analysis'.
Fix `type_is_reusable' for dummy and no-tag types.
compiler/structure_sharing.domain.m:
Replace calls to `var_has_non_reusable_type' by
`var_needs_sharing_analysis'.
Make `selector_sharing_set_rename' handle type substitutions which map
two different type variables to the same type, instead of aborting.
Index: compiler/ctgc.selector.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/ctgc.selector.m,v
retrieving revision 1.9
diff -u -r1.9 ctgc.selector.m
--- compiler/ctgc.selector.m 11 Feb 2008 21:25:52 -0000 1.9
+++ compiler/ctgc.selector.m 19 Feb 2008 00:43:56 -0000
@@ -309,9 +309,18 @@
(
!.Selector = [UnitSelector | SelRest],
CtorCat = classify_type(ModuleInfo, VarType),
- % XXX This test seems to be a bug: it shouldn't succeed for either
- % notag types or dummy types.
- ( CtorCat = ctor_cat_user(_) ->
+ ( CtorCat = ctor_cat_user(CatUser) ->
+ (
+ CatUser = cat_user_general
+ ;
+ CatUser = cat_user_notag
+ ;
+ CatUser = cat_user_direct_dummy,
+ % We should not be producing selectors for dummy types.
+ unexpected(this_file,
+ "do_normalize_selector: cat_user_direct_dummy")
+ ),
+
% If it is either a term-selector of a non-existentially typed
% functor or is a type-selector, construct the branch map and
% proceed with normalization. If it is a term-selector of an
@@ -346,7 +355,9 @@
)
;
% Existentially typed functor.
- append(SelectorAcc0, !Selector)
+ % XXX awaiting confirmation on this from Nancy but it seems
+ % right to me --pw
+ append(SelectorAcc0, [UnitSelector], !:Selector)
)
;
% If it is not a user type, SelRest is empty anyhow, and
Index: compiler/ctgc.util.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/ctgc.util.m,v
retrieving revision 1.17
diff -u -r1.17 ctgc.util.m
--- compiler/ctgc.util.m 11 Feb 2008 21:25:52 -0000 1.17
+++ compiler/ctgc.util.m 19 Feb 2008 00:43:56 -0000
@@ -52,11 +52,12 @@
:- func get_type_substitution(module_info, pred_proc_id, list(mer_type),
tvarset, head_type_params) = tsubst.
- % var_has_non_reusable_type(ModuleInfo, ProcInfo, Var).
+ % var_needs_sharing_analysis(ModuleInfo, ProcInfo, Var).
%
- % Succeed iff Var is of a type for which we don't support structure reuse.
+ % Succeed iff Var is of a type for which we need to consider structure
+ % sharing.
%
-:- pred var_has_non_reusable_type(module_info::in, proc_info::in,
+:- pred var_needs_sharing_analysis(module_info::in, proc_info::in,
prog_var::in) is semidet.
% Succeed iff type is one for which we support structure reuse.
@@ -175,10 +176,38 @@
%-----------------------------------------------------------------------------%
-var_has_non_reusable_type(ModuleInfo, ProcInfo, Var):-
+var_needs_sharing_analysis(ModuleInfo, ProcInfo, Var) :-
proc_info_get_vartypes(ProcInfo, VarTypes),
map.lookup(VarTypes, Var, Type),
- not type_is_reusable(ModuleInfo, Type).
+ type_needs_sharing_analysis(ModuleInfo, Type).
+
+:- pred type_needs_sharing_analysis(module_info::in, mer_type::in) is semidet.
+
+type_needs_sharing_analysis(ModuleInfo, Type) :-
+ TypeCat = classify_type(ModuleInfo, Type),
+ type_category_needs_sharing_analysis(TypeCat) = yes.
+
+:- func type_category_needs_sharing_analysis(type_ctor_category) = bool.
+
+type_category_needs_sharing_analysis(CtorCat) = NeedsSharingAnalysis :-
+ (
+ ( CtorCat = ctor_cat_builtin(_)
+ ; CtorCat = ctor_cat_higher_order
+ ; CtorCat = ctor_cat_enum(_)
+ ; CtorCat = ctor_cat_builtin_dummy
+ ; CtorCat = ctor_cat_void
+ ; CtorCat = ctor_cat_system(_)
+ ),
+ NeedsSharingAnalysis = no
+ ;
+ ( CtorCat = ctor_cat_variable
+ ; CtorCat = ctor_cat_tuple
+ ; CtorCat = ctor_cat_user(_)
+ ),
+ NeedsSharingAnalysis = yes
+ ).
+
+%-----------------------------------------------------------------------------%
type_is_reusable(ModuleInfo, Type) :-
TypeCat = classify_type(ModuleInfo, Type),
@@ -195,12 +224,13 @@
; CtorCat = ctor_cat_variable
; CtorCat = ctor_cat_void
; CtorCat = ctor_cat_system(_)
+ ; CtorCat = ctor_cat_user(cat_user_direct_dummy)
+ ; CtorCat = ctor_cat_user(cat_user_notag)
),
Reusable = no
;
- % XXX I don't think notag user types should be reusable.
( CtorCat = ctor_cat_tuple
- ; CtorCat = ctor_cat_user(_)
+ ; CtorCat = ctor_cat_user(cat_user_general)
),
Reusable = yes
).
Index: compiler/structure_sharing.domain.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/structure_sharing.domain.m,v
retrieving revision 1.26
diff -u -r1.26 structure_sharing.domain.m
--- compiler/structure_sharing.domain.m 29 Jan 2008 03:03:34 -0000 1.26
+++ compiler/structure_sharing.domain.m 19 Feb 2008 00:44:00 -0000
@@ -445,9 +445,7 @@
= Sharing :-
(
Unification = construct(Var, ConsId, Args0, _, _, _, _),
- ( var_has_non_reusable_type(ModuleInfo, ProcInfo, Var) ->
- Sharing = sharing_as_init
- ;
+ ( var_needs_sharing_analysis(ModuleInfo, ProcInfo, Var) ->
list.takewhile(is_introduced_typeinfo_arg(ProcInfo), Args0,
_TypeInfoArgs, Args),
number_args(Args, NumberedArgs),
@@ -460,6 +458,8 @@
NumberedArgs, !SharingSet),
Sharing = wrap(!.SharingSet)
)
+ ;
+ Sharing = sharing_as_init
)
;
Unification = deconstruct(Var, ConsId, Args0, _, _, _),
@@ -475,13 +475,13 @@
)
;
Unification = assign(X, Y),
- ( var_has_non_reusable_type(ModuleInfo, ProcInfo, X) ->
- Sharing = sharing_as_init
- ;
+ ( var_needs_sharing_analysis(ModuleInfo, ProcInfo, X) ->
new_entry(ModuleInfo, ProcInfo,
datastruct_init(X) - datastruct_init(Y),
sharing_set_init, SharingSet),
Sharing = wrap(SharingSet)
+ ;
+ Sharing = sharing_as_init
)
;
Unification = simple_test(_, _),
@@ -512,12 +512,12 @@
sharing_set::in, sharing_set::out) is det.
add_var_arg_sharing(ModuleInfo, ProcInfo, Var, ConsId, N - Arg, !Sharing) :-
- ( var_has_non_reusable_type(ModuleInfo, ProcInfo, Arg) ->
- true
- ;
+ ( var_needs_sharing_analysis(ModuleInfo, ProcInfo, Arg) ->
Data1 = datastruct_init_with_pos(Var, ConsId, N),
Data2 = datastruct_init(Arg),
new_entry(ModuleInfo, ProcInfo, Data1 - Data2, !Sharing)
+ ;
+ true
).
% When two positions within the constructed term refer to the same variable,
@@ -1676,7 +1676,14 @@
selector_sharing_set_rename_2(Dict, Subst, Selector0, DataSet0, !Map) :-
rename_selector(Subst, Selector0, Selector),
data_set_rename(Dict, Subst, DataSet0, DataSet),
- svmap.det_insert(Selector, DataSet, !Map).
+ ( map.search(!.Map, Selector, DataSetOld) ->
+ % This can happen if Subst maps two different type variables to the
+ % same type.
+ data_set_add(DataSet, DataSetOld, CombinedDataSet),
+ svmap.set(Selector, CombinedDataSet, !Map)
+ ;
+ svmap.det_insert(Selector, DataSet, !Map)
+ ).
selector_sharing_set_add(SelectorSetA, SelectorSetB, SelectorSet):-
SelectorSetA = selector_sharing_set(_, MapA),
--------------------------------------------------------------------------
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