[m-rev.] fix bug with --high-level-data & .NET back-end
Fergus Henderson
fjh at cs.mu.OZ.AU
Thu Nov 8 19:44:41 AEDT 2001
I've tested this change by bootchecking it in both the
`hlc.gc' and `hl.gc' grades. I'll do some more testing
of the `ilc' and `il' grades before committing it.
Branches: main
Estimated hours taken: 12
Fix a bug in the --high-level-data version of the .NET back-end,
where when constructing data types with polymorphically typed fields,
we were calling the constructor with incorrect (unboxed) argument types.
It was documented in mlds.m that handling this boxing/unboxing was the
responsibility of the MLDS->target code generator, but the MLDS didn't
contain enough information (in particular it was missing the unsubstituted
constructor argument types for new_object statements). So I ended up
fixing it by doing this boxing in the HLDS->MLDS code generator.
This makes it more consistent with how we handle other cases where
boxing/unboxing is needed.
compiler/mlds.m:
Update the documentation about who is responsible for handling
boxing/unboxing for `new_object' statements (and improve the
documentation about the same issue for `field' rvals).
compiler/ml_unify_gen.m:
When generating new_object statements, look up the original
(unsubstituted) types for the constructor arguments, and
where necessary box the actual arguments to match.
compiler/mlds_to_il.m:
Don't box arguments of new_object statements, since this
is now done in ml_unify_gen.m.
compiler/type_util.m:
Export type_util__get_cons_defn, for use by ml_unify_gen.m.
Add some comments.
compiler/ml_code_util.m:
compiler/mlds_to_c.m:
Improve the comments.
Workspace: /home/earth/fjh/ws-earth3/mercury
Index: compiler/ml_code_util.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/ml_code_util.m,v
retrieving revision 1.47
diff -u -d -r1.47 ml_code_util.m
--- compiler/ml_code_util.m 31 Oct 2001 16:58:09 -0000 1.47
+++ compiler/ml_code_util.m 6 Nov 2001 06:53:19 -0000
@@ -332,7 +332,8 @@
:- func ml_gen_field_name(maybe(ctor_field_name), int) = mlds__field_name.
% Succeed iff the specified type must be boxed when used as a field.
- % We need to box types that are not word-sized, because the code
+ % For the MLDS->C and MLDS->asm back-ends,
+ % we need to box types that are not word-sized, because the code
% for `arg' etc. in std_util.m rely on all arguments being word-sized.
:- pred ml_must_box_field_type(prog_type, module_info).
:- mode ml_must_box_field_type(in, in) is semidet.
@@ -1572,8 +1573,12 @@
).
% Succeed iff the specified type must be boxed when used as a field.
- % We need to box types that are not word-sized, because the code
+ % For the MLDS->C and MLDS->asm back-ends,
+ % we need to box types that are not word-sized, because the code
% for `arg' etc. in std_util.m rely on all arguments being word-sized.
+ % XXX Currently we box such types even for the other MLDS based
+ % back-ends that don't need it, e.g. the .NET and Java back-ends.
+ % This routine should be modified to check the target.
ml_must_box_field_type(Type, ModuleInfo) :-
classify_type(Type, ModuleInfo, Category),
( Category = float_type
Index: compiler/ml_unify_gen.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/ml_unify_gen.m,v
retrieving revision 1.47
diff -u -d -r1.47 ml_unify_gen.m
--- compiler/ml_unify_gen.m 4 Nov 2001 14:30:35 -0000 1.47
+++ compiler/ml_unify_gen.m 8 Nov 2001 06:35:20 -0000
@@ -93,6 +93,7 @@
:- import_module globals, options.
:- import_module bool, int, string, list, map, require, std_util, term, varset.
+:- import_module assoc_list.
%-----------------------------------------------------------------------------%
@@ -1105,19 +1106,27 @@
MaybeTag = yes(Tag)
},
ml_variable_types(ArgVars, ArgTypes),
- list__map_foldl(ml_gen_type, ArgTypes, MLDS_ArgTypes0),
(
{ HowToConstruct = construct_dynamically },
%
- % Generate rvals for the arguments
+ % Find out the types of the constructor arguments
+ % and generate rvals for them (boxing/unboxing if needed)
%
ml_gen_var_list(ArgVars, ArgLvals),
=(Info),
{ ml_gen_info_get_module_info(Info, ModuleInfo) },
- { ml_gen_cons_args(ArgLvals, ArgTypes, ArgModes, ModuleInfo,
- ArgRvals0) },
+ { MaybeConsId = yes(ConsId) ->
+ ConsArgTypes = constructor_arg_types(ConsId,
+ ArgTypes, Type, ModuleInfo)
+ ;
+ % it's a closure
+ % XXX do we need to box the arguments here?
+ ConsArgTypes = ArgTypes
+ },
+ ml_gen_cons_args(ArgLvals, ArgTypes, ConsArgTypes, ArgModes,
+ ModuleInfo, ArgRvals0, MLDS_ArgTypes0),
%
% Insert the extra rvals at the start
@@ -1135,7 +1144,7 @@
% Generate a `new_object' statement to dynamically allocate
% the memory for this term from the heap. The `new_object'
% statement will also initialize the fields of this term
- % with boxed versions of the specified arguments.
+ % with the specified arguments.
%
{ MakeNewObject = new_object(VarLval, MaybeTag, HasSecTag,
MLDS_Type, yes(SizeInWordsRval), MaybeCtorName,
@@ -1148,6 +1157,21 @@
;
{ HowToConstruct = construct_statically(StaticArgs) },
+ % XXX This code gets the types wrong
+ % for the --high-level-data case
+
+ list__map_foldl(ml_gen_type, ArgTypes, MLDS_ArgTypes0),
+ /****
+ XXX We ought to do something like this instead:
+ =(Info),
+ { ml_gen_info_get_module_info(Info, ModuleInfo) },
+ { ConsArgTypes = constructor_arg_types(CtorId,
+ ArgTypes, Type, ModuleInfo) },
+ list__map_foldl(ml_gen_field_type, ConsArgTypes,
+ MLDS_ArgTypes0),
+ ...
+ *****/
+
%
% Generate rvals for the arguments
%
@@ -1255,6 +1279,104 @@
{ MLDS_Statements = [MLDS_Statement | MLDS_Statements0] }
).
+:- pred ml_type_as_field(prog_type, module_info, bool, prog_type).
+:- mode ml_type_as_field(in, in, in, out) is det.
+ml_type_as_field(FieldType, ModuleInfo, HighLevelData, BoxedFieldType) :-
+ (
+ %
+ % With the low-level data representation,
+ % we store all fields as boxed, so we ignore the
+ % original field type and instead generate a polymorphic
+ % type BoxedFieldType which we use for the type of the field.
+ % This type is used in the calls to
+ % ml_gen_box_or_unbox_rval to ensure that we
+ % box values when storing them into fields and
+ % unbox them when extracting them from fields.
+ %
+ % With the high-level data representation,
+ % we don't box everything, but for the MLDS->C and MLDS->asm
+ % back-ends we still need to box floating point fields
+ %
+ (
+ HighLevelData = no
+ ;
+ HighLevelData = yes,
+ ml_must_box_field_type(FieldType, ModuleInfo)
+ )
+ ->
+ varset__init(TypeVarSet0),
+ varset__new_var(TypeVarSet0, TypeVar, _TypeVarSet),
+ type_util__var(BoxedFieldType, TypeVar)
+ ;
+ BoxedFieldType = FieldType
+ ).
+
+:- func constructor_arg_types(cons_id, list(prog_type), prog_type,
+ module_info) = list(prog_type).
+
+constructor_arg_types(CtorId, ArgTypes, Type, ModuleInfo) = ConsArgTypes :-
+ (
+ CtorId = cons(_, _),
+ \+ is_introduced_type_info_type(Type)
+ ->
+ % Use the type to determine the type_id
+ ( type_to_type_id(Type, TypeId0, _) ->
+ TypeId = TypeId0
+ ;
+ % the type-checker should ensure that this never
+ % happens: the type for a ctor_id should never
+ % be a free type variable
+ unexpected(this_file,
+ "cons_id_to_arg_types: invalid type")
+ ),
+
+ % Given the type_id, lookup up the constructor
+ (
+ type_util__get_cons_defn(ModuleInfo, TypeId, CtorId,
+ ConsDefn)
+ ->
+ ConsDefn = hlds_cons_defn(_, _, ConsArgDefns, _, _),
+ assoc_list__values(ConsArgDefns, ConsArgTypes0),
+ %
+ % There may have been additional types inserted
+ % to hold the type_infos and type_class_infos
+ % for existentially quantified types.
+ % We can get these from the ArgTypes.
+ %
+ NumExtraArgs = list__length(ArgTypes) -
+ list__length(ConsArgTypes0),
+ ExtraArgTypes = list__take_upto(NumExtraArgs, ArgTypes),
+ ConsArgTypes = ExtraArgTypes ++ ConsArgTypes0
+ ;
+ % If we didn't find a constructor definition,
+ % maybe that is because this type was a built-in
+ % tuple type
+ type_is_tuple(Type, _)
+ ->
+ % In this case, the argument types are all fresh
+ % variables. Note that we don't need to worry about
+ % using the right varset here, since all we really
+ % care about at this point is whether something is
+ % a type variable or not, not which type variable it
+ % is.
+ varset__init(VarSet0),
+ varset__new_vars(VarSet0, list__length(ArgTypes),
+ ConsArgTypeVars, _VarSet),
+ term__var_list_to_term_list(ConsArgTypeVars,
+ ConsArgTypes)
+ ;
+ % type_util__get_cons_defn shouldn't have failed
+ unexpected(this_file,
+ "cons_id_to_arg_types: get_cons_defn failed")
+ )
+ ;
+ % For cases when CtorId \= cons(_, _) and it is not a tuple,
+ % as can happen e.g. for closures and type_infos,
+ % we assume that the arguments all have the right type already
+ % XXX is this the right thing to do?
+ ArgTypes = ConsArgTypes
+ ).
+
:- func ml_gen_mktag(int) = mlds__rval.
ml_gen_mktag(Tag) = unop(std_unop(mktag), const(int_const(Tag))).
@@ -1395,37 +1517,69 @@
),
{ QualifiedConsId = qual(ModuleName, ConsId) }.
-:- pred ml_gen_cons_args(list(mlds__lval), list(prog_type),
- list(uni_mode), module_info, list(mlds__rval)).
-:- mode ml_gen_cons_args(in, in, in, in, out) is det.
-
-ml_gen_cons_args(Lvals, Types, Modes, ModuleInfo, Rvals) :-
- ( ml_gen_cons_args_2(Lvals, Types, Modes, ModuleInfo, Rvals0) ->
- Rvals = Rvals0
- ;
- error("ml_gen_cons_args: length mismatch")
- ).
-
% Create a list of rvals for the arguments
% for a construction unification. For each argument which
% is input to the construction unification, we produce the
- % corresponding lval, but if the argument is free,
- % we produce a null value.
-
-:- pred ml_gen_cons_args_2(list(mlds__lval), list(prog_type),
- list(uni_mode), module_info, list(mlds__rval)).
-:- mode ml_gen_cons_args_2(in, in, in, in, out) is semidet.
+ % corresponding lval, boxed or unboxed if needed,
+ % but if the argument is free, we produce a null value.
+ %
+:- pred ml_gen_cons_args(list(mlds__lval), list(prog_type), list(prog_type),
+ list(uni_mode), module_info,
+ list(mlds__rval), list(mlds__type), ml_gen_info, ml_gen_info).
+:- mode ml_gen_cons_args(in, in, in, in, in, out, out, in, out) is det.
-ml_gen_cons_args_2([], [], [], _, []).
-ml_gen_cons_args_2([Lval|Lvals], [Type|Types], [UniMode|UniModes],
- ModuleInfo, [Rval|Rvals]) :-
- UniMode = ((_LI - RI) -> (_LF - RF)),
- ( mode_to_arg_mode(ModuleInfo, (RI -> RF), Type, top_in) ->
- Rval = lval(Lval)
+ml_gen_cons_args(Lvals, ArgTypes, ConsArgTypes, UniModes, ModuleInfo,
+ Rvals, MLDS_Types) -->
+ (
+ { Lvals = [] },
+ { ArgTypes = [] },
+ { ConsArgTypes = [] },
+ { UniModes = [] }
+ ->
+ { Rvals = [] },
+ { MLDS_Types = [] }
;
- Rval = const(null(mercury_type_to_mlds_type(ModuleInfo, Type)))
- ),
- ml_gen_cons_args_2(Lvals, Types, UniModes, ModuleInfo, Rvals).
+ { Lvals = [Lval | Lvals1] },
+ { ArgTypes = [ArgType | ArgTypes1] },
+ { ConsArgTypes = [ConsArgType | ConsArgTypes1] },
+ { UniModes = [UniMode | UniModes1] }
+ ->
+ %
+ % Figure out the type of the field.
+ % Note that for the MLDS->C and MLDS->asm back-ends,
+ % we need to box floating point fields.
+ %
+ { module_info_globals(ModuleInfo, Globals) },
+ { globals__lookup_bool_option(Globals, highlevel_data,
+ HighLevelData) },
+ { ml_type_as_field(ConsArgType, ModuleInfo, HighLevelData,
+ BoxedArgType) },
+ { MLDS_Type = mercury_type_to_mlds_type(ModuleInfo,
+ BoxedArgType) },
+ %
+ % Compute the value of the field
+ %
+ { UniMode = ((_LI - RI) -> (_LF - RF)) },
+ (
+ { mode_to_arg_mode(ModuleInfo, (RI -> RF), ArgType,
+ top_in) }
+ ->
+ ml_gen_box_or_unbox_rval(ArgType, BoxedArgType,
+ lval(Lval), Rval)
+ ;
+ { Rval = const(null(MLDS_Type)) }
+ ),
+ %
+ % Process the remaining arguments
+ %
+ ml_gen_cons_args(Lvals1, ArgTypes1, ConsArgTypes1, UniModes1,
+ ModuleInfo, Rvals1, MLDS_Types1),
+ { Rvals = [Rval | Rvals1] },
+ { MLDS_Types = [MLDS_Type | MLDS_Types1] }
+ ;
+ { unexpected(this_file,
+ "ml_gen_cons_args: length mismatch") }
+ ).
%-----------------------------------------------------------------------------%
@@ -1745,34 +1899,11 @@
)
)
},
- {
%
- % With the low-level data representation,
- % we store all fields as boxed, so we ignore the field
- % type from `Field' and instead generate a polymorphic
- % type BoxedFieldType which we use for the type of the field.
- % This type is used in the calls to
- % ml_gen_box_or_unbox_rval below to ensure that we
- % box values when storing them into fields and
- % unbox them when extracting them from fields.
- %
- % With the high-level data representation,
- % we don't box everything, but we still need
- % to box floating point fields.
+ % Box the field type, if needed
%
- (
- HighLevelData = no
- ;
- HighLevelData = yes,
- ml_must_box_field_type(FieldType, ModuleInfo)
- )
- ->
- varset__init(TypeVarSet0),
- varset__new_var(TypeVarSet0, TypeVar, _TypeVarSet),
- type_util__var(BoxedFieldType, TypeVar)
- ;
- BoxedFieldType = FieldType
- },
+ { ml_type_as_field(FieldType, ModuleInfo, HighLevelData,
+ BoxedFieldType) },
%
% Generate lvals for the LHS and the RHS
Index: compiler/mlds.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/mlds.m,v
retrieving revision 1.74
diff -u -d -r1.74 mlds.m
--- compiler/mlds.m 31 Oct 2001 13:12:19 -0000 1.74
+++ compiler/mlds.m 6 Nov 2001 06:23:32 -0000
@@ -1119,14 +1119,20 @@
% The types of the arguments to the
% constructor.
%
- % Note that currently we store all
- % fields as type mlds__generic_type.
- % But the type here is the actual
- % argument type, which does not
- % have to be mlds__generic_type.
- % It is the responsibility of the
- % MLDS->target code output phase
- % to box the arguments if necessary.
+ % Note that for --low-level-data, we box
+ % all fields of objects created with
+ % new_object, i.e. they are reprsented
+ % with type mlds__generic_type.
+ % We also do that for some fields
+ % even for --high-level-data
+ % (e.g. floating point fields for the
+ % MLDS->C and MLDS->asm back-ends).
+ % In such cases, the type here
+ % should be mlds__generic_type;
+ % it is the responsibility of the
+ % HLDS->MLDS code generator
+ % to insert code to box/unbox
+ % the arguments.
%
)
@@ -1311,9 +1317,15 @@
% The PtrType is the type of the pointer
% from which we are fetching the field.
%
- % Note that currently we store all fields
- % of objects created with new_object
- % as type mlds__generic_type. For such objects,
+ % Note that for --low-level-data, we box
+ % all fields of objects created with
+ % new_object, i.e. they are reprsented
+ % with type mlds__generic_type.
+ % We also do that for some fields
+ % even for --high-level-data
+ % (e.g. floating point fields for the
+ % MLDS->C and MLDS->asm back-ends)
+ % In such cases,
% the type here should be mlds__generic_type,
% not the actual type of the field.
% If the actual type is different, then it
Index: compiler/mlds_to_c.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/mlds_to_c.m,v
retrieving revision 1.107
diff -u -d -r1.107 mlds_to_c.m
--- compiler/mlds_to_c.m 25 Oct 2001 08:35:37 -0000 1.107
+++ compiler/mlds_to_c.m 6 Nov 2001 06:57:16 -0000
@@ -2612,8 +2612,15 @@
mlds_output_init_args([Arg|Args], [ArgType|ArgTypes], Context,
ArgNum, Target, Tag, Indent) -->
%
- % Currently all fields of new_object instructions are
- % represented as MR_Box, so we need to box them if necessary.
+ % The MR_hl_field() macro expects its argument to
+ % have type MR_Box, so we need to box the arguments
+ % if they aren't already boxed. Hence the use of
+ % mlds_output_boxed_rval below.
+ %
+ % XXX For --high-level-data, we ought to generate
+ % assignments to the fields (or perhaps a call to
+ % a constructor function) rather than using the
+ % MR_hl_field() macro.
%
mlds_indent(Context, Indent),
io__write_string("MR_hl_field("),
Index: compiler/mlds_to_il.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/mlds_to_il.m,v
retrieving revision 1.89
diff -u -d -r1.89 mlds_to_il.m
--- compiler/mlds_to_il.m 25 Oct 2001 08:56:24 -0000 1.89
+++ compiler/mlds_to_il.m 6 Nov 2001 06:15:25 -0000
@@ -598,7 +598,8 @@
CtorFlags = init_decl_flags(public, per_instance, non_virtual,
overridable, modifiable, concrete),
- CtorDefn = mlds__defn(export(".ctor"), Context, CtorFlags, Ctor),
+ CtorDefn = mlds__defn(export(".ctor"), Context, CtorFlags,
+ Ctor),
Ctors = [CtorDefn]
;
Ctors = Ctors0
@@ -1946,7 +1947,7 @@
instr_node(CallCtor),
StoreLvalInstrs
]) }
- ;
+ ;
% Otherwise this is a generic mercury object -- we
% use an array of System::Object to represent
% it.
@@ -1960,24 +1961,18 @@
%
% dup
% ldc <array index>
- % ... load and box rval ...
+ % ... load rval ...
% stelem System::Object
%
% Finally, after all the array elements have
% been set:
%
% ... store to memory reference ...
-
- % We need to do the boxing ourselves because
- % MLDS hasn't done it. We add boxing unops to
- % the rvals.
- { Box = (pred(A - T::in, B::out) is det :-
- B = unop(box(T), A)
- ) },
- { assoc_list__from_corresponding_lists(Args0, ArgTypes0,
- ArgsAndTypes) },
- { list__map(Box, ArgsAndTypes, BoxedArgs) },
-
+ %
+ % Note that the MLDS code generator is
+ % responsible for boxing/unboxing the
+ % arguments if needed.
+
% Load each rval
% (XXX we do almost exactly the same code when
% initializing array data structures -- we
@@ -1993,7 +1988,7 @@
Arg = (Index + 1) - S
) },
=(State0),
- { list__map_foldl(LoadInArray, BoxedArgs, ArgsLoadInstrsTrees,
+ { list__map_foldl(LoadInArray, Args0, ArgsLoadInstrsTrees,
0 - State0, _ - State) },
{ ArgsLoadInstrs = tree__list(ArgsLoadInstrsTrees) },
dcg_set(State),
Index: compiler/type_util.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/type_util.m,v
retrieving revision 1.100
diff -u -d -r1.100 type_util.m
--- compiler/type_util.m 13 Sep 2001 23:18:15 -0000 1.100
+++ compiler/type_util.m 6 Nov 2001 04:58:07 -0000
@@ -198,9 +198,13 @@
:- pred type_util__switch_type_num_functors(module_info::in, (type)::in,
int::out) is semidet.
- % Work out the types of the arguments of a functor.
+ % Work out the types of the arguments of a functor,
+ % given the cons_id and type of the functor.
% Aborts if the functor is existentially typed.
% The cons_id is expected to be un-module-qualified.
+ % Note that this will substitute appropriate values for
+ % any type variables in the functor's argument types,
+ % to match their bindings in the functor's type.
:- pred type_util__get_cons_id_arg_types(module_info::in, (type)::in,
cons_id::in, list(type)::out) is det.
@@ -219,13 +223,27 @@
% Given a type and a cons_id, look up the definitions of that
% type and constructor. Aborts if the cons_id is not user-defined.
+ % Note that this will NOT bind type variables in the
+ % functor's argument types; they will be left unbound,
+ % so the caller find out the original types from the constructor
+ % definition. The caller must do that sustitution itself if required.
:- pred type_util__get_type_and_cons_defn(module_info, (type), cons_id,
hlds_type_defn, hlds_cons_defn).
:- mode type_util__get_type_and_cons_defn(in, in, in, out, out) is det.
+ % Like type_util__get_type_and_cons_defn (above), except that it
+ % only returns the definition of the constructor, not the type.
+:- pred type_util__get_cons_defn(module_info::in, type_id::in, cons_id::in,
+ hlds_cons_defn::out) is semidet.
+
+
% Given a type and a cons_id, look up the definition of that
% constructor; if it is existentially typed, return its definition,
% otherwise fail.
+ % Note that this will NOT bind type variables in the
+ % functor's argument types; they will be left unbound,
+ % so the caller find out the original types from the constructor
+ % definition. The caller must do that sustitution itself if required.
:- pred type_util__get_existq_cons_defn(module_info::in,
(type)::in, cons_id::in, ctor_defn::out) is semidet.
@@ -967,9 +985,6 @@
type_util__get_cons_defn(ModuleInfo, TypeId, ConsId, ConsDefn),
module_info_types(ModuleInfo, Types),
map__lookup(Types, TypeId, TypeDefn).
-
-:- pred type_util__get_cons_defn(module_info::in, type_id::in, cons_id::in,
- hlds_cons_defn::out) is semidet.
type_util__get_cons_defn(ModuleInfo, TypeId, ConsId, ConsDefn) :-
module_info_ctors(ModuleInfo, Ctors),
--
Fergus Henderson <fjh at cs.mu.oz.au> | "I have always known that the pursuit
The University of Melbourne | of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh> | -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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