Bug 28294 - Devirtualise marking
: Devirtualise marking
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-08-13 22:07 PDT by Oliver Hunt
Modified: 2009-08-19 13:08 PDT (History)
1 user (show)

See Also:


Attachments
Patch v1 (28.99 KB, patch)
2009-08-13 22:14 PDT, Oliver Hunt
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2009-08-13 22:07:52 PDT
Marking currently uses virtual methods, but in the general case these virtual calls are unnecessary and can be elided.
Comment 1 Oliver Hunt 2009-08-13 22:14:51 PDT
Created attachment 34809 [details]
Patch v1
Comment 2 Maciej Stachowiak 2009-08-13 22:19:29 PDT
Comment on attachment 34809 [details]
Patch v1

r=me but consider reformatting the long inline method definitions as suggested.
Comment 3 Oliver Hunt 2009-08-13 22:35:48 PDT
Committed r47267
Comment 4 Gabor Loki 2009-08-18 06:07:09 PDT
It looks like this patch breaks the ARM JIT port at r47269 with ENABLE_YARR=1 ENABLE_YARR_JIT=1 ENABLE_JIT=1 WTF_USE_JSVALUE32=1.

There is a regression at JavaScriptCore/tests/mozilla/ecma/Array/15.4.4.4-1.js .

The GDB backtrace says:
#0  JSC::JSValue::isGetterSetter (this=0x4007ea4c) at JavaScriptCore/runtime/JSCell.h:186
#1  callDefaultValueFunction (exec=0x426a004c, object=0x428b2660, propertyName=@0x2e4320) at JavaScriptCore/runtime/JSObject.cpp:218
#2  JSC::JSObject::defaultValue (this=0x428b2660, exec=0x426a004c, hint=JSC::NoPreference) at JavaScriptCore/runtime/JSObject.cpp:245
#3  JSC::JSObject::toPrimitive (this=0x428b2660, exec=0x426a004c, preferredType=JSC::NoPreference)
    at JavaScriptCore/runtime/JSObject.h:538
#4  JSC::JSValue::toPrimitive (this=0x4007eae8, exec=0x426a004c, preferredType=JSC::NoPreference)
    at JavaScriptCore/runtime/JSCell.h:261
#5  cti_op_to_primitive (args=0x4007eb04) at JavaScriptCore/jit/JITStubs.cpp:2786
#6  ctiTrampoline ()

Do you have any hint or thought how to fix it?
Comment 5 Gabor Loki 2009-08-19 07:45:23 PDT
The bug can be reproduced on ARM with interpreter and jit as well. The situation is the same with debug and with release mode also. Does iPhone run correctly?
Comment 6 Oliver Hunt 2009-08-19 10:45:51 PDT
(In reply to comment #5)
> The bug can be reproduced on ARM with interpreter and jit as well. The
> situation is the same with debug and with release mode also. Does iPhone run
> correctly?

I just did a trivial amount of debugging myself, and forced JSVALUE_32 on x86 and this crash occurred.  Given that's the only substantive difference between ARM and x86 build settings that seems obvious and demonstrates the bug.
Comment 7 Oliver Hunt 2009-08-19 13:08:42 PDT
JSVALUE32 fix landed in r47522