[Webkit-unassigned] [Bug 30144] MIPS JIT Supports

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=30144





--- Comment #20 from Oliver Hunt <oliver at apple.com>  2009-10-28 13:19:22 PDT ---
(From update of attachment 41741)
I am just doing a cursory review -- Gavin should probably look at this in more
detail.

I find the comments thaat repeat the funciton name to be superfluous, eg
 257     void lui(RegisterID rt, int imm)
 258     {
 259         /* lui */
 260         emitInst(0x3c000000 | (rt << OP_SH_RT) | (imm & 0xffff));
 261     }

The comment /* lui */ contains no useful information so should be removed.

Same applies for all the other cases (addiu, addu, subu, ... )

I don't really like the repeated 
 #if __mips == 1
         nop();
 #endif

And would prefer a function akin to

void loadDelayNop()
{
#if __mips == 1
         nop();
#endif
}

Or whatever the name should be (i'm guessing about the name, it's been years
since i did anything at all with mips) to give it most clarity, and then
replace those conditional nops with an anconditional call to the helper
function.

I have to disappear for a bit -- i'm up to assembler/MacroAssemblerMIPS.h --
overall this patch looks really good though.  Possibly the first large patch
from a new contributor to have (so far) perfect code style :D

--Oliver

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.