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

[bugzilla-daemon at webkit.org]

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





--- Comment #43 from Chao-ying Fu <fu at mips.com>  2010-02-24 17:50:38 PST ---
(In reply to comment #42)
> Some style issues... (please check the style bot)
> 
> Tests for true/false, null/non-null, and zero/non-zero should all be done
> without equality comparisons. For example:
> 
> >+        ASSERT(*(insn - 1) == 0 && *(insn - 2) == 0 && *(insn - 3) == 0 && *(insn - 5) == 0);
> 
> or
> 
> >+        if (!imm.m_isPointer && imm.m_value == 0 && !m_fixedWidth)
> 
> 
> >+    void lshift32(RegisterID shift_amount, RegisterID dest)
> >+    {
> >+        m_assembler.sllv(dest, dest, shift_amount);
> >+    }
> 
> Do not use '_' in your variable names!

  Yes.  I will fix all style issues that come from my code.

> 
> Some other issues...
> 
> Well, it would be nice if you define a new macro which can hold the instruction
> set architecture value and an other one which can test the requested
> instruction set. For example.:
> #define WTF_MIPS_ARCH __mips
> #define WTF_MIPS_ISA(v) (defined WTF_MIPS_ARCH && WTF_MIPS_ARCH == v)
> or
> #define WTF_MIPS_ISA_AT_LEAST(v) (defined WTF_MIPS_ARCH && WTF_MIPS_ARCH >= v)
> 
> Same for _ABIO32, __mips_isa_rev and others.

  Yes.  I will use WTF_* for all tests.  For _ABIO32, I will use it only in
Platform.h and not in other .cpp/.h files, since I can only support O32 now. 
In the future, if n32/o64 systems are added, we will define a new WTF_* for ABI
selections.

> 
> >+        __clear_cache(reinterpret_cast<char*>(start), reinterpret_cast<char*>(end));
> >+#else
> >+        intptr_t end = reinterpret_cast<intptr_t>(code) + size;
> >+        __clear_cache(reinterpret_cast<char*>(code), reinterpret_cast<char*>(end));
> >+#endif
> 
> Please read my comment about __clear_cache issue at
> https://bugs.webkit.org/show_bug.cgi?id=28886#c35 .
> (Try to avoid internal __clear_cache function)

  I will change __clear_cache to __builtin____clear_cache that is a GCC builtin
function from GCC manual.  Is it ok?

> 
> 
> >+#if defined(__MIPSEB__)
> Use CPU(BIG_ENDIAN)!

  Yes.

> 
> Anyway, this patch looks good to me as well ;)

  Thanks a lot!  I am testing my new patch and will post soon.
> 
> BTW, do you have some comparison data about how the JIT performs on MIPS?

  From my last December results, the non-jit SunSpider is 18774.6ms and the jit
SunSpider is 10748.6ms on a MIPS 74kf 660Mhz sigma board.  The total time
reduction is about 42%.  Thanks!

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