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

[Tyson Dowd]

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.

> > +	Info = Info1 ^ file_c_code := ContainsCCode,
> > +	ClassDecls = Info ^ classdecls,
> > +	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! ;-)

> > +	% initialize this value, leave it on the stack.
> > +	% XXX the code generator doesn't box these values
> > +	% we need to look ahead at them and box them appropriately.
> > +:- pred data_initializer_to_instrs(mlds__initializer::in,
> > +	instr_tree::out, instr_tree::out, il_info::in, il_info::out) is det.
> > +data_initializer_to_instrs(init_obj(Rval), node([]), InitInstrs) --> 
> > +	load(Rval, InitInstrs).
> 
> Is the XXX comment here still correct?

AFAIK.  
 
> > +	% XXX check these, what should we do about multi strings, 
> > +	% characters, etc.
> > +load(const(Const), Instrs, Info, Info) :- 
> > +	( Const = true,
> > +		Instrs = instr_node(ldc(int32, i(1)))
> > +	; Const = false,
> > +		Instrs = instr_node(ldc(int32, i(0)))
> 
> 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.

> > +load(mem_addr(Lval), Instrs, Info0, Info) :- 
> > +	( Lval = var(Var),
> > +		mangle_mlds_var(Var, MangledVarStr),
> > +		Info0 = Info,
> > +		( is_local(MangledVarStr, Info) ->
> > +			Instrs = instr_node(ldloca(name(MangledVarStr)))
> > +		;
> > +			Instrs = instr_node(ldarga(name(MangledVarStr)))
> > +		)
> > +	; Lval = field(_MaybeTag, Rval, FieldNum, FieldType, ClassType),
> > +		get_fieldref(FieldNum, FieldType, ClassType, FieldRef),
> > +		load(Rval, RvalLoadInstrs, Info0, Info),
> > +		Instrs = tree__list([
> > +			RvalLoadInstrs, 
> > +			instr_node(ldflda(FieldRef))
> > +			])
> > +	; Lval = mem_ref(_, _),
> > +		Info0 = Info,
> > +		Instrs = throw_unimplemented("load mem_addr lval mem_ref")
> > +	).
> 
> Hmm, should there be an XXX comment above the call to throw_unimplemented?
> Or is there some reason why this case won't arise?

It's just not implemented.

All calls to throw_unimplemented are unimplemented but should be
implemented.

> > +unaryop_to_il(std_unop(hash_string), _,
> > +	throw_unimplemented("unimplemented hash_string unop")) --> [].
> 
> That one will need to be implemented sometime,
> since I plan to make use of it for string switches...
> The code for this can just be a call to string__hash in library/string.m.

Same deal here.

> > +	% Integer comparison
> > +binaryop_to_il((<), node([clt(signed)])) --> [].
> > +binaryop_to_il((>), node([cgt(signed)])) --> [].
> > +binaryop_to_il((<=), node([cgt(signed), ldc(int32, i(0)), ceq])) --> [].
> > +binaryop_to_il((>=), node([clt(signed), ldc(int32, i(0)), ceq])) --> [].
> 
> All of those hard-coded `int32's here and elsewhere are probably
> not a good idea.  For most of these, it would be better to use
> a more abstract name, e.g. `mercury_int_il_type'.
> On a 64-bit architecture it might make more sense to map Mercury ints
> to int64 rather than int32.
> 

Ok, I've added that to the to-do list.

> > +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.

> 
> > +:- func convert_to_object(ilds__type) = methodref.
> > +
> > +convert_to_object(Type) = methoddef(call_conv(no, default), 
> > +		simple_type(il_generic_simple_type),
> > +		member_name(ConvertClass, id("ToObject")), [Type]) :-
> > +	ConvertClass = ["mercury", "mr_convert"].
> 
> "mr_" should be "MR_" (throughout).

Maybe (it doesn't really matter what it is called in the mercury
namespace, but MR_ is a good as anything), but I'm not going to change
it just yet because it involves changing the rest of the library and
runtime code too, and I'd like to get it all working again before I
break it.

This code was supposed to be temporary anyway, since box/unbox are
supposed to be implemented correctly some day.  Aargh, yell, scream,
etc.

> 
> > +:- 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.

> 
> > +:- func il_string_class_name = structured_name.
> > +il_string_class_name = ["mscorlib", "System", "String"].
> 
> I think it would be a good idea to abstract out all the occurrences
> of "mscorlib" and put them in a function -- MS might change the name
> at some point.

Well they already changed the name "System"... (it used to be "Microsoft")

> 
> > +:- 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.

> > +:- 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"?

-- 
       Tyson Dowd           # 
                            #  Surreal humour isn't everyone's cup of fur.
     trd at cs.mu.oz.au        # 
http://www.cs.mu.oz.au/~trd #
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------