[Mono-dev] [PATCH] Mono port to the MIPS 64-bit

[Paolo Molaro]

On 04/24/09 Ian Dichkovsky wrote:
> And we have prepared ChangeLogs and added them to
> https://bugzilla.novell.com/show_bug.cgi?id=497320
> ChangeLog.diff
> ChangeLog_libgc.diff
> ChangeLog_mono_arch.diff
> ChangeLog_mono_io-layer.diff
> ChangeLog_mono_mini.diff

Thanks for the port!
A few general comments on the changes:

*) please post the next iteration on this list, each patch with its
changelog (bugzilla is not the best en to comment on patches)

*) please use something like MONO_PPC_32_64_CASE in the ppc code when
you need simple values to differ in 32 and 64 bit mode. For example you
just increase trampoline sizes, but for 32 code they should remain
smaller and not waste memory. This helps reducing also #ifdefs in the
code in a few places

*) mono_arch_find_jit_info() is duplicated entirely, it's better to aoid
too much code duplication there.

*) in some places you use (unsigned long long) casts: it's better to use
gsize, so the code deals better with the 32 bit case.

*) some of the mini-mips.h changes likely break existing 32 bit code:
you need to #ifdef the 64 bit specific stuff
(MONO_ARCH_INST_IS_REGPAIR...)

*) things like:
+#if SIZEOF_VOID_P == 4
			add_int32_arg (cinfo, &cinfo->args[n]);
+#else
+			add_int64_arg (cinfo, &cinfo->args[n]);
+#endif

are better expressed with a single #ifdef:

#if SIZEOF_VOID_P == 4
#define add_reg_arg(a,b)	add_int32_arg (a, b)
#else
#define add_reg_arg(a,b)	add_int64_arg (a, b)
#endif

and then using add_reg_arg() in the code.

*) in quite a few cases indentation is incorrect, with the opening brace
put on a line on its own when it should be in the same line as
if/while/etc. In other cases there is the orrible:
		...
	}
	else {
		...
which should be instead:
		...
	} else {
		...

*) in general it seems that in a few places the older code was simply
replaced. Now I don't know the arch details, so the new code may as well
be correct also for the 32 bit port, but it seems that in some cases too
much code just code removed and things were tests on 64 bit only.

Anyway, good work, looking forward the updated patches and Mark's
comments.

lupus

-- 
-----------------------------------------------------------------
lupus at debian.org                                     debian/rules
lupus at ximian.com                             Monkeys do it better