Bug 19271 – eliminate PIC branches by changing NaN handling in JSValue::toNumber

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Darin Adler]

Many function have PIC branches only because they call one of the many functions from JSValue that in turn call JSImmediate::toDouble, which gets NaN for the undefined and null cases. Getting the NaN value involves access to a global, and hence a PIC branch. Changing this to use a function, and one that's not inlined, would make code inside JavaScriptCore faster on Mac OS X. It would make code outside JavaScriptCore slower, because you'd still need a PIC branch just to call the function.

Ideas?

[Gavin Barraclough]

Created attachment 101688 [details]
The patch

=============================================================================

** TOTAL **:           1.005x as fast    244.5ms +/- 0.3%   243.2ms +/- 0.4%     significant

=============================================================================

[WebKit Review Bot]

Comment on attachment 101688 [details]
The patch

Attachment 101688 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/9209880

[Gavin Barraclough]

Fixed in r91555

[Kent Tamura]

(In reply to comment #3)
> Fixed in r91555

It broke build on Leopard, SnowLeopard, and Windows.

SnowLeopard:
Ld /Volumes/Big/slave/snowleopard-intel-debug/build/WebKitBuild/Debug/JavaScriptGlue.framework/Versions/A/JavaScriptGlue normal x86_64
    cd /Volumes/Big/slave/snowleopard-intel-debug/build/Source/JavaScriptGlue
    setenv MACOSX_DEPLOYMENT_TARGET 10.6
    /Developer/usr/bin/g++-4.2 -arch x86_64 -dynamiclib -L/Volumes/Big/slave/snowleopard-intel-debug/build/WebKitBuild/Debug -F/Volumes/Big/slave/snowleopard-intel-debug/build/WebKitBuild/Debug -filelist /Volumes/Big/slave/snowleopard-intel-debug/build/WebKitBuild/JavaScriptGlue.build/Debug/JavaScriptGlue.build/Objects-normal/x86_64/JavaScriptGlue.LinkFileList -Xlinker --no-demangle -exported_symbols_list JavaScriptGlue.exp -install_name /Volumes/Big/slave/snowleopard-intel-debug/build/WebKitBuild/Debug/JavaScriptGlue.framework/Versions/A/JavaScriptGlue -mmacosx-version-min=10.6 -dead_strip -framework CoreServices -framework JavaScriptCore -single_module -compatibility_version 1 -current_version 535.1 -o /Volumes/Big/slave/snowleopard-intel-debug/build/WebKitBuild/Debug/JavaScriptGlue.framework/Versions/A/JavaScriptGlue

Undefined symbols:
  "__ZNK3JSC7JSValue16toNumberSlowCaseEPNS_9ExecStateE", referenced from:
      __ZNK3JSC7JSValue8toNumberEPNS_9ExecStateE in JSUtils.o
ld: symbol(s) not found


Windows:

9>Linking...
9>   Creating library C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\lib\WebKit.lib and object C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\lib\WebKit.exp
9>NPRuntimeObjectMap.obj : error LNK2019: unresolved external symbol "private: double __thiscall JSC::JSValue::toNumberSlowCase(class JSC::ExecState *)const " (?toNumberSlowCase@JSValue@JSC@@ABENPAVExecState@2@@Z) referenced in function "public: double __thiscall JSC::JSValue::toNumber(class JSC::ExecState *)const " (?toNumber@JSValue@JSC@@QBENPAVExecState@2@@Z)
9>WebCore.lib(JSBindingsAllInOne.obj) : error LNK2001: unresolved external symbol "private: double __thiscall JSC::JSValue::toNumberSlowCase(class JSC::ExecState *)const " (?toNumberSlowCase@JSValue@JSC@@ABENPAVExecState@2@@Z)
9>WebCore.lib(DerivedSources.obj) : error LNK2001: unresolved external symbol "private: double __thiscall JSC::JSValue::toNumberSlowCase(class JSC::ExecState *)const " (?toNumberSlowCase@JSValue@JSC@@ABENPAVExecState@2@@Z)
9>WebCore.lib(c_utility.obj) : error LNK2001: unresolved external symbol "private: double __thiscall JSC::JSValue::toNumberSlowCase(class JSC::ExecState *)const " (?toNumberSlowCase@JSValue@JSC@@ABENPAVExecState@2@@Z)
9>C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\bin\WebKit.dll : fatal error LNK1120: 1 unresolved externals

[Ryosuke Niwa]

It appears that we just need to add __ZNK3JSC7JSValue16toNumberSlowCaseEPNS_9ExecStateE to JavaScriptCore.exp.  Will land a fix soon.