Bug 75812

Summary:

JSC should be a triple-tier VM

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aroben, gustavo, hausmann, loki, morrita, mrobinson, ossy, pvarga, rakuco, sam, webkit.review.bot, wingo, xan.lopez, zherczeg

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

78791

Bug Blocks:

Attachments:

Description	Flags
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	gyuyoung.kim: commit-queue-
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
work in progress	none
the patch	none
the patch	webkit-ews: commit-queue-
the patch	gyuyoung.kim: commit-queue-
the patch	gyuyoung.kim: commit-queue-
the patch	none
the patch	gyuyoung.kim: commit-queue-
the patch	none
the patch	none
the patch	gyuyoung.kim: commit-queue-
the patch	gyuyoung.kim: commit-queue-
the patch	none
LLIntAssembly.h	none
the patch	none
the patch, sort of	none
the patch	none
the patch	none
the patch	none
the patch	none
work in progress	none
the patch	none
the patch	none
the patch	none
the patch	none
the patch	gyuyoung.kim: commit-queue-
the patch	none
the patch	none
the patch	none
the patch	none
the patch	none
the patch	barraclough: review+
the patch	none
the patch	none
the patch	none
fix for elf	none
fix for non-DFG build and DFG crashes	ossy: review+
the patch	none

Filip Pizlo

Reported 2012-01-08 16:28:43 PST

This is a work in progress.

Attachments
work in progress (222.76 KB, patch) 2012-01-08 16:32 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (248.18 KB, patch) 2012-01-08 18:09 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (251.76 KB, patch) 2012-01-08 21:14 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (261.77 KB, patch) 2012-01-08 23:36 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (269.23 KB, patch) 2012-01-09 16:00 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (287.13 KB, patch) 2012-01-09 17:29 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (291.30 KB, patch) 2012-01-10 01:09 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (314.57 KB, patch) 2012-01-10 14:45 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (327.44 KB, patch) 2012-01-10 17:09 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (349.30 KB, patch) 2012-01-11 17:31 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (372.48 KB, patch) 2012-01-11 21:07 PST, Filip Pizlo	gyuyoung.kim: commit-queue-	Details Formatted Diff Diff
work in progress (378.12 KB, patch) 2012-01-11 22:37 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (384.46 KB, patch) 2012-01-12 14:24 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (386.13 KB, patch) 2012-01-12 22:55 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (386.76 KB, patch) 2012-01-16 18:02 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (387.07 KB, patch) 2012-01-17 17:08 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (394.86 KB, patch) 2012-01-17 22:26 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (397.64 KB, patch) 2012-01-18 17:16 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (397.56 KB, patch) 2012-01-18 19:46 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (400.87 KB, patch) 2012-01-18 21:15 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (406.86 KB, patch) 2012-01-19 01:16 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (421.21 KB, patch) 2012-01-22 21:46 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (423.69 KB, patch) 2012-01-23 15:23 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (424.95 KB, patch) 2012-01-23 22:59 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (425.81 KB, patch) 2012-01-24 12:32 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (422.94 KB, patch) 2012-01-24 15:08 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (427.56 KB, patch) 2012-01-24 17:28 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (427.56 KB, patch) 2012-01-24 17:33 PST, Filip Pizlo	webkit-ews: commit-queue-	Details Formatted Diff Diff
the patch (429.96 KB, patch) 2012-01-24 21:01 PST, Filip Pizlo	gyuyoung.kim: commit-queue-	Details Formatted Diff Diff
the patch (433.25 KB, patch) 2012-01-25 10:27 PST, Filip Pizlo	gyuyoung.kim: commit-queue-	Details Formatted Diff Diff
the patch (434.77 KB, patch) 2012-01-26 15:48 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (426.52 KB, patch) 2012-01-26 15:54 PST, Filip Pizlo	gyuyoung.kim: commit-queue-	Details Formatted Diff Diff
the patch (431.69 KB, patch) 2012-01-26 17:37 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (431.62 KB, patch) 2012-01-26 18:03 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (430.90 KB, patch) 2012-01-26 20:12 PST, Filip Pizlo	gyuyoung.kim: commit-queue-	Details Formatted Diff Diff
the patch (432.12 KB, patch) 2012-01-27 13:37 PST, Filip Pizlo	gyuyoung.kim: commit-queue-	Details Formatted Diff Diff
the patch (436.06 KB, patch) 2012-01-27 14:42 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
LLIntAssembly.h (215.58 KB, text/plain) 2012-01-27 14:43 PST, Filip Pizlo	no flags	Details
the patch (436.23 KB, patch) 2012-01-27 14:50 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch, sort of (441.30 KB, patch) 2012-01-28 00:41 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (448.98 KB, patch) 2012-01-29 17:37 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (453.70 KB, patch) 2012-01-29 22:00 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (464.41 KB, patch) 2012-01-30 15:12 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (464.85 KB, patch) 2012-01-30 15:56 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
work in progress (470.50 KB, patch) 2012-01-30 18:07 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (479.20 KB, patch) 2012-01-31 19:28 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (479.23 KB, patch) 2012-01-31 20:14 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (479.61 KB, patch) 2012-01-31 23:23 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (478.32 KB, patch) 2012-02-01 15:41 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (495.62 KB, patch) 2012-02-01 23:55 PST, Filip Pizlo	gyuyoung.kim: commit-queue-	Details Formatted Diff Diff
the patch (495.90 KB, patch) 2012-02-02 00:37 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (475.64 KB, patch) 2012-02-02 13:57 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (476.52 KB, patch) 2012-02-02 14:53 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (475.76 KB, patch) 2012-02-06 15:17 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (491.21 KB, patch) 2012-02-06 17:18 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (490.42 KB, patch) 2012-02-14 11:19 PST, Filip Pizlo	barraclough: review+	Details Formatted Diff Diff
the patch (495.55 KB, patch) 2012-02-18 11:10 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (501.74 KB, patch) 2012-02-18 12:07 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (501.73 KB, patch) 2012-02-20 09:15 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
fix for elf (4.44 KB, patch) 2012-02-21 01:04 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
fix for non-DFG build and DFG crashes (2.19 KB, patch) 2012-02-21 01:33 PST, Filip Pizlo	ossy: review+	Details Formatted Diff Diff
the patch (499.12 KB, patch) 2012-02-21 12:15 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
Show Obsolete (60) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2012-01-08 16:29:57 PST

<rdar://problem/10079694>

Filip Pizlo

Comment 2 2012-01-08 16:32:56 PST

Created attachment 121601 [details] work in progress Doesn't work yet, not ready for review.

Filip Pizlo

