https://bugzilla.novell.com/show_bug.cgi?id=324134
User [EMAIL PROTECTED] added comment https://bugzilla.novell.com/show_bug.cgi?id=324134#c10 --- Comment #10 from Geoff Norton <[EMAIL PROTECTED]> 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. _______________________________________________ mono-bugs maillist - [email protected] http://lists.ximian.com/mailman/listinfo/mono-bugs
