19271 2008-05-27 11:45:04 -0700 eliminate PIC branches by changing NaN handling in JSValue::toNumber 2011-07-21 23:25:25 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Mac OS X 10.5 RESOLVED FIXED P2 Normal --- 1 darin webkit-unassigned barraclough darin ggaren mjs oliver rniwa tkent oldest_to_newest 81625 0 darin 2008-05-27 11:45:04 -0700 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? 440794 1 101688 barraclough 2011-07-21 19:28:16 -0700 Created attachment 101688 The patch ============================================================================= ** TOTAL **: 1.005x as fast 244.5ms +/- 0.3% 243.2ms +/- 0.4% significant ============================================================================= 440819 2 101688 webkit.review.bot 2011-07-21 20:29:23 -0700 Comment on attachment 101688 The patch Attachment 101688 did not pass mac-ews (mac): Output: http://queues.webkit.org/results/9209880 440842 3 barraclough 2011-07-21 22:44:36 -0700 Fixed in r91555 440848 4 tkent 2011-07-21 22:56:17 -0700 (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 440853 5 rniwa 2011-07-21 23:25:25 -0700 It appears that we just need to add __ZNK3JSC7JSValue16toNumberSlowCaseEPNS_9ExecStateE to JavaScriptCore.exp. Will land a fix soon. 101688 2011-07-21 19:28:16 -0700 2011-07-21 20:29:23 -0700 The patch 19271.patch text/plain 3139 barraclough SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gOTE1NDUpCisrKyBTb3VyY2Uv SmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjIgQEAK KzIwMTEtMDctMjEgIEdhdmluIEJhcnJhY2xvdWdoICA8YmFycmFjbG91Z2hAYXBwbGUuY29tPgor CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTI3MQor ICAgICAgICBlbGltaW5hdGUgUElDIGJyYW5jaGVzIGJ5IGNoYW5naW5nIE5hTiBoYW5kbGluZyBp biBKU1ZhbHVlOjp0b051bWJlcgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIE1vdmluZyB0aGUgbm9uLW51bWVyaWMgY2FzZXMgb3V0IG9mIGxpbmUgc2Vl bXMgdG8gYmUgYSBjb25zaXN0ZW50CisgICAgICAgIHdpbiBvbiBTdW5TcGlkZXIgZm9yIG1lLCB0 byB0aGUgb3JkZXIgb2YgYWJvdXQgMC41JS4KKworICAgICAgICAqIHJ1bnRpbWUvSlNDZWxsLmg6 CisgICAgICAgIChKU0M6OkpTQ2VsbDo6SlNWYWx1ZTo6dG9OdW1iZXIpOgorICAgICAgICAgICAg LSBDaGFuZ2VkIHRvIG9ubHkgaGFuZGxlIHZhbHVlcyB0aGF0IGFyZSBhbHJlYWR5IG51bWJlcnMs IG1vY2Ugbm9uLW51bWVyaWMgY2FzZXMgb3V0IG9mIGxpbmUuCisgICAgICAgICogcnVudGltZS9K U1ZhbHVlLmNwcDoKKyAgICAgICAgKEpTQzo6SlNWYWx1ZTo6dG9OdW1iZXJTbG93Q2FzZSk6Cisg ICAgICAgICAgICAtIEFkZGVkIHRvTnVtYmVyU2xvd0Nhc2UsIGhhbmRsaW5nIG5vbi1udW1lcmlj IGNhc2VzLgorICAgICAgICAqIHJ1bnRpbWUvSlNWYWx1ZS5oOgorICAgICAgICAgICAgLSBBZGQg ZGVjbGFyYXRpb24gb2YgdG9OdW1iZXJTbG93Q2FzZS4KKwogMjAxMS0wNy0yMSAgR2F2aW4gQmFy cmFjbG91Z2ggIDxiYXJyYWNsb3VnaEBhcHBsZS5jb20+CiAKICAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTY0OTAwCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlw dENvcmUvcnVudGltZS9KU0NlbGwuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENv cmUvcnVudGltZS9KU0NlbGwuaAkocmV2aXNpb24gOTE0MjkpCisrKyBTb3VyY2UvSmF2YVNjcmlw dENvcmUvcnVudGltZS9KU0NlbGwuaAkod29ya2luZyBjb3B5KQpAQCAtMzI0LDExICszMjQsNyBA QCBuYW1lc3BhY2UgSlNDIHsKICAgICAgICAgICAgIHJldHVybiBhc0ludDMyKCk7CiAgICAgICAg IGlmIChpc0RvdWJsZSgpKQogICAgICAgICAgICAgcmV0dXJuIGFzRG91YmxlKCk7Ci0gICAgICAg IGlmIChpc0NlbGwoKSkKLSAgICAgICAgICAgIHJldHVybiBhc0NlbGwoKS0+dG9OdW1iZXIoZXhl Yyk7Ci0gICAgICAgIGlmIChpc1RydWUoKSkKLSAgICAgICAgICAgIHJldHVybiAxLjA7Ci0gICAg ICAgIHJldHVybiBpc1VuZGVmaW5lZCgpID8gc3RkOjpudW1lcmljX2xpbWl0czxkb3VibGU+Ojpx dWlldF9OYU4oKSA6IDA7IC8vIG51bGwgYW5kIGZhbHNlIGJvdGggY29udmVydCB0byAwLgorICAg ICAgICByZXR1cm4gdG9OdW1iZXJTbG93Q2FzZShleGVjKTsKICAgICB9CiAKICAgICBpbmxpbmUg SlNWYWx1ZSBKU1ZhbHVlOjpnZXRKU051bWJlcigpCkluZGV4OiBTb3VyY2UvSmF2YVNjcmlwdENv cmUvcnVudGltZS9KU1ZhbHVlLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvSmF2YVNjcmlwdENv cmUvcnVudGltZS9KU1ZhbHVlLmNwcAkocmV2aXNpb24gOTE0MjkpCisrKyBTb3VyY2UvSmF2YVNj cmlwdENvcmUvcnVudGltZS9KU1ZhbHVlLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNTQsNiArNTQs MTYgQEAgZG91YmxlIEpTVmFsdWU6OnRvSW50ZWdlclByZXNlcnZlTmFOKEV4ZQogICAgIHJldHVy biB0cnVuYyh0b051bWJlcihleGVjKSk7CiB9CiAKK2RvdWJsZSBKU1ZhbHVlOjp0b051bWJlclNs b3dDYXNlKEV4ZWNTdGF0ZSogZXhlYykgY29uc3QKK3sKKyAgICBBU1NFUlQoIWFzSW50MzIoKSAm JiAhaXNEb3VibGUoKSk7CisgICAgaWYgKGlzQ2VsbCgpKQorICAgICAgICByZXR1cm4gYXNDZWxs KCktPnRvTnVtYmVyKGV4ZWMpOworICAgIGlmIChpc1RydWUoKSkKKyAgICAgICAgcmV0dXJuIDEu MDsKKyAgICByZXR1cm4gaXNVbmRlZmluZWQoKSA/IHN0ZDo6bnVtZXJpY19saW1pdHM8ZG91Ymxl Pjo6cXVpZXRfTmFOKCkgOiAwOyAvLyBudWxsIGFuZCBmYWxzZSBib3RoIGNvbnZlcnQgdG8gMC4K K30KKwogSlNPYmplY3QqIEpTVmFsdWU6OnRvT2JqZWN0U2xvd0Nhc2UoRXhlY1N0YXRlKiBleGVj LCBKU0dsb2JhbE9iamVjdCogZ2xvYmFsT2JqZWN0KSBjb25zdAogewogICAgIEFTU0VSVCghaXND ZWxsKCkpOwpJbmRleDogU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvSlNWYWx1ZS5oCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0pTVmFsdWUuaAkocmV2 aXNpb24gOTE0MjkpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9KU1ZhbHVlLmgJ KHdvcmtpbmcgY29weSkKQEAgLTIzNiw2ICsyMzYsNyBAQCBuYW1lc3BhY2UgSlNDIHsKICAgICAg ICAgSlNWYWx1ZShIYXNoVGFibGVEZWxldGVkVmFsdWVUYWcpOwogCiAgICAgICAgIGlubGluZSBj b25zdCBKU1ZhbHVlIGFzVmFsdWUoKSBjb25zdCB7IHJldHVybiAqdGhpczsgfQorICAgICAgICBk b3VibGUgdG9OdW1iZXJTbG93Q2FzZShFeGVjU3RhdGUqKSBjb25zdDsKICAgICAgICAgSlNPYmpl Y3QqIHRvT2JqZWN0U2xvd0Nhc2UoRXhlY1N0YXRlKiwgSlNHbG9iYWxPYmplY3QqKSBjb25zdDsK ICAgICAgICAgSlNPYmplY3QqIHRvVGhpc09iamVjdFNsb3dDYXNlKEV4ZWNTdGF0ZSopIGNvbnN0 OwogCg==