Comment 3 2012-01-08 18:09:50 PST

Created attachment 121607 [details] work in progress Added a chunk of the build logic, but not yet all of it.

Filip Pizlo

Comment 4 2012-01-08 21:14:45 PST

Created attachment 121617 [details] work in progress It's starting to build, sort of.

Sam Weinig

Comment 5 2012-01-08 21:38:34 PST

Triple? Meh. Let's go to five -- http://onion.com/bWBRid.

Filip Pizlo

Comment 6 2012-01-08 21:39:42 PST

(In reply to comment #5) > Triple? Meh. Let's go to five -- http://onion.com/bWBRid. I can't argue with you on that one. ;-)

Filip Pizlo

Comment 7 2012-01-08 23:36:38 PST

Created attachment 121624 [details] work in progress Wrote more glue.

Filip Pizlo

Comment 8 2012-01-09 16:00:49 PST

Created attachment 121744 [details] work in progress Getting closer to having it build.

Filip Pizlo

Comment 9 2012-01-09 17:29:19 PST

Created attachment 121769 [details] work in progress It now compiles. But does not link.

Filip Pizlo

Comment 10 2012-01-10 01:09:22 PST

Created attachment 121809 [details] work in progress It compiles and links but there is still a tiny bit of glue to write.

Filip Pizlo

Comment 11 2012-01-10 14:45:09 PST

Created attachment 121915 [details] work in progress Rebased, and started to work on the yucky glue.

Filip Pizlo

Comment 12 2012-01-10 17:09:11 PST

Created attachment 121946 [details] work in progress Call linking is fixed up. But still more work to do.

Filip Pizlo

Comment 13 2012-01-11 17:31:35 PST

Created attachment 122140 [details] work in progress

Filip Pizlo

Comment 14 2012-01-11 21:07:34 PST

Created attachment 122163 [details] work in progress Did another rebase and wrote most (all?) of the remaining glue that is needed to get this to work. Not sure if it builds at this point. And I'm pretty sure that if it does, it'll crash in some humorous way.

WebKit Review Bot

Comment 15 2012-01-11 21:11:26 PST

Attachment 122163 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Last 3072 characters of output: ec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:114: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:120: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:121: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:122: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1087: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1120: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:43: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:38: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:44: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:46: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:52: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:54: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:65: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:27: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:28: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 27 in 79 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 16 2012-01-11 21:11:34 PST

Comment on attachment 122163 [details] work in progress Clearing r? because I didn't mean to set it.

Gyuyoung Kim

Comment 17 2012-01-11 21:25:14 PST

Comment on attachment 122163 [details] work in progress Attachment 122163 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11213281

Early Warning System Bot

Comment 18 2012-01-11 21:31:32 PST

Comment on attachment 122163 [details] work in progress Attachment 122163 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11222238

Filip Pizlo

Comment 19 2012-01-11 22:37:51 PST

Created attachment 122172 [details] work in progress It just ran "hello". [pizlo@nitroflex OpenSource] WebKitBuild/Debug/jsc hello.js hello! [pizlo@nitroflex OpenSource]

Filip Pizlo

Comment 20 2012-01-12 14:24:45 PST

Created attachment 122310 [details] work in progress testapi works as do a bunch of random tests (eval, call caching, property access caching, global variables, printing, arithmetic, string concatenation, and other stuff), but run-javascriptcore-tests still fails. But progress is being made.

Filip Pizlo

Comment 21 2012-01-12 22:55:11 PST

Created attachment 122371 [details] work in progress Tons of bug fixes. It's starting to work, but there are still test failures.

Filip Pizlo

Comment 22 2012-01-16 18:02:26 PST

Created attachment 122701 [details] work in progress Fixed loads more regressions, but there's still more to go!

Filip Pizlo

Comment 23 2012-01-17 17:08:31 PST

Created attachment 122841 [details] work in progress It can now run javascriptcore-tests, SunSpider, V8, and Kraken. Backing up before I rebase.

Filip Pizlo

Comment 24 2012-01-17 22:26:11 PST

Created attachment 122872 [details] work in progress This now runs SunSpider, V8, and Kraken in release mode. It turns out that there was a bug in the offline assembler's interaction with the system linker, where the assembler's use of non-local but non-global labels (i.e. labels beginning with '_' but lacking a '.globl') was causing a hilarious bug: an entire snippet of code between two such labels disappeared sometime between when the inline assembly was passed to the C++ compiler and when it was loaded into memory when the binary ran! Anyway, here's the performance of just the low-level interpreter (LLInt) without tiering support. Remaining work includes: - Running layout tests. Not anticipating anything big there. - Enabling tiering and enjoying the bug explosion. Benchmark report for SunSpider, V8, and Kraken on oldmac (MacPro4,1). VMs tested: "TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc (r105216) "LLInt" at /Volumes/Data/pizlo/senary/OpenSource/WebKitBuild/Release/jsc (r105216) "OldInt" at /Volumes/Data/pizlo/septenary/OpenSource/WebKitBuild/Release/jsc (r105216) Collected 12 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. TipOfTree LLInt OldInt OldInt v. TipOfTree SunSpider: 3d-cube 7.6638+-0.0505 ! 20.0177+-0.0229 ! 51.2581+-0.0747 ! definitely 6.6884x slower 3d-morph 14.0859+-0.0497 ! 22.2417+-0.0705 ! 63.8518+-0.1799 ! definitely 4.5330x slower 3d-raytrace 11.8198+-0.0278 ! 32.3048+-0.2806 ! 57.2185+-0.0874 ! definitely 4.8409x slower access-binary-trees 2.2516+-0.0116 ! 11.5333+-0.0726 ! 17.5089+-0.0268 ! definitely 7.7761x slower access-fannkuch 11.4253+-0.0212 ! 51.4479+-1.7268 ! 119.4145+-0.4435 ! definitely 10.4517x slower access-nbody 6.8891+-0.0643 ! 22.1227+-0.0842 ! 56.7224+-0.0504 ! definitely 8.2336x slower access-nsieve 3.8053+-0.0326 ! 10.4458+-0.0395 ! 25.1232+-0.0232 ! definitely 6.6021x slower bitops-3bit-bits-in-byte 1.5363+-0.0249 ! 20.9110+-0.0530 ! 42.8032+-0.0428 ! definitely 27.8613x slower bitops-bits-in-byte 6.0055+-0.0246 ! 27.4117+-0.7046 ! 44.8535+-0.1989 ! definitely 7.4687x slower bitops-bitwise-and 4.6773+-0.0433 ! 22.7415+-0.1085 ! 47.1811+-0.0167 ! definitely 10.0871x slower bitops-nsieve-bits 8.2503+-0.0383 ! 25.2988+-0.0695 ! 60.1579+-0.0419 ! definitely 7.2916x slower controlflow-recursive 3.5350+-0.0092 ! 12.4984+-0.0564 ! 26.1094+-0.1379 ! definitely 7.3861x slower crypto-aes 11.2509+-0.1880 ! 23.7763+-0.1513 ! 40.7436+-0.1964 ! definitely 3.6214x slower crypto-md5 3.4951+-0.0344 ! 15.9365+-0.1302 ! 27.5367+-0.1492 ! definitely 7.8786x slower crypto-sha1 3.0708+-0.0170 ! 14.6742+-0.0774 ! 27.7720+-0.3279 ! definitely 9.0438x slower date-format-tofte 14.1085+-0.1157 ! 31.0783+-0.0933 ^ 30.7639+-0.0352 ! definitely 2.1805x slower date-format-xparb 14.4195+-0.1487 ! 26.5260+-0.1848 ^ 23.1269+-0.0614 ! definitely 1.6039x slower math-cordic 12.1021+-0.0200 ! 36.6978+-0.1130 ! 70.9342+-0.3044 ! definitely 5.8613x slower math-partial-sums 15.0148+-0.0252 ! 22.0465+-0.0671 ! 54.6326+-0.0661 ! definitely 3.6386x slower math-spectral-norm 3.2588+-0.0080 ! 18.0681+-1.2429 ! 42.4210+-0.0473 ! definitely 13.0173x slower regexp-dna 11.1681+-0.0469 ? 11.2181+-0.0782 ! 233.8707+-0.7245 ! definitely 20.9410x slower string-base64 5.7609+-0.0243 ! 25.8767+-0.1465 ! 27.5596+-0.1349 ! definitely 4.7839x slower string-fasta 10.5268+-0.0296 ! 24.8933+-0.2741 ! 36.4612+-0.0171 ! definitely 3.4637x slower string-tagcloud 16.5318+-0.0611 ! 26.0234+-0.3133 ! 45.0726+-0.6773 ! definitely 2.7264x slower string-unpack-code 26.5009+-0.1188 ! 30.0149+-0.2176 ! 64.0745+-0.1974 ! definitely 2.4178x slower string-validate-input 7.6353+-0.0251 ! 17.3225+-0.0934 ! 23.3600+-0.1823 ! definitely 3.0595x slower <arithmetic> * 9.1073+-0.0249 ! 23.1972+-0.1000 ! 52.3282+-0.0400 ! definitely 5.7457x slower <geometric> 7.3686+-0.0196 ! 21.6519+-0.0729 ! 43.7337+-0.0305 ! definitely 5.9351x slower <harmonic> 5.7195+-0.0203 ! 20.1591+-0.0641 ! 38.5437+-0.0333 ! definitely 6.7391x slower TipOfTree LLInt OldInt OldInt v. TipOfTree V8: crypto 120.1883+-0.5165 ! 1037.6748+-3.7260 ! 2807.2836+-3.1412 ! definitely 23.3574x slower deltablue 226.1341+-0.6206 ! 2734.4194+-28.1251 ^ 2493.4806+-10.8510 ! definitely 11.0266x slower earley-boyer 143.4873+-1.6203 ! 466.2559+-3.5907 ! 789.9330+-4.2703 ! definitely 5.5052x slower raytrace 68.8719+-0.2381 ! 303.0984+-1.9271 ! 353.9877+-5.8820 ! definitely 5.1398x slower regexp 129.1626+-0.3948 ! 162.7258+-0.5501 ! 1202.7013+-13.1267 ! definitely 9.3115x slower richards 240.3431+-1.1474 ! 2640.3920+-12.0460 ! 2896.8581+-5.3771 ! definitely 12.0530x slower splay 120.6794+-1.3454 ! 246.4478+-1.9810 ! 289.9561+-8.3939 ! definitely 2.4027x slower <arithmetic> 149.8381+-0.3944 ! 1084.4306+-4.6843 ! 1547.7429+-3.4411 ! definitely 10.3294x slower <geometric> * 139.0662+-0.3535 ! 636.7670+-2.0252 ! 1102.1020+-4.9534 ! definitely 7.9250x slower <harmonic> 128.6159+-0.2863 ! 403.3166+-1.4094 ! 738.4358+-7.3911 ! definitely 5.7414x slower TipOfTree LLInt OldInt OldInt v. TipOfTree Kraken: ai-astar 986.4699+-1.5020 ! 5711.2708+-1.5013 ^ 5656.3493+-4.4791 ! definitely 5.7339x slower audio-beat-detection 300.4027+-2.5656 ! 1544.7168+-1.8202 ! 3461.4428+-8.8822 ! definitely 11.5227x slower audio-dft 449.7360+-2.2174 ! 1563.2987+-14.5969 ! 2986.4066+-8.7263 ! definitely 6.6404x slower audio-fft 188.6152+-0.6885 ! 1491.1198+-2.0865 ! 3378.8465+-4.2707 ! definitely 17.9140x slower audio-oscillator 531.0049+-1.3694 ! 1119.1909+-1.2375 ! 2355.1291+-46.6195 ! definitely 4.4352x slower imaging-darkroom 477.9397+-5.2400 ! 1952.3063+-16.5109 ! 4594.1441+-9.4076 ! definitely 9.6124x slower imaging-desaturate 349.4066+-0.2174 ! 3459.0173+-2.5981 ! 6846.5305+-2.5816 ! definitely 19.5947x slower imaging-gaussian-blur 804.0863+-0.3510 ! 10101.1572+-114.0651 ! 28450.3464+-137.4128 ! definitely 35.3822x slower json-parse-financial 84.1573+-0.1870 ? 84.3888+-0.5073 83.9558+-0.0540 json-stringify-tinderbox 127.6504+-0.3823 127.5610+-0.3344 ^ 126.5883+-0.5533 ^ definitely 1.0084x faster stanford-crypto-aes 165.0739+-0.5711 ! 668.7007+-4.0118 ! 1169.5522+-2.7745 ! definitely 7.0850x slower stanford-crypto-ccm 148.1637+-0.5867 ! 478.2820+-0.8416 ! 805.8471+-1.0524 ! definitely 5.4389x slower stanford-crypto-pbkdf2 304.4971+-2.9252 ! 1632.6897+-1.5222 ! 2874.7453+-3.3609 ! definitely 9.4410x slower stanford-crypto-sha256-iterative 131.1675+-1.9117 ! 598.6028+-0.7485 ! 938.0714+-3.1941 ! definitely 7.1517x slower <arithmetic> * 360.5980+-0.3558 ! 2180.8788+-7.8414 ! 4551.9968+-7.8917 ! definitely 12.6235x slower <geometric> 280.6399+-0.3003 ! 1126.2141+-0.9497 ! 1946.9019+-2.5327 ! definitely 6.9374x slower <harmonic> 220.8662+-0.2670 ! 474.7984+-1.2485 ! 554.6496+-0.7083 ! definitely 2.5112x slower TipOfTree LLInt OldInt OldInt v. TipOfTree All benchmarks: <arithmetic> 134.7665+-0.1527 ! 823.9669+-1.8847 ! 1615.3763+-2.1290 ! definitely 11.9865x slower <geometric> 33.7496+-0.0654 ! 116.2493+-0.2059 ! 219.0713+-0.1464 ! definitely 6.4911x slower <harmonic> 10.0778+-0.0350 ! 35.1643+-0.1067 ! 66.2642+-0.0545 ! definitely 6.5752x slower TipOfTree LLInt OldInt OldInt v. TipOfTree Geomean of preferred means: <scaled-result> 77.0094+-0.1378 ! 318.1832+-0.5270 ! 640.2995+-0.7543 ! definitely 8.3146x slower

Filip Pizlo

Comment 25 2012-01-18 17:16:01 PST

Created attachment 123042 [details] work in progress fast/js passes.

Filip Pizlo

Comment 26 2012-01-18 19:46:45 PST

Created attachment 123055 [details] work in progress fast/js, sputnik, and ietestcenter all pass now.

Filip Pizlo

Comment 27 2012-01-18 21:15:55 PST

Created attachment 123066 [details] work in progress Just got LLInt->oldJIT tiering to work. And oldJIT->DFG tiering still works, too. Going to do more tests and then it'll be time to commit.

Filip Pizlo

Comment 28 2012-01-19 01:16:31 PST

Created attachment 123089 [details] work in progress I've got some hilarious value profiling pathologies to fix still. Right now this patch results in a 5% slow-down on SunSpider purely because some code never gets profiled, and the DFG doesn't handle that gracefully. Should be easy to fix though.

Filip Pizlo

Comment 29 2012-01-22 21:46:33 PST

Created attachment 123518 [details] work in progress Triple-tiering now works on SunSpider and run-javascript-core tests and does not cause regressions. Not too shabby, since SunSpider was not the benchmark we were trying to progress on with this change. Benchmark report for SunSpider on nitroflex (MacBookPro8,2). VMs tested: "TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc (r105584) "LLInt" at /Volumes/Data/pizlo/septenary/OpenSource/WebKitBuild/Release/jsc (r105584) Collected 12 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. TipOfTree LLInt 3d-cube 5.6745+-0.1568 ? 5.8361+-0.1208 ? might be 1.0285x slower 3d-morph 11.6210+-0.1631 ? 12.1069+-0.4708 ? might be 1.0418x slower 3d-raytrace 8.9877+-0.2091 ? 9.0600+-0.2257 ? access-binary-trees 1.7678+-0.0542 1.7197+-0.0319 might be 1.0280x faster access-fannkuch 7.4614+-0.0947 ? 7.6526+-0.2332 ? might be 1.0256x slower access-nbody 5.2353+-0.1827 ^ 4.7338+-0.1034 ^ definitely 1.1059x faster access-nsieve 2.9858+-0.0898 2.9686+-0.0710 bitops-3bit-bits-in-byte 1.2672+-0.0291 ? 1.2888+-0.0322 ? might be 1.0171x slower bitops-bits-in-byte 2.9313+-0.0499 ? 3.0038+-0.0783 ? might be 1.0247x slower bitops-bitwise-and 2.8171+-0.1788 ? 2.9507+-0.2950 ? might be 1.0474x slower bitops-nsieve-bits 6.5448+-0.3039 ? 6.5532+-0.3514 ? controlflow-recursive 2.3911+-0.0410 2.3722+-0.0297 crypto-aes 8.3165+-0.1755 8.2634+-0.2232 crypto-md5 2.7578+-0.1087 ! 3.0056+-0.0823 ! definitely 1.0899x slower crypto-sha1 2.4191+-0.0348 2.3804+-0.0404 might be 1.0163x faster date-format-tofte 10.3733+-0.2814 10.2813+-0.1830 date-format-xparb 11.3328+-0.7074 10.5627+-0.1865 might be 1.0729x faster math-cordic 8.0856+-0.0717 ? 8.0955+-0.1207 ? math-partial-sums 9.6584+-0.2356 9.5466+-0.1533 might be 1.0117x faster math-spectral-norm 2.4937+-0.0390 ? 2.4996+-0.0752 ? regexp-dna 7.8369+-0.1595 7.7539+-0.1160 might be 1.0107x faster string-base64 4.9288+-0.1364 ? 5.0067+-0.1325 ? might be 1.0158x slower string-fasta 7.7753+-0.1828 7.7097+-0.1765 string-tagcloud 12.4376+-0.2461 ? 12.4669+-0.2336 ? string-unpack-code 19.4631+-0.2433 ? 19.9456+-0.3127 ? might be 1.0248x slower string-validate-input 6.3725+-0.1745 6.3249+-0.1717 <arithmetic> * 6.6899+-0.0577 ? 6.6957+-0.0426 ? might be 1.0009x slower <geometric> 5.4089+-0.0367 ? 5.4170+-0.0346 ? might be 1.0015x slower <harmonic> 4.2462+-0.0267 ? 4.2621+-0.0290 ? might be 1.0037x slower

Filip Pizlo

Comment 30 2012-01-23 15:23:27 PST

Created attachment 123633 [details] work in progress Can now run all major benchmarks in triple-tier mode.

Filip Pizlo

Comment 31 2012-01-23 22:59:46 PST

Created attachment 123701 [details] work in progress It works on 32-bit Mac. Also modified the offlineasm to do its own dependency tracking, since convincing Xcode to do it is hard. In short, the offsets extractor will not do any work (and will hence not mutate the LLIntDesiredOffsets.h file) unless the input has changed, and in turn the assembler will not do any work (and not mutate the LLIntAssembly.h file) unless the offset extract binary has different offsets than before and the input file had changed. This is done by storing SHA1 signatures of the input in the output files. The result is that you don't pay the cost of running the offlineasm, or of any downstream rebuilds, if you don't change either the LowLevelInterpreter.asm file or actually modify some structures/classes to have different sizes and offsets.

Filip Pizlo

Comment 32 2012-01-24 12:32:11 PST

Created attachment 123783 [details] work in progress Made inlining of interpreted functions work.

Filip Pizlo

Comment 33 2012-01-24 15:08:07 PST

Created attachment 123822 [details] work in progress It completely works on 32-bit Mac now. On to other platforms...

Filip Pizlo

Comment 34 2012-01-24 17:28:22 PST

Created attachment 123855 [details] the patch It's ready.

WebKit Review Bot

Comment 35 2012-01-24 17:31:32 PST

Attachment 123855 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Last 3072 characters of output: aScriptCore/llint/LLIntHelpers.cpp:126: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:133: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:228: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:557: One line control clauses should not use braces. [whitespace/braces] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1313: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1346: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:43: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:41: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:55: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:57: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:68: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 29 in 93 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 36 2012-01-24 17:33:44 PST

Created attachment 123858 [details] the patch Fixed the real style errors. The rest are false positives.

WebKit Review Bot

Comment 37 2012-01-24 17:38:37 PST

Attachment 123858 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Last 3072 characters of output: se underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:120: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:126: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:133: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:228: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:41: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:55: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:57: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:68: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 93 files If any of these errors are false positives, please file a bug against check-webkit-style.

Gavin Barraclough

Comment 38 2012-01-24 18:44:57 PST

Comment on attachment 123858 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=123858&action=review > Source/JavaScriptCore/bytecode/CodeBlock.cpp:-62 > -#if !defined(NDEBUG) || ENABLE(OPCODE_SAMPLING) This is enabling dumping in release builds? - I don't think we'll want all these strings in a release binary, this change should be reverted. > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1535 > +#if ENABLE(LLINT) I'm saddened to see yet another build flag – though this is clearly necessary until LLInt is ported to all platforms. Looking toward the future, we may need to think about rationalizing down – typing together LLINT + JIT + DFG + MACRO_ASSEMBLER + YARR_JIT into a single brave-new-world-of-fast-JS-with-JITs switch. > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1754 > + if (!interpreter->enabled()) { This confuses me a little - it looks like if LLInt is being used, interpreter->enabled() will be true, does LLInt not use the structure slots embedded within the instruction stream? – a comment here would probably be useful. :-)

Early Warning System Bot

Comment 39 2012-01-24 19:19:33 PST

Comment on attachment 123858 [details] the patch Attachment 123858 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11242417

Gyuyoung Kim

Comment 40 2012-01-24 19:47:57 PST

Comment on attachment 123858 [details] the patch Attachment 123858 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11343158

Filip Pizlo

Comment 41 2012-01-24 19:49:23 PST

(In reply to comment #38) > (From update of attachment 123858 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=123858&action=review > > > Source/JavaScriptCore/bytecode/CodeBlock.cpp:-62 > > -#if !defined(NDEBUG) || ENABLE(OPCODE_SAMPLING) > > This is enabling dumping in release builds? - I don't think we'll want all these strings in a release binary, this change should be reverted. Should we really bothered by 824 bytes worth of strings and code? The way C++ inlines template code like crazy, probably any change we make introduces that much. Here's the size of a release build of the JavaScriptCore framework in ToT: 3578504 bytes Here's the size of a release build with CodeBlock dumping compiled in: 3579328 bytes So compiling in the dumping code leads to 824 bytes *total* increase in the framework size. That's strings and code, combined. Another way to put it is that it's a 0.02% increase - hardly noticable. It's pretty much routine for me at this point to have to enable this dumping thingy in release builds when dealing with release-only crashes. It's really annoying, and the NDEBUG checks are super annoying. I agree with your comment below that we should strive to reduce the amount of #if madness in our code. Are we really willing to have #if NDEBUG statements in a bunch of places to protect against having a 0.02% increase in code size? > > > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1535 > > +#if ENABLE(LLINT) > > I'm saddened to see yet another build flag – though this is clearly necessary until LLInt is ported to all platforms. > > Looking toward the future, we may need to think about rationalizing down – typing together LLINT + JIT + DFG + MACRO_ASSEMBLER + YARR_JIT into a single brave-new-world-of-fast-JS-with-JITs switch. > > > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1754 > > + if (!interpreter->enabled()) { > > This confuses me a little - it looks like if LLInt is being used, interpreter->enabled() will be true, does LLInt not use the structure slots embedded within the instruction stream? – a comment here would probably be useful. :-) interpreter->enabled() means that LLInt was compiled in but we ended up using the old interpreter instead, a mode that I still support because there's probably a

Filip Pizlo

Comment 42 2012-01-24 21:01:42 PST

Created attachment 123880 [details] the patch Attempt to fix Qt/EFL/GTK builds.

Filip Pizlo

Comment 43 2012-01-24 21:02:06 PST

(In reply to comment #42) > Created an attachment (id=123880) [details] > the patch > > Attempt to fix Qt/EFL/GTK builds. And also addressed Gavin's comment about not having comments in the !interpreter->enabled() check.

WebKit Review Bot

Comment 44 2012-01-24 21:08:29 PST

Attachment 123880 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: se underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:120: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:126: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:133: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:228: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:41: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:55: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:57: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:68: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 97 files If any of these errors are false positives, please file a bug against check-webkit-style.

Gyuyoung Kim

Comment 45 2012-01-24 21:24:54 PST

Comment on attachment 123880 [details] the patch Attachment 123880 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11344251

Early Warning System Bot

Comment 46 2012-01-24 21:37:15 PST

Comment on attachment 123880 [details] the patch Attachment 123880 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11159224

Gustavo Noronha (kov)

Comment 47 2012-01-25 06:22:47 PST

Comment on attachment 123880 [details] the patch Attachment 123880 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11177485

Filip Pizlo

Comment 48 2012-01-25 10:27:00 PST

Created attachment 123970 [details] the patch Fixed bunch of builds.

WebKit Review Bot

Comment 49 2012-01-25 10:37:57 PST

Attachment 123970 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: se underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:120: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:126: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:133: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:228: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:41: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:55: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:57: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:68: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 99 files If any of these errors are false positives, please file a bug against check-webkit-style.

Gyuyoung Kim

Comment 50 2012-01-25 11:12:07 PST

Comment on attachment 123970 [details] the patch Attachment 123970 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11242884

Filip Pizlo

Comment 51 2012-01-26 15:48:23 PST

Created attachment 124199 [details] the patch Fix build on EFL, Windows.

WebKit Review Bot

Comment 52 2012-01-26 15:51:53 PST

Attachment 124199 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:120: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:126: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:133: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:228: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:41: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:55: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:57: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:68: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 101 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 53 2012-01-26 15:54:33 PST

Created attachment 124201 [details] the patch Cleaned up the comment style that I used in offlineasm.

WebKit Review Bot

Comment 54 2012-01-26 15:59:07 PST

Attachment 124201 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:120: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:126: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:133: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:228: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:41: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:55: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:57: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:68: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 101 files If any of these errors are false positives, please file a bug against check-webkit-style.

Gyuyoung Kim

Comment 55 2012-01-26 16:58:18 PST

Comment on attachment 124201 [details] the patch Attachment 124201 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11349525

Filip Pizlo

Comment 56 2012-01-26 17:37:00 PST

Created attachment 124225 [details] the patch Trying to fix the build more.

Filip Pizlo

Comment 57 2012-01-26 18:03:42 PST

Created attachment 124229 [details] the patch Rebased.

WebKit Review Bot

Comment 58 2012-01-26 18:07:01 PST

Attachment 124229 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:120: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:126: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:133: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:228: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:46: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:48: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:54: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:67: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 104 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 59 2012-01-26 20:12:46 PST

Created attachment 124248 [details] the patch Another rebase!

WebKit Review Bot

Comment 60 2012-01-26 20:16:43 PST

Attachment 124248 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:120: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:126: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:133: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:228: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:64: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:46: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:48: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:54: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:67: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 103 files If any of these errors are false positives, please file a bug against check-webkit-style.

Gyuyoung Kim

Comment 61 2012-01-26 20:51:01 PST

Comment on attachment 124248 [details] the patch Attachment 124248 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11349587

Filip Pizlo

Comment 62 2012-01-27 13:37:03 PST

Created attachment 124360 [details] the patch Again trying to fix EFL and Windows build. Maybe I got it this time? ;-)

WebKit Review Bot

Comment 63 2012-01-27 13:40:04 PST

Attachment 124360 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:120: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:126: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:133: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:228: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:64: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:48: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:50: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:69: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 105 files If any of these errors are false positives, please file a bug against check-webkit-style.

Gyuyoung Kim

Comment 64 2012-01-27 14:15:00 PST

Comment on attachment 124360 [details] the patch Attachment 124360 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11163853

Filip Pizlo

Comment 65 2012-01-27 14:42:19 PST

Created attachment 124372 [details] the patch - More EFL build fixes. - Made the offline assembler capable of hashing itself for depedency tracking. So if you change the offline assembler, then it will realize that it must rebuild LLInt. - Made the offline assembler use InlineASM.h's macros.

Filip Pizlo

Comment 66 2012-01-27 14:43:10 PST

Created attachment 124373 [details] LLIntAssembly.h Generated assembly code with latest offlineasm changes.

Filip Pizlo

Comment 67 2012-01-27 14:46:58 PST

(In reply to comment #66) > Created an attachment (id=124373) [details] > LLIntAssembly.h > > Generated assembly code with latest offlineasm changes. Hajime, Simon, can you check that the LLIntAssembly.h file does the symbol magic consistently with the changes you guys are making? Note that I had to introduce a new macro in InlineASM.h to create local labels. Currently I only have this working on Mac, but it should be easy to expand support for this to other platforms in future.

Filip Pizlo

Comment 68 2012-01-27 14:50:37 PST

Created attachment 124374 [details] the patch Another rebase!

WebKit Review Bot

Comment 69 2012-01-27 14:54:41 PST

Attachment 124374 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:120: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:126: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:133: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:228: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:64: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 108 files If any of these errors are false positives, please file a bug against check-webkit-style.

Hajime Morrita

Comment 70 2012-01-27 15:28:28 PST

(In reply to comment #67) > (In reply to comment #66) > > Created an attachment (id=124373) [details] [details] > > LLIntAssembly.h > > > > Generated assembly code with latest offlineasm changes. > > Hajime, Simon, can you check that the LLIntAssembly.h file does the symbol magic consistently with the changes you guys are making? Filip, Thanks for caring. It looks good to me although Simon should know more. Because the generated code is using same machinery as existing code, it should work fine. And the code generator looks straightforward enough to fix possible problem later without significant impact.

Filip Pizlo

Comment 71 2012-01-28 00:41:10 PST

Created attachment 124432 [details] the patch, sort of This patch may be slightly polluted by debug garbage, as well as a bugfix that is to be reviewed separately. I will "remove" that bugfix from this patch by rebasing after that one lands.

WebKit Review Bot

Comment 72 2012-01-28 00:44:10 PST

Attachment 124432 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:120: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:126: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:133: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:228: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1316: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1349: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:64: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 112 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 73 2012-01-29 17:37:38 PST

Created attachment 124478 [details] the patch Includes beginnings of proper ARMv7 support in the offlineasm backend. This is done by doing a series of micropasses that break down the larger operations (like say "addi 5000, 4[foo, bar, 8]") into smaller ones ("move 4, tmp1; addp foo, tmp1; loadi [tmp1, bar, 8], tmp2; move 5000, tmp3; addi tmp3, tmp2; storei tmp2, [tmp1, bar, 8]"). Also includes a build fix in tools/; I'll land that build fix separately and exclude it from this patch in the next rebase.

WebKit Review Bot

Comment 74 2012-01-29 17:41:24 PST

Attachment 124478 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:120: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:126: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:133: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:228: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1316: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1349: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:64: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 111 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 75 2012-01-29 22:00:04 PST

Created attachment 124496 [details] the patch Rabased, and added more ARMv7 code

WebKit Review Bot

Comment 76 2012-01-29 22:03:45 PST

Attachment 124496 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:120: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:126: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:133: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:228: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1316: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1349: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:64: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:34: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 110 files If any of these errors are false positives, please file a bug against check-webkit-style.

Simon Hausmann

Comment 77 2012-01-30 06:48:24 PST

(In reply to comment #70) > (In reply to comment #67) > > (In reply to comment #66) > > > Created an attachment (id=124373) [details] [details] [details] > > > LLIntAssembly.h > > > > > > Generated assembly code with latest offlineasm changes. > > > > Hajime, Simon, can you check that the LLIntAssembly.h file does the symbol magic consistently with the changes you guys are making? > > Filip, Thanks for caring. > It looks good to me although Simon should know more. > Because the generated code is using same machinery as existing code, it should work fine. > And the code generator looks straightforward enough to fix possible problem later > without significant impact. Looks good AFAICS :) Simon

Filip Pizlo

Comment 78 2012-01-30 15:12:29 PST

Created attachment 124608 [details] the patch - Rebased for new way of dealing with getters and setters. - More ARMv7 stuff.

WebKit Review Bot

Comment 79 2012-01-30 15:15:51 PST

Attachment 124608 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:121: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:128: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:136: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:229: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1322: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1355: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 110 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 80 2012-01-30 15:56:09 PST

Created attachment 124619 [details] the patch - complete (though untested) implementation of ARMv7 and another rebase

WebKit Review Bot

Comment 81 2012-01-30 16:00:58 PST

Attachment 124619 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:121: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:127: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:128: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:134: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:135: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:136: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:229: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1322: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1355: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 110 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 82 2012-01-30 18:07:50 PST

Created attachment 124646 [details] work in progress Doing a bunch of hacking to get ARMv7 to work. I'm almost there. Backing up.

Filip Pizlo

Comment 83 2012-01-31 19:28:44 PST

Created attachment 124872 [details] the patch ARM is starting to work.

WebKit Review Bot

Comment 84 2012-01-31 19:34:37 PST

Attachment 124872 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:123: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:129: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:130: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:136: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:137: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:138: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:231: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1412: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1449: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 111 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 85 2012-01-31 20:14:51 PST

Created attachment 124875 [details] the patch Realized that the previous patch had some unnecessary printf's. Other than that - still working on fleshing out ARM support.

WebKit Review Bot

Comment 86 2012-01-31 20:20:11 PST

Attachment 124875 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:123: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:129: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:130: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:136: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:137: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:138: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:231: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1412: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1449: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 111 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 87 2012-01-31 23:23:21 PST

Created attachment 124888 [details] the patch ARM works in LLInt-only config. Haven't tried LLInt+JIT yet, but I'm guessing it'll just work.

Filip Pizlo

Comment 88 2012-02-01 15:41:40 PST

Created attachment 125035 [details] the patch Rebased again. We can now run LLInt+JIT on ARM.

WebKit Review Bot

Comment 89 2012-02-01 15:46:11 PST

Attachment 125035 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:123: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:129: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:130: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:136: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:137: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:138: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:231: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1412: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1449: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 111 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 90 2012-02-01 23:55:37 PST

Created attachment 125085 [details] the patch Made SunSpider, V8, and Kraken run on ARM. To do this I had to make more debug printing work in release builds. :-(

WebKit Review Bot

Comment 91 2012-02-01 23:58:55 PST

Attachment 125085 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:123: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:129: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:130: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:136: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:137: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:138: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:231: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1413: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1450: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:69: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 111 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 92 2012-02-02 00:02:23 PST

(In reply to comment #90) > Created an attachment (id=125085) [details] > the patch > > Made SunSpider, V8, and Kraken run on ARM. > > To do this I had to make more debug printing work in release builds. :-( Never mind, bad patch. I left in some horrible debug magic. I will retest without it and repost.

Gyuyoung Kim

Comment 93 2012-02-02 00:24:33 PST

Comment on attachment 125085 [details] the patch Attachment 125085 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11387775

Filip Pizlo

Comment 94 2012-02-02 00:37:31 PST

Created attachment 125091 [details] the patch - Fixed windows/efl, hopefully. - Removed some really dumb debug stuff.

WebKit Review Bot

Comment 95 2012-02-02 00:41:55 PST

Attachment 125091 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:123: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:129: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:130: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:136: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:137: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:138: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:231: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1413: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1450: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:69: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 112 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 96 2012-02-02 13:57:15 PST

Created attachment 125186 [details] the patch Rebased to include the release debugging changes.

WebKit Review Bot

Comment 97 2012-02-02 14:00:47 PST

Attachment 125186 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:123: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:129: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:130: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:136: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:137: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:138: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:231: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1413: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1450: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:69: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 111 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 98 2012-02-02 14:53:55 PST

Created attachment 125194 [details] the patch Optimized generate_offset_extractor.rb. It now runs 10x faster.

Filip Pizlo

Comment 99 2012-02-06 15:17:08 PST

Created attachment 125715 [details] the patch Another rebase.

WebKit Review Bot

Comment 100 2012-02-06 15:21:06 PST

Attachment 125715 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:123: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:129: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:130: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:136: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:137: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:138: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:231: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1413: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1450: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:69: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 112 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 101 2012-02-06 17:18:58 PST

Created attachment 125735 [details] the patch Rebased and switched to using 4 space indentation in Ruby code.

WebKit Review Bot

Comment 102 2012-02-06 17:22:47 PST

Attachment 125735 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Last 3072 characters of output: e underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:123: __ct_globalData is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:129: __cce_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:130: __cce_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:136: __cr_exec is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:137: __cr_pc is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:138: __cr_callTarget is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/llint/LLIntHelpers.cpp:231: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1413: The parameter name "v" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/llint/LLIntHelpers.cpp:1450: The parameter name "!" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/StructureChain.h:40: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/runtime/JSTypeInfo.h:37: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/runtime/JSVariableObject.h:41: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/jit/HostCallReturnValue.cpp:69: The parameter name """" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:48: Code inside a namespace should not be indented. [whitespace/indent] [4] Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 27 in 112 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 103 2012-02-14 11:19:01 PST

Created attachment 127001 [details] the patch Rebased again.

WebKit Review Bot

Comment 104 2012-02-15 01:26:48 PST

Attachment 127001 [details] did not pass style-queue: Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9 Updating OpenSource M ChangeLog A LayoutTests/fast/css/style-scoped/registering-shadowroot-expected.txt A LayoutTests/fast/css/style-scoped/registering-shadowroot.html M LayoutTests/ChangeLog M Source/WebCore/WebCore.exp.in M Source/WebCore/testing/Internals.cpp M Source/WebCore/testing/Internals.h M Source/WebCore/testing/Internals.idl M Source/WebCore/dom/ShadowRootList.h M Source/WebCore/dom/ShadowRootList.cpp M Source/WebCore/dom/Element.cpp M Source/WebCore/dom/Node.h M Source/WebCore/dom/Element.h M Source/WebCore/dom/NodeRareData.h M Source/WebCore/dom/Node.cpp M Source/WebCore/dom/ElementRareData.h M Source/WebCore/ChangeLog M Source/WebCore/platform/graphics/FontCache.cpp M Source/WebCore/html/HTMLStyleElement.cpp M Source/autotools/symbols.filter M Source/WebKit2/ChangeLog M Source/WebKit2/win/WebKit2CFLite.def M Source/WebKit2/win/WebKit2.def r107794 = c59b9dab74417b86c7b5b60a4408ab3e5d1659a8 (refs/remotes/origin/master) First, rewinding head to replay your work on top of it... Applying: [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets Using index info to reconstruct a base tree... <stdin>:1578: trailing whitespace. <stdin>:1647: trailing whitespace. <stdin>:1657: trailing whitespace. <stdin>:1672: trailing whitespace. return 0; <stdin>:1674: trailing whitespace. warning: squelched 7 whitespace errors warning: 12 lines add whitespace errors. Falling back to patching base and 3-way merge... warning: too many files (created: 168755 deleted: 3), skipping inexact rename detection Auto-merging LayoutTests/ChangeLog CONFLICT (content): Merge conflict in LayoutTests/ChangeLog Auto-merging LayoutTests/platform/wk2/Skipped Auto-merging Source/WebCore/ChangeLog CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog Auto-merging Source/WebCore/css/CSSCalculationValue.cpp Auto-merging Source/WebCore/css/CSSCalculationValue.h Auto-merging Source/WebCore/css/CSSParser.cpp Auto-merging Source/WebKit/mac/ChangeLog CONFLICT (content): Merge conflict in Source/WebKit/mac/ChangeLog Auto-merging Source/WebKit2/ChangeLog CONFLICT (content): Merge conflict in Source/WebKit2/ChangeLog Auto-merging Tools/ChangeLog CONFLICT (content): Merge conflict in Tools/ChangeLog Failed to merge in the changes. Patch failed at 0001 [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. If any of these errors are false positives, please file a bug against check-webkit-style.

Gavin Barraclough

Comment 105 2012-02-17 23:07:06 PST

Comment on attachment 127001 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=127001&action=review Okay, this basically all looks good. Couple small of issues: Please revert JSActivation.h & Heap.cpp (whitespace only changes). Please rename Interpreter::enabled() (to make it clear which interpreter this is enabling - e.g. 'classicEnabled()' would be fine.) Per our discussion, I think 'Helpers' isn't a great name, since it isn't descriptive of what it helps. I'm happy with your suggested 'SlowPaths'. It's a shame that LLIntOffsetsExtractor is LLInt specific – it seems this pattern could be generalized to provide offsets into classes for use by the JITs, too, and reduce the friendliness of the codebase. But I'm not asking for this change in this patch, it's fine as is. > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:70 > +# These declarations must match interpreter/RegisterFile.h. Are these constants defended anywhere by compile time ASSERTs?

Filip Pizlo

Comment 106 2012-02-17 23:11:36 PST

(In reply to comment #105) > (From update of attachment 127001 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=127001&action=review > > Okay, this basically all looks good. Couple small of issues: Thanks! :-) > > Please revert JSActivation.h & Heap.cpp (whitespace only changes). Ack. > > Please rename Interpreter::enabled() (to make it clear which interpreter this is enabling - e.g. 'classicEnabled()' would be fine.) Good idea. > > Per our discussion, I think 'Helpers' isn't a great name, since it isn't descriptive of what it helps. I'm happy with your suggested 'SlowPaths'. Will do. > > It's a shame that LLIntOffsetsExtractor is LLInt specific – it seems this pattern could be generalized to provide offsets into classes for use by the JITs, too, and reduce the friendliness of the codebase. But I'm not asking for this change in this patch, it's fine as is. Agree. > > > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:70 > > +# These declarations must match interpreter/RegisterFile.h. > > Are these constants defended anywhere by compile time ASSERTs? Unfortunately not. :-( I will add these to LLIntData.cpp, since LLInt initialization funnels through there. I'd like to come up with a way of having these constants get extracted similarly to how we do offsets and sizes. I'm not there yet. :-(

Filip Pizlo

Comment 107 2012-02-18 11:10:37 PST

Created attachment 127706 [details] the patch Rebasing and slowly incorporating Gavin's comments. Putting up for EWS.

Filip Pizlo

Comment 108 2012-02-18 12:07:18 PST

Created attachment 127707 [details] the patch Addressed Gavin's concerns. Will have to do more testing before landing. Putting up for EWS to let it simmer a bit.

Filip Pizlo

Comment 109 2012-02-20 09:15:29 PST

Created attachment 127825 [details] the patch Rebasing.

Filip Pizlo

Comment 110 2012-02-20 22:55:11 PST

Landed in http://trac.webkit.org/changeset/108309

Csaba Osztrogonác

Comment 111 2012-02-21 00:26:49 PST

Reopen, because it broke the Qt 64 bit debug build: /usr/bin/gold: /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/Source/JavaScriptCore/debug/libJavaScriptCore.a(DFGOperations.o): in function handleHostCall:../../../../Source/JavaScriptCore/dfg/DFGOperations.cpp:761: error: undefined reference to 'getHostCallReturnValue' /usr/bin/gold: /home/webkitbuildbot/slaves/debug64bit/buildslave/qt-linux-64-debug/build/WebKitBuild/Debug/Source/JavaScriptCore/debug/libJavaScriptCore.a(DFGOperations.o): in function handleHostCall:../../../../Source/JavaScriptCore/dfg/DFGOperations.cpp:781: error: undefined reference to 'getHostCallReturnValue'

Csaba Osztrogonác

Comment 112 2012-02-21 00:27:43 PST

Filip Pizlo

Comment 113 2012-02-21 01:04:27 PST

Created attachment 127932 [details] fix for elf Fix for ELF. Can Qt/Gtk people check if this fixes things?

Filip Pizlo

Comment 114 2012-02-21 01:14:20 PST

Build fix for ELF platforms landed in http://trac.webkit.org/changeset/108323

Filip Pizlo

Comment 115 2012-02-21 01:18:35 PST

Comment on attachment 127932 [details] fix for elf Clearing review flag because this was already landed.

Csaba Osztrogonác

Comment 116 2012-02-21 01:23:46 PST

Reopen again, because build works, but tests crash. :-/

Filip Pizlo

Comment 117 2012-02-21 01:33:06 PST

Created attachment 127938 [details] fix for non-DFG build and DFG crashes

Csaba Osztrogonác

Comment 118 2012-02-21 01:47:15 PST

Comment on attachment 127938 [details] fix for non-DFG build and DFG crashes It fixed the crashes on Qt Linux debug, and fixes at leasat the non-DFG Qt Windows build.

Filip Pizlo

Comment 119 2012-02-21 01:49:57 PST

Fix for non-DFG build and DFG crashed landed in http://trac.webkit.org/changeset/108326

Adam Roben (:aroben)

Comment 120 2012-02-21 09:59:15 PST

I rolled this out in r108358 because it broke the 32-bit Lion build.

Filip Pizlo

Comment 121 2012-02-21 12:15:59 PST

Created attachment 128019 [details] the patch Fat binary build fixes.

Filip Pizlo

Comment 122 2012-02-21 21:23:26 PST

Landed with variety of platform fixes in http://trac.webkit.org/changeset/108444

Note You need to log in before you can comment on or make changes to this bug.