[Webkit-unassigned] [Bug 24986] ARM JIT port

[bugzilla-daemon at webkit.org]

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


barraclough at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #32047|review?                     |review-
               Flag|                            |




------- Comment #63 from barraclough at apple.com  2009-07-01 17:06 PDT -------
(From update of attachment 32047)
This all looks really great, couple of small comments.

In the MacroAssembler, we normally try to only use UNUSED_PARAM if a parameter
is only not use on some (ifdef'ed) code paths, so we'd normally write:

void mulDouble(Address src, FPRegisterID dest)
{
    UNUSED_PARAM(src);
    UNUSED_PARAM(dest);
    ASSERT_NOT_REACHED();
}

as:

void mulDouble(Address, FPRegisterID)
{
    ASSERT_NOT_REACHED();
}

But if you prefer to leave this unchanged, this would be okay.

In Platform.h:

 546 #elif PLATFORM(ARM)
 547     /* Under development, temporarily disabled. */
 548     #define ENABLE_JIT 0
 549     #define ENABLE_JIT_OPTIMIZE_NATIVE_CALL 0

This should really be added as a part of the JIT patch, let's revert this for
now (it really doesn't make a lot of sense without the rest of the JIT changes
:o) ).  Also...

 596  || (!defined(ENABLE_YARR_JIT) && PLATFORM(ARM) && 0) \

We will only want to enable YARR & the ARM JIT on operating systems it has been
tested on.  What OS are you testing on?  Linux?  GTK or KDE?  Or other?  Also,
if Linux, this may also be the wrong place to be enabling them.  On Linux I
believe the platform specific settings are configured in configure.ac (GTK?)
and JavaScriptCore/JavaScriptCore.pri (KDE?) – and I believe this is where YARR
is enabled on x86-Linux.  (Personally, I find the fact that we configure these
things in different places on different OSes a little odd, but given that we
do, it certainly seems that it would be more correct to keep the way we do
things on ARM-Linux in line with the way we work on x86-Linux - again, assuming
that it is Linux you're targeting).

The rest of the patch all looks great.  r-ing while the Platform.h questions
are answered, but otherwise this all looks good to land.


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