[webkit-reviews] review granted: [Bug 34424] Soft modulo routine for ARM_TRADITIONAL : [Attachment 48140] Add a soft modulo operation to ARM JIT using a trampoline function (use it second).

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 4 11:47:17 PST 2010


Gavin Barraclough <barraclough at apple.com> has granted Tamas Szirbucz
<Szirbucz.Tamas at stud.u-szeged.hu>'s request for review:
Bug 34424: Soft modulo routine for ARM_TRADITIONAL
https://bugs.webkit.org/show_bug.cgi?id=34424

Attachment 48140:  Add a soft modulo operation to ARM JIT using a trampoline
function (use it second).
https://bugs.webkit.org/attachment.cgi?id=48140&action=review

------- Additional Comments from Gavin Barraclough <barraclough at apple.com>
Okay, this basically all looks great.  A couple of small things:

(1) One of the implementations of 'emitSlow_op_mod' contains an
'ASSERT_NOT_REACHED();', one doesn't (before and after your patch).  I think
this should be there in both cases, unless I'm missing something please add
this to the case without the assert.
(2) For configurable JIT optimizations we've been using switches of the format
"ENABLE_JIT_OPTIMIZE_???", and I think it would make sense to move the switch
to bring it in line with these.  (part of the reason is that it may be nice at
some point to move these out of Platform so that changing these switches only
means recompiling the JIT rather than all of WebCore).	I'd suggest
"ENABLE_JIT_OPTIMIZE_MOD", alongside the other JIT_OPTIMIZE switches, and I'd
suggest defining it as something like:

#ifndef ENABLE_JIT_OPTIMIZE_MOD
#define ENABLE_JIT_OPTIMIZE_MOD (CPU(ARM_TRADITIONAL) &&
WTF_ARM_ARCH_AT_LEAST(5))
#endif

With these changes, r+.


More information about the webkit-reviews mailing list