I plan to add two new entries to it, and we don't have enough space in the 32-bit int.
Created attachment 287161 [details] bench results
I ran sun spider for more iterations. Seems neutral: VMs tested: "og" at /Volumes/Data/WK/a/OpenSource/WebKitBuild/Release/jsc (r205031) "change" at /Volumes/Data/WK/b/OpenSource/WebKitBuild/Release/jsc (r205046) Collected 20 samples per benchmark/VM, with 20 VM invocations per benchmark. Emitted a call to gc() between sample measurements. Used 1 benchmark iteration per VM invocation for warm-up. Used the jsc-specific preciseTime() function to get microsecond-level timing. Reporting benchmark execution times with 95% confidence intervals in milliseconds. og change 3d-cube 4.4742+-0.1129 ? 4.8421+-0.5106 ? might be 1.0822x slower 3d-morph 4.8733+-0.1396 ? 4.9183+-0.1489 ? 3d-raytrace 4.7089+-0.1347 ? 4.8762+-0.2436 ? might be 1.0355x slower access-binary-trees 2.0261+-0.2069 ? 2.0385+-0.2315 ? access-fannkuch 5.4078+-0.2145 ^ 4.9936+-0.1585 ^ definitely 1.0829x faster access-nbody 2.3995+-0.2520 2.2865+-0.1149 might be 1.0494x faster access-nsieve 3.1811+-0.3418 ? 3.1835+-0.1785 ? bitops-3bit-bits-in-byte 1.0306+-0.0917 1.0004+-0.0270 might be 1.0301x faster bitops-bits-in-byte 2.4764+-0.1477 ? 2.5496+-0.1401 ? might be 1.0296x slower bitops-bitwise-and 1.9476+-0.1216 1.9002+-0.0419 might be 1.0249x faster bitops-nsieve-bits 2.9961+-0.1524 ? 3.0250+-0.1350 ? controlflow-recursive 2.3993+-0.3184 2.1664+-0.0488 might be 1.1075x faster crypto-aes 4.1460+-0.1466 ? 4.5112+-0.3820 ? might be 1.0881x slower crypto-md5 2.6766+-0.2089 2.6467+-0.1506 might be 1.0113x faster crypto-sha1 2.5983+-0.0933 2.5894+-0.0765 date-format-tofte 6.5434+-0.3718 6.2648+-0.1982 might be 1.0445x faster date-format-xparb 4.6716+-0.5373 4.4075+-0.0530 might be 1.0599x faster math-cordic 2.6840+-0.1437 ? 2.7348+-0.2634 ? might be 1.0189x slower math-partial-sums 3.8651+-0.1442 3.8397+-0.0633 math-spectral-norm 1.9297+-0.0868 1.8879+-0.1242 might be 1.0221x faster regexp-dna 6.2257+-0.1885 6.0042+-0.0970 might be 1.0369x faster string-base64 4.3364+-0.5977 3.8230+-0.1789 might be 1.1343x faster string-fasta 5.2875+-0.1719 ? 5.4146+-0.2049 ? might be 1.0240x slower string-tagcloud 8.4890+-0.7232 7.9107+-0.2256 might be 1.0731x faster string-unpack-code 17.6484+-0.4734 17.3723+-0.3443 might be 1.0159x faster string-validate-input 3.8912+-0.1658 ? 3.9788+-0.2443 ? might be 1.0225x slower <arithmetic> 4.3428+-0.0622 4.2756+-0.0469 might be 1.0157x faster
(In reply to comment #1) > Created attachment 287161 [details] > bench results Note that nothing changed in JSC between r205031 and r205046. Also, octane/zlib is crashing for reasons unknown to me. I'm filing a bug.
Ok, I was running a stale octane. It wasn't crashing. It had a syntax error. Here is a new run of octane. Collected 4 samples per benchmark/VM, with 4 VM invocations per benchmark. Emitted a call to gc() between sample measurements. Used 1 benchmark iteration per VM invocation for warm-up. Used the jsc-specific preciseTime() function to get microsecond-level timing. Reporting benchmark execution times with 95% confidence intervals in milliseconds. og change encrypt 0.14933+-0.00151 ? 0.14993+-0.00175 ? decrypt 2.50409+-0.01647 ? 2.52090+-0.02592 ? deltablue x2 0.12672+-0.00766 0.12368+-0.00276 might be 1.0246x faster earley 0.27113+-0.00582 0.27080+-0.00154 boyer 4.64829+-0.31196 4.48091+-0.27852 might be 1.0374x faster navier-stokes x2 4.63976+-0.06540 ? 4.64351+-0.08296 ? raytrace x2 0.75674+-0.00699 0.74796+-0.00414 might be 1.0117x faster richards x2 0.07671+-0.00295 0.07664+-0.00293 splay x2 0.32629+-0.00423 ? 0.32771+-0.00363 ? regexp x2 15.82487+-0.47393 ? 16.07022+-0.61532 ? might be 1.0155x slower pdfjs x2 35.63338+-1.05705 ? 36.21936+-0.34584 ? might be 1.0164x slower mandreel x2 39.41680+-0.41283 ? 39.71157+-0.30214 ? gbemu x2 27.84621+-0.47470 ? 27.90936+-0.49965 ? closure 0.46175+-0.00772 ? 0.46655+-0.00507 ? might be 1.0104x slower jquery 6.28443+-0.11179 ? 6.36670+-0.15409 ? might be 1.0131x slower box2d x2 8.59893+-0.11928 ? 8.67511+-0.04550 ? zlib x2 331.46051+-21.39808 ? 335.75600+-19.48167 ? might be 1.0130x slower typescript x2 576.16992+-14.91424 573.33197+-15.50176 <geometric> 4.72847+-0.02567 ? 4.73646+-0.03637 ? might be 1.0017x slower
Created attachment 287164 [details] patch
Comment on attachment 287164 [details] patch r=me
Attachment 287164 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:42: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:44: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:46: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 5 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
This has a nice tail of things that need to be changed for 32-bit. Doing that now, and I'll upload a new patch for review.
Basically, DFGNode must grow by 4 bytes on 32-bits to make this work.
Created attachment 287183 [details] patch Up for review again since this changed quite a bit.
Comment on attachment 287183 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=287183&action=review > Source/JavaScriptCore/dfg/DFGNode.h:2342 > + struct OpInfoWrapper { I wonder if I should just turn OpInfo into this Struct. Thoughts?
Attachment 287183 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGNode.h:2361: uint32_t is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/dfg/DFGNode.h:2362: int32_t is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/dfg/DFGNode.h:2363: uint64_t is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:42: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:44: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:46: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 8 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Oh man, debug builds are broken. Let me fix that.
(In reply to comment #11) > Comment on attachment 287183 [details] > patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=287183&action=review > > > Source/JavaScriptCore/dfg/DFGNode.h:2342 > > + struct OpInfoWrapper { > > I wonder if I should just turn OpInfo into this Struct. Thoughts? Fil, Ben, any thoughts on this?
View in context: https://bugs.webkit.org/attachment.cgi?id=287183&action=review > Source/JavaScriptCore/dfg/DFGNode.h:-559 > - m_opInfo = bitwise_cast<intptr_t>(data); The operator=(void*) does not work here? > Source/JavaScriptCore/dfg/DFGNode.h:570 > + m_opInfo = nullptr; > + m_opInfo2 = nullptr; m_opInfo.clear()? or m_opInfo = OpInfoWrapper() ? > Source/JavaScriptCore/dfg/DFGNode.h:2279 > + return static_cast<BasicBlockLocation*>(m_opInfo.u.pointer); What do you think of: m_opInfo->as<BasicBlockLocation*>() instead of casts at the call site? > Source/JavaScriptCore/dfg/DFGPromotedHeapLocation.h:130 > + uint64_t m_info; Why is this switched to 64bits?
(In reply to comment #15) > View in context: > https://bugs.webkit.org/attachment.cgi?id=287183&action=review > > > Source/JavaScriptCore/dfg/DFGNode.h:-559 > > - m_opInfo = bitwise_cast<intptr_t>(data); > > The operator=(void*) does not work here? > > > Source/JavaScriptCore/dfg/DFGNode.h:570 > > + m_opInfo = nullptr; > > + m_opInfo2 = nullptr; > > m_opInfo.clear()? > or > m_opInfo = OpInfoWrapper() > ? I'll do m_opInfo = OpInfoWrapper() > > > Source/JavaScriptCore/dfg/DFGNode.h:2279 > > + return static_cast<BasicBlockLocation*>(m_opInfo.u.pointer); > > What do you think of: > m_opInfo->as<BasicBlockLocation*>() > instead of casts at the call site? Yeah, I was thinking of something like that as I was writing all those casts. I'll do that. What do you think about the asPtr<> instead of as<>? > > > Source/JavaScriptCore/dfg/DFGPromotedHeapLocation.h:130 > > + uint64_t m_info; > > Why is this switched to 64bits? I guess I wasn't sure if it was being used as 64 bits (it's not), and just to be consistent and prevent gotchas in the future, I wanted to make it 32-bits. I guess I can keep it as 32-bit.
Created attachment 287239 [details] patch for landing
Attachment 287239 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGNode.h:2392: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGNode.h:2397: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGNode.h:2402: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:42: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:44: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:46: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 8 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 287240 [details] patch for landing Trying to appease non Darwin ports.
Attachment 287240 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGNode.h:2387: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGNode.h:2392: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGNode.h:2397: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:40: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:41: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:42: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:44: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/JavaScriptCore/dfg/DFGOpInfo.h:46: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] Total errors found: 8 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 287240 [details] patch for landing Clearing flags on attachment: 287240 Committed r205107: <http://trac.webkit.org/changeset/205107>
All reviewed patches have been landed. Closing bug.
GTK port fails with the debug assertion. I think the reason is the following. Now the struct layout of the AbstractValue becomes changed. Before this change, AbstractValue was dense. But now padding is inserted due to 64bit SpeculatedType. While the other fileds are initialized with zeros correctly, this padding area is not ensured that it is zeros. Looking.