[m-rev.] for review: fix bug with polymorphic modes and intermodule optimization
Julien Fischer
juliensf at cs.mu.OZ.AU
Mon Nov 7 19:44:36 AEDT 2005
On Mon, 7 Nov 2005, Mark Brown wrote:
> On 07-Nov-2005, Julien Fischer <juliensf at cs.mu.OZ.AU> wrote:
> > Estimated hours taken: 16
> > Branches: main
> >
> > Fix the problem that has been causing hard_coded/intermod_poly_mode to fail.
> >
> > When writing foreign_procs to .opt files, make sure that any inst variables
> > have the correct name.
>
> On a related note, the reference manual doesn't appear to say anything about
> the scope of inst variables. Does the scope extend to foreign_proc decls
> (like how the scope of type variables extends to clauses), or can we use
> different names in the foreign_proc decl? If the former is the case, then
> you should also check that the names in the foreign_proc match those in the
> selected mode.
>
I would argue that the scope of inst variables should extend to cover
foreign_procs, although the compiler currently doesn't enforce this
(and I'm not planning on fixing it as part of this change).
> >
> > compiler/add_pragma.m:
> > When searching for the predmode decl corresponding to a foreign_proc
> > allow for a renaming between inst variables. If we don't do this,
> > then the compiler cannot find the corresponding predmode declaration
> > for opt_imported foreign_procs that have polymorphically moded
> > arguments. (The reason this is occurring is a bit obscure, it it
>
> s/it it/it/
>
Fixed.
> > looks like the inst_vars in the predmode decl and those in the
> > foreign_proc are being allocated from different inst_varsets. Tracing
> > this through in the debugger indicates that this happening quite early
> > on, it may be that we've never made the necessary connection between
> > the sets of inst variables and we've never used polymorphically moded
> > procedures extensively enough to notice.)
>
> See my comment below, in mercury_to_mercury.m.
>
> > Index: compiler/add_pragma.m
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/compiler/add_pragma.m,v
> > retrieving revision 1.16
> > diff -u -r1.16 add_pragma.m
> > --- compiler/add_pragma.m 4 Nov 2005 03:40:42 -0000 1.16
> > +++ compiler/add_pragma.m 7 Nov 2005 04:03:27 -0000
> > @@ -1372,8 +1372,14 @@
> > map__to_assoc_list(Procs, ExistingProcs),
> > pragma_get_modes(PVars, Modes),
> > (
> > - get_procedure_matching_argmodes(ExistingProcs, Modes,
> > - !.ModuleInfo, ProcId)
> > + % XXX We need to allow for a renaming between inst variables
> > + % here, otherwise we won't find a match for opt_imported
> > + % foreign_procs that have polymorphically moded arguments.
> > + % This occurs because the inst vars in the foreign_proc
> > + % and the corresponding mode declaration seem to be allocated
> > + % from different inst_varsets.
> > + get_procedure_matching_argmodes_with_renaming(ExistingProcs,
> > + Modes, !.ModuleInfo, ProcId)
>
> Why is this an XXX? The variables come from different varsets, so it is only
> natural that we should have to bind the variables in one context to those in
> the other.
>
At the time I wrote that I wasn't sure what was going on. It's
now not an XXX.
> > Index: compiler/mercury_to_mercury.m
> > ===================================================================
> > RCS file: /home/mercury1/repository/mercury/compiler/mercury_to_mercury.m,v
> > retrieving revision 1.272
> > diff -u -r1.272 mercury_to_mercury.m
> > --- compiler/mercury_to_mercury.m 28 Oct 2005 02:10:18 -0000 1.272
> > +++ compiler/mercury_to_mercury.m 7 Nov 2005 04:02:15 -0000
> > @@ -558,8 +560,10 @@
> > ;
> > Pragma = foreign_proc(Attributes, Pred, PredOrFunc, Vars, VarSet,
> > PragmaCode),
> > + % The inst_varset isn't available to us here.
> > + InstVarset = varset.init,
>
> The varset in the foreign_proc pragma_type, although it is a prog_varset,
> appears to contain the names of the inst variables. When parsing the
> foreign_proc declaration we just cast the varset to a prog_varset and that's
> it. Instead, we should handle the varset similarly to the processing of
> pred_mode declarations, which also contain two sorts of variables (in this
> case, type variables and inst variables). This will require an extra field
> to be added to the foreign_proc functor.
Done.
> Once you have made that change it should be fine to commit this, but you
> should still post the relative diff.
>
I'll commit it after bootchecking it again. See below for the relative
diff.
Cheers,
Julien.
diff -u compiler/add_pragma.m compiler/add_pragma.m
--- compiler/add_pragma.m 7 Nov 2005 04:03:27 -0000
+++ compiler/add_pragma.m 7 Nov 2005 08:34:28 -0000
@@ -70,9 +70,9 @@
:- pred module_add_pragma_foreign_proc(pragma_foreign_proc_attributes::in,
sym_name::in, pred_or_func::in, list(pragma_var)::in, prog_varset::in,
- pragma_foreign_code_impl::in, import_status::in, prog_context::in,
- module_info::in, module_info::out, qual_info::in, qual_info::out,
- io::di, io::uo) is det.
+ inst_varset::in, pragma_foreign_code_impl::in, import_status::in,
+ prog_context::in, module_info::in, module_info::out,
+ qual_info::in, qual_info::out, io::di, io::uo) is det.
:- pred module_add_pragma_tabled(eval_method::in, sym_name::in, int::in,
maybe(pred_or_func)::in, maybe(list(mer_mode))::in, import_status::in,
@@ -188,7 +188,7 @@
module_add_foreign_import_module(Lang, Import, Context, !ModuleInfo)
;
% Handle pragma foreign procs later on (when we process clauses).
- Pragma = foreign_proc(_, _, _, _, _, _)
+ Pragma = foreign_proc(_, _, _, _, _, _, _)
;
% Handle pragma tabled decls later on (when we process clauses).
Pragma = tabled(_, _, _, _, _)
@@ -1253,7 +1253,8 @@
%-----------------------------------------------------------------------------%
module_add_pragma_foreign_proc(Attributes0, PredName, PredOrFunc, PVars,
- VarSet, PragmaImpl, Status, Context, !ModuleInfo, !QualInfo, !IO) :-
+ ProgVarSet, _InstVarset, PragmaImpl, Status, Context, !ModuleInfo,
+ !QualInfo, !IO) :-
%
% Begin by replacing any maybe_thread_safe foreign_proc attributes
% with the actual thread safety attributes which we get from the
@@ -1372,12 +1373,16 @@
map__to_assoc_list(Procs, ExistingProcs),
pragma_get_modes(PVars, Modes),
(
- % XXX We need to allow for a renaming between inst variables
- % here, otherwise we won't find a match for opt_imported
- % foreign_procs that have polymorphically moded arguments.
- % This occurs because the inst vars in the foreign_proc
- % and the corresponding mode declaration seem to be allocated
- % from different inst_varsets.
+ % The inst variables for the foreign_proc declaration
+ % and predmode declarations are from different varsets.
+ % We cannot unify the argument modes directly because
+ % the representation of the inst variables may be different.
+ % Instead we need to allow for a renaming between the
+ % inst variables in the argument modes of the foreign_proc
+ % and those of the predmode declaration.
+ %
+ % XXX We should probably also check that each pair in
+ % the renaming has the same name.
get_procedure_matching_argmodes_with_renaming(ExistingProcs,
Modes, !.ModuleInfo, ProcId)
->
@@ -1385,7 +1390,7 @@
pred_info_arg_types(!.PredInfo, ArgTypes),
pred_info_get_purity(!.PredInfo, Purity),
clauses_info_add_pragma_foreign_proc(Purity, Attributes,
- PredId, ProcId, VarSet, PVars, ArgTypes, PragmaImpl,
+ PredId, ProcId, ProgVarSet, PVars, ArgTypes, PragmaImpl,
Context, PredOrFunc, PredName, Arity, Clauses0, Clauses,
!ModuleInfo, !IO),
pred_info_set_clauses_info(Clauses, !PredInfo),
@@ -1838,10 +1843,11 @@
PredOrFunc, Arity, ArgTypes, Status, Context, !ModuleInfo, !QualInfo,
!IO) :-
map__lookup(ProcTable, ProcID, ProcInfo),
- varset__init(VarSet0),
- varset__new_vars(VarSet0, Arity, Vars, VarSet),
+ varset__init(ProgVarSet0),
+ varset__new_vars(ProgVarSet0, Arity, Vars, ProgVarSet),
proc_info_argmodes(ProcInfo, Modes),
- fact_table_pragma_vars(Vars, Modes, VarSet, PragmaVars),
+ proc_info_inst_varset(ProcInfo, InstVarSet),
+ fact_table_pragma_vars(Vars, Modes, ProgVarSet, PragmaVars),
fact_table_generate_c_code(SymName, PragmaVars, ProcID, PrimaryProcID,
ProcInfo, ArgTypes, !.ModuleInfo, C_ProcCode, C_ExtraCode, !IO),
@@ -1852,7 +1858,7 @@
% Fact tables procedures should be considered pure.
set_purity(purity_pure, Attrs2, Attrs),
module_add_pragma_foreign_proc(Attrs, SymName, PredOrFunc, PragmaVars,
- VarSet, ordinary(C_ProcCode, no), Status, Context,
+ ProgVarSet, InstVarSet, ordinary(C_ProcCode, no), Status, Context,
!ModuleInfo, !QualInfo, !IO),
( C_ExtraCode = "" ->
true
diff -u compiler/mercury_to_mercury.m compiler/mercury_to_mercury.m
--- compiler/mercury_to_mercury.m 7 Nov 2005 06:04:08 -0000
+++ compiler/mercury_to_mercury.m 7 Nov 2005 07:41:46 -0000
@@ -558,10 +558,10 @@
Pragma = foreign_code(Lang, Code),
mercury_output_pragma_foreign_body_code(Lang, Code, !IO)
;
- Pragma = foreign_proc(Attributes, Pred, PredOrFunc, Vars, VarSet,
- PragmaCode),
+ Pragma = foreign_proc(Attributes, Pred, PredOrFunc, Vars, ProgVarset,
+ InstVarset, PragmaCode),
mercury_output_pragma_foreign_code(Attributes, Pred,
- PredOrFunc, Vars, VarSet, varset.coerce(VarSet), PragmaCode, !IO)
+ PredOrFunc, Vars, ProgVarset, InstVarset, PragmaCode, !IO)
;
Pragma = import(Pred, PredOrFunc, ModeList, Attributes, C_Function),
mercury_format_pragma_import(Pred, PredOrFunc, ModeList,
only in patch2:
unchanged:
--- compiler/add_solver.m 28 Oct 2005 02:09:57 -0000 1.6
+++ compiler/add_solver.m 7 Nov 2005 07:55:46 -0000
@@ -183,10 +183,12 @@
OutAnyMode = out_mode(AnyInst),
OutGroundMode = out_mode(GroundInst),
- VarSet0 = varset__init,
- varset__new_var(VarSet0, X, VarSet1),
- varset__new_var(VarSet1, Y, VarSet),
-
+ ProgVarSet0 = varset__init,
+ varset__new_var(ProgVarSet0, X, ProgVarSet1),
+ varset__new_var(ProgVarSet1, Y, ProgVarSet),
+
+ InstVarSet = varset__init,
+
Attrs0 = default_attributes(c),
some [!Attrs] (
!:Attrs = Attrs0,
@@ -209,7 +211,8 @@
ToGroundRepnSymName,
function,
ToGroundRepnArgs,
- VarSet,
+ ProgVarSet,
+ InstVarSet,
Impl
),
ToGroundRepnItem = pragma(compiler(solver_type), ToGroundRepnForeignProc),
@@ -227,7 +230,8 @@
ToAnyRepnSymName,
function,
ToAnyRepnArgs,
- VarSet,
+ ProgVarSet,
+ InstVarSet,
Impl
),
ToAnyRepnItem = pragma(compiler(solver_type), ToAnyRepnForeignProc),
@@ -245,7 +249,8 @@
FromGroundRepnSymName,
function,
FromGroundRepnArgs,
- VarSet,
+ ProgVarSet,
+ InstVarSet,
Impl
),
FromGroundRepnItem = pragma(compiler(solver_type),
@@ -264,7 +269,8 @@
FromAnyRepnSymName,
function,
FromAnyRepnArgs,
- VarSet,
+ ProgVarSet,
+ InstVarSet,
Impl
),
FromAnyRepnItem = pragma(compiler(solver_type), FromAnyRepnForeignProc),
only in patch2:
unchanged:
--- compiler/make_hlds_passes.m 4 Nov 2005 03:40:49 -0000 1.20
+++ compiler/make_hlds_passes.m 7 Nov 2005 08:12:14 -0000
@@ -819,10 +819,10 @@
Item = pragma(Origin, Pragma),
(
Pragma = foreign_proc(Attributes, Pred, PredOrFunc,
- Vars, VarSet, PragmaImpl)
+ Vars, ProgVarSet, InstVarSet, PragmaImpl)
->
module_add_pragma_foreign_proc(Attributes, Pred, PredOrFunc,
- Vars, VarSet, PragmaImpl, !.Status, Context,
+ Vars, ProgVarSet, InstVarSet, PragmaImpl, !.Status, Context,
!ModuleInfo, !QualInfo, !IO)
;
Pragma = import(Name, PredOrFunc, Modes, Attributes, C_Function)
@@ -1124,7 +1124,8 @@
Item = mutable(Name, _Type, InitTerm, Inst, MutAttrs),
( status_defined_in_this_module(!.Status, yes) ->
module_info_get_name(!.ModuleInfo, ModuleName),
- varset__new_named_var(varset.init, "X", X, VarSet0),
+ varset.new_named_var(varset.init, "X", X, ProgVarSet0),
+ InstVarset = varset.init,
Attrs0 = default_attributes(c),
set_may_call_mercury(will_not_call_mercury, Attrs0, Attrs1),
( mutable_var_thread_safe(MutAttrs) = thread_safe ->
@@ -1159,7 +1160,7 @@
NonPureGetClause = pragma(compiler(mutable_decl),
foreign_proc(GetAttrs,
mutable_get_pred_sym_name(ModuleName, Name), predicate,
- [pragma_var(X, "X", out_mode(Inst))], VarSet0,
+ [pragma_var(X, "X", out_mode(Inst))], ProgVarSet0, InstVarset,
ordinary("X = " ++ TargetMutableName ++ ";", yes(Context)))),
add_item_clause(NonPureGetClause, !Status, Context, !ModuleInfo,
!QualInfo, !IO),
@@ -1192,7 +1193,7 @@
),
NonPureSetClause = pragma(compiler(mutable_decl), foreign_proc(Attrs,
mutable_set_pred_sym_name(ModuleName, Name), predicate,
- [pragma_var(X, "X", in_mode(Inst))], VarSet0,
+ [pragma_var(X, "X", in_mode(Inst))], ProgVarSet0, InstVarset,
ordinary(TrailCode ++ TargetMutableName ++ " = X;",
yes(Context)))),
add_item_clause(NonPureSetClause, !Status, Context, !ModuleInfo,
@@ -1208,8 +1209,8 @@
( mutable_var_attach_to_io_state(MutAttrs) = yes ->
set_tabled_for_io(tabled_for_io, Attrs0, PureIntAttrs0),
set_purity(purity_pure, PureIntAttrs0, PureIntAttrs),
- varset.new_named_var(VarSet0, "IO0", IO0, VarSet1),
- varset.new_named_var(VarSet1, "IO", IO, VarSet),
+ varset.new_named_var(ProgVarSet0, "IO0", IO0, ProgVarSet1),
+ varset.new_named_var(ProgVarSet1, "IO", IO, ProgVarSet),
PureSetClause = pragma(compiler(mutable_decl),
foreign_proc(PureIntAttrs,
mutable_set_pred_sym_name(ModuleName, Name), predicate,
@@ -1217,7 +1218,7 @@
pragma_var(X, "X", in_mode(Inst)),
pragma_var(IO0, "IO0", di_mode),
pragma_var(IO, "IO", uo_mode)
- ], VarSet,
+ ], ProgVarSet, InstVarset,
ordinary(TargetMutableName ++ " = X; IO = IO0;",
yes(Context)
)
@@ -1232,7 +1233,7 @@
pragma_var(X, "X", out_mode(Inst)),
pragma_var(IO0, "IO0", di_mode),
pragma_var(IO, "IO", uo_mode)
- ], VarSet,
+ ], ProgVarSet, InstVarset,
ordinary(
"X = " ++ TargetMutableName ++ ";" ++
"IO = IO0;",
only in patch2:
unchanged:
--- compiler/modules.m 4 Nov 2005 03:40:53 -0000 1.355
+++ compiler/modules.m 7 Nov 2005 07:42:56 -0000
@@ -2099,7 +2099,7 @@
pragma_allowed_in_interface(foreign_decl(_, _, _), no).
pragma_allowed_in_interface(foreign_import_module(_, _), yes).
pragma_allowed_in_interface(foreign_code(_, _), no).
-pragma_allowed_in_interface(foreign_proc(_, _, _, _, _, _), no).
+pragma_allowed_in_interface(foreign_proc(_, _, _, _, _, _, _), no).
pragma_allowed_in_interface(inline(_, _), no).
pragma_allowed_in_interface(no_inline(_, _), no).
pragma_allowed_in_interface(obsolete(_, _), yes).
@@ -5820,7 +5820,7 @@
Info = Info0 ^ used_foreign_languages :=
set__insert(Info0 ^ used_foreign_languages, Lang)
;
- Pragma = foreign_proc(Attrs, Name, _, _, _, _)
+ Pragma = foreign_proc(Attrs, Name, _, _, _, _, _)
->
NewLang = foreign_language(Attrs),
( OldLang = Info0 ^ foreign_proc_languages ^ elem(Name) ->
@@ -7388,7 +7388,7 @@
Lang = foreign_type_language(ForeignType).
item_needs_foreign_imports(pragma(_, foreign_decl(Lang, _, _)), Lang).
item_needs_foreign_imports(pragma(_, foreign_code(Lang, _)), Lang).
-item_needs_foreign_imports(pragma(_, foreign_proc(Attrs, _, _, _, _, _)),
+item_needs_foreign_imports(pragma(_, foreign_proc(Attrs, _, _, _, _, _, _)),
foreign_language(Attrs)).
item_needs_foreign_imports(mutable(_, _, _, _, _), Lang) :-
foreign_language(Lang).
@@ -7688,7 +7688,7 @@
; Pragma = foreign_code(_, _), Reorderable = no
; Pragma = foreign_decl(_, _, _), Reorderable = no
; Pragma = foreign_import_module(_, _), Reorderable = no
- ; Pragma = foreign_proc(_, _, _, _, _, _), Reorderable = no
+ ; Pragma = foreign_proc(_, _, _, _, _, _, _), Reorderable = no
; Pragma = import(_, _, _, _, _), Reorderable = no
; Pragma = inline(_, _), Reorderable = yes
; Pragma = mode_check_clauses(_, _), Reorderable = yes
@@ -7776,7 +7776,7 @@
; Pragma = foreign_code(_, _), Reorderable = no
; Pragma = foreign_decl(_, _, _), Reorderable = no
; Pragma = foreign_import_module(_, _), Reorderable = no
- ; Pragma = foreign_proc(_, _, _, _, _, _), Reorderable = no
+ ; Pragma = foreign_proc(_, _, _, _, _, _, _), Reorderable = no
; Pragma = import(_, _, _, _, _), Reorderable = no
; Pragma = inline(_, _), Reorderable = yes
; Pragma = mode_check_clauses(_, _), Reorderable = yes
only in patch2:
unchanged:
--- compiler/prog_data.m 4 Nov 2005 03:40:54 -0000 1.145
+++ compiler/prog_data.m 7 Nov 2005 07:34:31 -0000
@@ -427,6 +427,7 @@
proc_p_or_f :: pred_or_func,
proc_vars :: list(pragma_var),
proc_varset :: prog_varset,
+ proc_instvarset :: inst_varset,
proc_impl :: pragma_foreign_code_impl
% Set of foreign proc attributes, eg.:
% what language this code is in
only in patch2:
unchanged:
--- compiler/prog_io_pragma.m 28 Oct 2005 02:10:30 -0000 1.92
+++ compiler/prog_io_pragma.m 7 Nov 2005 07:45:02 -0000
@@ -1583,9 +1583,10 @@
parse_pragma_c_code_varlist(VarSet0, VarList, PragmaVars, Error),
(
Error = no,
- varset__coerce(VarSet0, VarSet),
+ varset__coerce(VarSet0, ProgVarSet),
+ varset__coerce(VarSet0, InstVarSet),
Result = ok(pragma(user, foreign_proc(Flags, PredName, PredOrFunc,
- PragmaVars, VarSet, PragmaImpl)))
+ PragmaVars, ProgVarSet, InstVarSet, PragmaImpl)))
;
Error = yes(ErrorMessage),
Result = error(ErrorMessage, PredAndVarsTerm0)
only in patch2:
unchanged:
--- compiler/recompilation.version.m 4 Nov 2005 03:40:55 -0000 1.30
+++ compiler/recompilation.version.m 7 Nov 2005 07:46:04 -0000
@@ -559,7 +559,7 @@
is_pred_pragma(foreign_decl(_, _, _), no).
is_pred_pragma(foreign_import_module(_, _), no).
is_pred_pragma(foreign_code(_, _), no).
-is_pred_pragma(foreign_proc(_, Name, PredOrFunc, Args, _, _),
+is_pred_pragma(foreign_proc(_, Name, PredOrFunc, Args, _, _, _),
yes(yes(PredOrFunc) - Name / Arity)) :-
adjust_func_arity(PredOrFunc, Arity, list__length(Args)).
is_pred_pragma(type_spec(Name, _, Arity, MaybePredOrFunc, _, _, _, _),
--------------------------------------------------------------------------
mercury-reviews mailing list
post: mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------
More information about the reviews
mailing list