I started this work some weeks ago, but I had to go abroad, and only recently find time to finnish the first patch. Notes: - JSVALUE32_64 is not enabled by default - and all optimizations are disabled (as usual for all enhancements). - emitPutJITStub... functions are ugly. Somehow stackIndexStart should be propagated from JITStubCall.h. Besides stackIndexStart should be machine dependent constant, defined by the assembler - emit_op_div contains x86 code, not looks nice. And still not found a nice solution for branchDouble with NaN arguments. x86 says anything compared to NaN is <= (set both equal and carry flag). Now branchDouble on ARM has an x86 unordered flag (default true), which just a partial solution, since it sets only the equal flag in case of comparing to NaN. Do you know what ECMA says about double comparison?
Created attachment 42365 [details] Proposed patch for JSValue32_64 on ARM
Comment on attachment 42365 [details] Proposed patch for JSValue32_64 on ARM As per our conversation on IRC, It will help this change to clean up some of the existing code in svn first, so let's hold this off for: https://bugs.webkit.org/show_bug.cgi?id=31104 to land first. I'm going to r- this patch, since these changes will conflict, please resubmit after 31104 is in & you've updated. ECMA says that math is IEEE754 compliant, and IEEE754 specifies that comparisons between unordered operands (i.e. when either operand is NaN) evaluates to false. The MacroAssembler interface in ToT is a mess in its handling of unordered operands, I've addressed this in the patch on bug 31104 by making the required handling of unordered operands explicit in the DoubleCondition. r-, will review a new patch post 31104.
Created attachment 42566 [details] Second patch
Comment on attachment 42566 [details] Second patch As discussed in irc, I've landed a version of the OBJECT_OFFSETOF(struct JITStackFrame, args) change in r50594. Also, I've changed the shift behaviour in r50595, so the extra and32's on the shifts should no longer be required. Also as discussed in irc, some ASSERTs to guard THUNK_RETURN_ADDRESS_OFFSET would be a great improvement. The empty '#elif PLATFORM(ARM_TRADITIONAL)' in JIT.h was a little confusing, would be great to add a quick comment explaining why this section is empty (something like "// patchOffset... values should go here; JIT optimizations currently not supported on ARM with JSVALUE32_64."). Otherwise looks good to go, r- since you'll need to update to r50594, r50595, but should be a straight r+ after that.
Created attachment 42648 [details] Another patch The patch contains buildfix for lshift/rsfhift. (rshiftPtr is not exists anymore) Reason: our commiter was away, and on the #irc channel no-one helped us to commit the fix.
Comment on attachment 42648 [details] Another patch Hey Zoltan, I haven't spotted an ASSERT / COMPILE_ASSERT guarding that THUNK_RETURN_ADDRESS_OFFSET matches the offset of thunkReturnAddress, did that get missed? Also, could we rename STRINGIZE_HELPER to STRINGIZE, and STRINGIZE to something like STRINGIZE_VALUE, STRINGIZE_VALUE_OF, STRINGIZE_PREPROCESSED. I think the STRINGIZE macro should match the functionality of the stringize operator (#), in just stringifying the token passed to it, and this is a useful macro in its own right. The version that doubly nests the macro to extract the value (if it is a preprocessor token) is doing something more than the standard stringize functionality, and I think it needs calling out as such. Ah, now i see also the comment in JIT.h is also missing - so I'm guessing you just hadn't got around to fixing the last review comments yet. :-)
Created attachment 42737 [details] Never ending story
Created attachment 42750 [details] First patch for JSValue32_64 optimizations
(In reply to comment #8) > Created an attachment (id=42750) [details] > First patch for JSValue32_64 optimizations Preliminary results: SunSpider: 7442.9ms +/- 0.2% (was: 9546 ms, 28% speedup) V8: 35815.8ms +/- 0.4% (was: 22866 ms, 56% slowdown) WindScorpion: N/A
Comment on attachment 42737 [details] Never ending story Awesome, r+.
Comment on attachment 42750 [details] First patch for JSValue32_64 optimizations All looks good to me, r+.
Comment on attachment 42737 [details] Never ending story Landed in 50981. http://trac.webkit.org/changeset/50981 Clearing review flag.
Comment on attachment 42750 [details] First patch for JSValue32_64 optimizations Landed in 50982. http://trac.webkit.org/changeset/50982 Clearing review flag.
Created attachment 43285 [details] Minor fixes for JSValue32_64 on ARM
Comment on attachment 43285 [details] Minor fixes for JSValue32_64 on ARM Landed in 51067. http://trac.webkit.org/changeset/51067 Clearing review flag.
All reviewed patches have been landed. Closing bug.