[Mono-bugs] [Bug 324134] Add support for Mac OS X / ppc64

[bugzilla_noreply at novell.com]

https://bugzilla.novell.com/show_bug.cgi?id=324134

User gnorton at novell.com added comment
https://bugzilla.novell.com/show_bug.cgi?id=324134#c10





--- Comment #10 from Geoff Norton <gnorton at novell.com>  2008-05-10 18:49:57 MST ---
Andreas,

  Regarding the general comments, changes like I referred to aren't about
wether a patch is ready for SVN or not, whitespace changes, and comments like
//CHECKME make the patch (in its current state) much more difficult to read.

Examples:

1:
+//#ifndef __ppc64__
 #define JIT_RUNTIME_WORKS
+//#endif

2:
-       g_assert(*code == 0x4e800021);
+       g_assert (*code == 0x4e800021);

3:
+//CHECKME ppc64

4:
+       //printf("mono_arch_create_trampoline_code\n");

5:
-       start = mono_global_codeman_reserve (168);
-       mono_arch_get_throw_exception_generic (start, 168, TRUE, FALSE);
+#if defined(__ppc64__) || defined(__powerpc64__)
+#define CODE_LENGTH 296
+#else
+#define CODE_LENGTH 168
+#endif
+       start = mono_global_codeman_reserve (CODE_LENGTH);
+       mono_arch_get_throw_exception_generic (start, CODE_LENGTH, TRUE,
FALSE);
+#undef CODE_LENGTH

There are more, thats just the first 5 that popped out to me

changes like:

-               ppc_patch ((char*)code, addr);
-               mono_arch_flush_icache ((char*)code, 4);
+               ppc_patch ((guint8*)code, addr);
+               mono_arch_flush_icache ((guint8*)code, 4);

also unecessarily bloat the patch.  They should be included in a seperate patch
after the logica changes.


As for the extra registers; you have this code:

+#if defined(__ppc64__) || defined(__powerpc64__)
+               for (i = 13; i < 32; i++) {
+                       ppc_std (buf, i, (guint16)offset >> 2, ppc_sp);
+                       offset += sizeof (gulong);
+               }
+#else
                ppc_stmw (buf, ppc_r13, ppc_r1, offset);
+#endif

Take a look yourself for the context.

sp == r1 is just more changes that should be in a seperate patch.

ppc-codegen.h is up for paolo to approve, but looks ok at a quick glance.  I'll
look at it more in detail.

Basically andreas I'm trying to get your patch as small as possible with code
changes so we can give you functional review without having to sift over tons
of code-style / whitespace / etc changes.

Thanks.
etc.

Its about making your patch more readable to keep the code changes so we can
give you functional reviews.


-- 
Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.