On ARM there is not any modulo assembly instruction, but in case of integers it can be implemented with a simply routine. I will attach patches for that.
Created attachment 47834 [details] Use an easily appendable structure for trampolines instead of pointer parameters.
Created attachment 47835 [details] Add a soft modulo operation to ARM JIT using a trampoline function.
Created attachment 47836 [details] SunSpider results on ARMv7 It looks like slowdowns are caused by change of the binary.
Created attachment 48045 [details] Well formated patch for trampoline structures. (Use it first) id=47834 attachment's well formated version.
Created attachment 48046 [details] Well formated patch for soft modulo operation on ARM_TRADITIONAL. (Use it second) id=47835 attachment's well formated version.
Attachment 48045 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 JavaScriptCore/jit/JITStubs.h:78: Missing space before { [whitespace/braces] [5] Total errors found: 1 If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 48112 [details] Trampoline structure (use it first) Sorry for the many attachments! I used wrong the check-webkit-style at last time.
Created attachment 48113 [details] Soft modulo routine for ARM_TRADITIONAL (use it second) Sorry for the many attachments! I used wrong the check-webkit-style at last time.
Created attachment 48140 [details] Add a soft modulo operation to ARM JIT using a trampoline function (use it second). The previous attachment caused compile errors on X86.
Comment on attachment 48140 [details] Add a soft modulo operation to ARM JIT using a trampoline function (use it second). 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+.
> #ifndef ENABLE_JIT_OPTIMIZE_MOD > #define ENABLE_JIT_OPTIMIZE_MOD (CPU(ARM_TRADITIONAL) && > WTF_ARM_ARCH_AT_LEAST(5)) > #endif This will not work for non-ARM architectures, because there is no way to break the evaluation of logical expression in a #define. In x86 the #if ENABLE(JIT_OPTIMIZE_MOD) will turn into something similar: #if defined ENABLE_JIT_OPTIMIZE_MOD && CPU(ARM_TRADITIONAL) && WTF_ARM_ARCH_AT_LEAST(5), where WTF_ARM_ARCH_AT_LEAST is not defined. :(
(In reply to comment #11) > > #ifndef ENABLE_JIT_OPTIMIZE_MOD > > #define ENABLE_JIT_OPTIMIZE_MOD (CPU(ARM_TRADITIONAL) && > > WTF_ARM_ARCH_AT_LEAST(5)) > > #endif > > This will not work for non-ARM architectures, because there is no way to break > the evaluation of logical expression in a #define. In x86 the #if > ENABLE(JIT_OPTIMIZE_MOD) will turn into something similar: > #if defined ENABLE_JIT_OPTIMIZE_MOD && CPU(ARM_TRADITIONAL) && > WTF_ARM_ARCH_AT_LEAST(5), where WTF_ARM_ARCH_AT_LEAST is not defined. :( I think so, if CPU(ARM_TRADITIONAL) is false, WTF_ARM_ARCH_AT_LEAST will be never evaluated in this expression.
(In reply to comment #12) > (In reply to comment #11) > > > #ifndef ENABLE_JIT_OPTIMIZE_MOD > > > #define ENABLE_JIT_OPTIMIZE_MOD (CPU(ARM_TRADITIONAL) && > > > WTF_ARM_ARCH_AT_LEAST(5)) > > > #endif > > > > This will not work for non-ARM architectures, because there is no way to break > > the evaluation of logical expression in a #define. In x86 the #if > > ENABLE(JIT_OPTIMIZE_MOD) will turn into something similar: > > #if defined ENABLE_JIT_OPTIMIZE_MOD && CPU(ARM_TRADITIONAL) && > > WTF_ARM_ARCH_AT_LEAST(5), where WTF_ARM_ARCH_AT_LEAST is not defined. :( > > I think so, if CPU(ARM_TRADITIONAL) is false, WTF_ARM_ARCH_AT_LEAST will be > never evaluated in this expression. Ok, I misread that! Gabor, you are right.
Comment on attachment 48112 [details] Trampoline structure (use it first) Landed (http://trac.webkit.org/changeset/54362)
Created attachment 48207 [details] macro proposal Something like that ^^ can be used. Although it creates a new macro block (CPU optimizations), it looks better than the first approach. This change also implies to change armModFast functions name to something general (for example: softModulo).
Created attachment 48218 [details] ARM soft modulo with proposed changes
Created attachment 48222 [details] ARM soft modulo with proposed changes
Comment on attachment 48140 [details] Add a soft modulo operation to ARM JIT using a trampoline function (use it second). Cleared Gavin Barraclough's review+ from obsolete attachment 48140 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Attachment 48222 [details] is landed in r54534. All patches are landed, so I close this bug as fixed.