[m-dev.] for review: The .NET MSIL backend.

[Fergus Henderson]

On 13-Oct-2000, Tyson Dowd <trd at cs.mu.OZ.AU> wrote:
> On 11-Oct-2000, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> > On 22-Sep-2000, Tyson Dowd <trd at cs.mu.OZ.AU> wrote:
> > > Index: compiler/mlds_to_il.m
> > > +generate_il(MLDS, ILAsm, ContainsCCode, IO, IO) :-
> > > +	MLDS = mlds(MercuryModuleName, _ForeignCode, Imports, Defns),
> > > +	ModuleName = mercury_module_name_to_mlds(MercuryModuleName),
> > > +	il_info_init(ModuleName, Imports, Info0),
> > > +
> > > +		% Generate code for all the methods in this module.
> > > +	list__foldl(generate_method_defn, Defns, Info0, Info1),
> > > +	or(Info1 ^ file_c_code, Info1 ^ method_c_code, ContainsCCode),
> > 
> > Using infix `or` or bool__or would be clearer here.
> 
> Hmmm, we don't have a functional version of or.

OK, just make it bool__or and leave that for now.

> > > +	InitInstrs = condense(flatten(Info ^ init_instrs)),
> > > +	AllocInstrs = condense(flatten(Info ^ alloc_instrs)),
> > 
> > I suggest s/condense/list__condense/ and s/flatten/list__flatten/
> 
> Or even better, s/flatten/tree__flatten, because that makes it compile
> and everything! ;-)

Ah... I think that illustrates my point ;-)

> > Doesn't IL have a bool type?
> 
> Yes it does, but I'm not sure how to load bool values.
> I've been meaning to look into it for a while.

Quickest way to find out is probably to see what the C# compiler does.

> > > +rval_to_type(mkword(_Tag, _Rval), Type, I, I) :- 
> > > +	ModuleName = mercury_module_name_to_mlds(unqualified("mercury")),
> > > +	Type = mlds__class_type(qual(ModuleName, "incorrect"), 0, mlds__class).
> > > +rval_to_type(unop(_, _), Type, I, I) :- 
> > > +	ModuleName = mercury_module_name_to_mlds(unqualified("mercury")),
> > > +	Type = mlds__class_type(qual(ModuleName, "incorrect"), 0, mlds__class).
> > > +rval_to_type(binop(_, _, _), Type, I, I) :- 
> > > +	ModuleName = mercury_module_name_to_mlds(unqualified("mercury")),
> > > +	Type = mlds__class_type(qual(ModuleName, "incorrect"), 0, mlds__class).
> > > +rval_to_type(mem_addr(_), Type, I, I) :-
> > > +	ModuleName = mercury_module_name_to_mlds(unqualified("mercury")),
> > > +	Type = mlds__class_type(qual(ModuleName, "incorrect"), 0, mlds__class).
> > 
> > That code is duplicated four times.  Also I'm not sure what it is
> > trying to do.  What's this "mercury.incorrect" class type for?
> 
> Causing runtime errors.  Should probably be called "invalid" not
> "incorrect".  I'm pretty sure they could be changed to error in future,
> but I'd prefer not to do that without some more testing.

OK, just add an XXX comment explaining it...

> > > +:- func simple_type_to_value_class_name(simple_type) = string.
> > > +simple_type_to_value_class_name(int8) = "Int8".
> > > +simple_type_to_value_class_name(int16) = "Int16".
> > > +simple_type_to_value_class_name(int32) = "Int32".
> > > +simple_type_to_value_class_name(int64) = "Int64".
> > > +simple_type_to_value_class_name(uint8) = "Int8".
> > > +simple_type_to_value_class_name(uint16) = "UInt16".
> > > +simple_type_to_value_class_name(uint32) = "UInt32".
> > > +simple_type_to_value_class_name(uint64) = "UInt64".
> > > +simple_type_to_value_class_name(float32) = "Single".
> > > +simple_type_to_value_class_name(float64) = "Double".
> > > +simple_type_to_value_class_name(bool) = "Bool".
> > > +simple_type_to_value_class_name(char) = "Char".
> > > +simple_type_to_value_class_name(refany) = _ :-
> > > +	error("no value class name for refany").
> > > +simple_type_to_value_class_name(class(Name)) = VCName :-
> > > +	( Name = ["mscorlib", "System", "String"] ->
> > > +		VCName = "String"
> > > +	;
> > > +		error("unknown class name")
> > > +	).
> > 
> > Is `String' really a value class??
> 
> No.  That should be fixed by the caller of this function, converting
> from Object to String should be a cast, not a call to mr_convert.

OK, is there any reason why this can't just call error/1 if it gets a string?

> > > +:- func make_methoddecls(instr_tree) = list(methoddecl).
> > > +make_methoddecls(InstrTree) = MethodDecls :-
> > > +	Instrs = list__condense(flatten(tree(InstrTree, instr_node(ret)))),
> > > +	MethodDecls = [
> > > +		maxstack(100),
> > > +		instrs(Instrs)
> > > +		].
> > 
> > There's that magic number 100 again, this time without even an XXX...
> > Why does this code occur twice?  Do you need a .zeroinit here too?
> 
> Half finished abstraction.

OK, so at very least please add an XXX comment...
it would be even better if at least the magic number was
abstracted out into a single function called from both places.

> > > +:- mode il_info_get_mlds_type(in, out, in, out) is det.
> > > +il_info_get_mlds_type(Id, Type, Info0, Info0) :- 
> > > +	( 
> > > +		map__search(Info0 ^ locals, Id, Type0)
> > > +	->
> > > +		Type = Type0
> > > +	;
> > > +		assoc_list__search(Info0 ^ arguments, Id, Type0)
> > > +	->
> > > +		Type = Type0
> > > +	;
> > > +		% If it isn't a local or an argument, it can only be a
> > > +		% "global variable" -- used by RTTI.  We will assume this
> > > +		% is an integer for now.
> > > +		Type = native_int_type
> > > +	).
> > 
> > You should add an XXX here.
> > 
> > Also it would be helpful if this code was linked with the
> > other occurrence of this, e.g. by abstracting the use of
> > native_int_type here into a separate function called from
> > both places.
> 
> I don't understand this comment -- what other occurrence?  Which is the
> other place of the "both places"?

The other place where you assume that references to global variables
must have type int32 is in load/4 in mlds_to_il.m:

 | Index: compiler/mlds_to_il.m
 ...
 | +load(lval(Lval), Instrs, Info0, Info) :- 
 | +	( Lval = var(Var),
 | +		mangle_mlds_var(Var, MangledVarStr),
 | +		( is_local(MangledVarStr, Info0) ->
 | +			Instrs = instr_node(ldloc(name(MangledVarStr)))
 | +		; is_argument(Var, Info0) ->
 | +			Instrs = instr_node(ldarg(name(MangledVarStr)))
 | +		;
 | +			% XXX RTTI generates vars which are references
 | +			% to other modules!
 | +			% XXX we have no type information about this
 | +			% thing, so we have to assume it is a constant
 | +			% int32 in private_builtin__c_code.
 | +			Var = qual(ModuleName, _),
 | +			mangle_dataname_module(no, ModuleName,
 | +				NewModuleName),
 | +			StructuredName = mlds_module_name_to_structured_name(
 | +				NewModuleName),
 | +			FieldRef = make_fieldref(ilds__type([], int32),
 | +				StructuredName, MangledVarStr),

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------