Bug 75812 - JSC should be a triple-tier VM
Summary: JSC should be a triple-tier VM
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on: 78791
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-08 16:28 PST by Filip Pizlo
Modified: 2012-02-21 21:23 PST (History)
14 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2012-01-08 16:28:43 PST
This is a work in progress.
Comment 1 Filip Pizlo 2012-01-08 16:29:57 PST
<rdar://problem/10079694>
Comment 2 Filip Pizlo 2012-01-08 16:32:56 PST
Created attachment 121601 [details]
work in progress

Doesn't work yet, not ready for review.
Comment 3 Filip Pizlo 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.
Comment 4 Filip Pizlo 2012-01-08 21:14:45 PST
Created attachment 121617 [details]
work in progress

It's starting to build, sort of.
Comment 5 Sam Weinig 2012-01-08 21:38:34 PST
Triple? Meh. Let's go to five -- http://onion.com/bWBRid.
Comment 6 Filip Pizlo 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. ;-)
Comment 7 Filip Pizlo 2012-01-08 23:36:38 PST
Created attachment 121624 [details]
work in progress

Wrote more glue.
Comment 8 Filip Pizlo 2012-01-09 16:00:49 PST
Created attachment 121744 [details]
work in progress

Getting closer to having it build.
Comment 9 Filip Pizlo 2012-01-09 17:29:19 PST
Created attachment 121769 [details]
work in progress

It now compiles.  But does not link.
Comment 10 Filip Pizlo 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.
Comment 11 Filip Pizlo 2012-01-10 14:45:09 PST
Created attachment 121915 [details]
work in progress

Rebased, and started to work on the yucky glue.
Comment 12 Filip Pizlo 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.
Comment 13 Filip Pizlo 2012-01-11 17:31:35 PST
Created attachment 122140 [details]
work in progress
Comment 14 Filip Pizlo 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.
Comment 15 WebKit Review Bot 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.
Comment 16 Filip Pizlo 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.
Comment 17 Gyuyoung Kim 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
Comment 18 Early Warning System Bot 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
Comment 19 Filip Pizlo 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]
Comment 20 Filip Pizlo 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.
Comment 21 Filip Pizlo 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.
Comment 22 Filip Pizlo 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!
Comment 23 Filip Pizlo 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.
Comment 24 Filip Pizlo 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
Comment 25 Filip Pizlo 2012-01-18 17:16:01 PST
Created attachment 123042 [details]
work in progress

fast/js passes.
Comment 26 Filip Pizlo 2012-01-18 19:46:45 PST
Created attachment 123055 [details]
work in progress

fast/js, sputnik, and ietestcenter all pass now.
Comment 27 Filip Pizlo 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.
Comment 28 Filip Pizlo 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.
Comment 29 Filip Pizlo 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
Comment 30 Filip Pizlo 2012-01-23 15:23:27 PST
Created attachment 123633 [details]
work in progress

Can now run all major benchmarks in triple-tier mode.
Comment 31 Filip Pizlo 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.
Comment 32 Filip Pizlo 2012-01-24 12:32:11 PST
Created attachment 123783 [details]
work in progress

Made inlining of interpreted functions work.
Comment 33 Filip Pizlo 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...
Comment 34 Filip Pizlo 2012-01-24 17:28:22 PST
Created attachment 123855 [details]
the patch

It's ready.
Comment 35 WebKit Review Bot 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.
Comment 36 Filip Pizlo 2012-01-24 17:33:44 PST
Created attachment 123858 [details]
the patch

Fixed the real style errors.  The rest are false positives.
Comment 37 WebKit Review Bot 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.
Comment 38 Gavin Barraclough 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. :-)
Comment 39 Early Warning System Bot 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
Comment 40 Gyuyoung Kim 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
Comment 41 Filip Pizlo 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
Comment 42 Filip Pizlo 2012-01-24 21:01:42 PST
Created attachment 123880 [details]
the patch

Attempt to fix Qt/EFL/GTK builds.
Comment 43 Filip Pizlo 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.
Comment 44 WebKit Review Bot 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.
Comment 45 Gyuyoung Kim 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
Comment 46 Early Warning System Bot 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
Comment 47 Gustavo Noronha (kov) 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
Comment 48 Filip Pizlo 2012-01-25 10:27:00 PST
Created attachment 123970 [details]
the patch

Fixed bunch of builds.
Comment 49 WebKit Review Bot 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.
Comment 50 Gyuyoung Kim 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
Comment 51 Filip Pizlo 2012-01-26 15:48:23 PST
Created attachment 124199 [details]
the patch

Fix build on EFL, Windows.
Comment 52 WebKit Review Bot 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.
Comment 53 Filip Pizlo 2012-01-26 15:54:33 PST
Created attachment 124201 [details]
the patch

Cleaned up the comment style that I used in offlineasm.
Comment 54 WebKit Review Bot 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.
Comment 55 Gyuyoung Kim 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
Comment 56 Filip Pizlo 2012-01-26 17:37:00 PST
Created attachment 124225 [details]
the patch

Trying to fix the build more.
Comment 57 Filip Pizlo 2012-01-26 18:03:42 PST
Created attachment 124229 [details]
the patch

Rebased.
Comment 58 WebKit Review Bot 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.
Comment 59 Filip Pizlo 2012-01-26 20:12:46 PST
Created attachment 124248 [details]
the patch

Another rebase!
Comment 60 WebKit Review Bot 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.
Comment 61 Gyuyoung Kim 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
Comment 62 Filip Pizlo 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? ;-)
Comment 63 WebKit Review Bot 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.
Comment 64 Gyuyoung Kim 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
Comment 65 Filip Pizlo 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.
Comment 66 Filip Pizlo 2012-01-27 14:43:10 PST
Created attachment 124373 [details]
LLIntAssembly.h

Generated assembly code with latest offlineasm changes.
Comment 67 Filip Pizlo 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.
Comment 68 Filip Pizlo 2012-01-27 14:50:37 PST
Created attachment 124374 [details]
the patch

Another rebase!
Comment 69 WebKit Review Bot 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.
Comment 70 Hajime Morrita 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.
Comment 71 Filip Pizlo 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.
Comment 72 WebKit Review Bot 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.
Comment 73 Filip Pizlo 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.
Comment 74 WebKit Review Bot 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.
Comment 75 Filip Pizlo 2012-01-29 22:00:04 PST
Created attachment 124496 [details]
the patch

Rabased, and added more ARMv7 code
Comment 76 WebKit Review Bot 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.
Comment 77 Simon Hausmann 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
Comment 78 Filip Pizlo 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.
Comment 79 WebKit Review Bot 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.
Comment 80 Filip Pizlo 2012-01-30 15:56:09 PST
Created attachment 124619 [details]
the patch

- complete (though untested) implementation of ARMv7 and another rebase
Comment 81 WebKit Review Bot 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.
Comment 82 Filip Pizlo 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.
Comment 83 Filip Pizlo 2012-01-31 19:28:44 PST
Created attachment 124872 [details]
the patch

ARM is starting to work.
Comment 84 WebKit Review Bot 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.
Comment 85 Filip Pizlo 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.
Comment 86 WebKit Review Bot 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.
Comment 87 Filip Pizlo 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.
Comment 88 Filip Pizlo 2012-02-01 15:41:40 PST
Created attachment 125035 [details]
the patch

Rebased again.  We can now run LLInt+JIT on ARM.
Comment 89 WebKit Review Bot 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.
Comment 90 Filip Pizlo 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. :-(
Comment 91 WebKit Review Bot 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.
Comment 92 Filip Pizlo 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.
Comment 93 Gyuyoung Kim 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
Comment 94 Filip Pizlo 2012-02-02 00:37:31 PST
Created attachment 125091 [details]
the patch

- Fixed windows/efl, hopefully.

- Removed some really dumb debug stuff.
Comment 95 WebKit Review Bot 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.
Comment 96 Filip Pizlo 2012-02-02 13:57:15 PST
Created attachment 125186 [details]
the patch

Rebased to include the release debugging changes.
Comment 97 WebKit Review Bot 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.
Comment 98 Filip Pizlo 2012-02-02 14:53:55 PST
Created attachment 125194 [details]
the patch

Optimized generate_offset_extractor.rb.  It now runs 10x faster.
Comment 99 Filip Pizlo 2012-02-06 15:17:08 PST
Created attachment 125715 [details]
the patch

Another rebase.
Comment 100 WebKit Review Bot 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.
Comment 101 Filip Pizlo 2012-02-06 17:18:58 PST
Created attachment 125735 [details]
the patch

Rebased and switched to using 4 space indentation in Ruby code.
Comment 102 WebKit Review Bot 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.
Comment 103 Filip Pizlo 2012-02-14 11:19:01 PST
Created attachment 127001 [details]
the patch

Rebased again.
Comment 104 WebKit Review Bot 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.
Comment 105 Gavin Barraclough 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?
Comment 106 Filip Pizlo 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. :-(
Comment 107 Filip Pizlo 2012-02-18 11:10:37 PST
Created attachment 127706 [details]
the patch

Rebasing and slowly incorporating Gavin's comments.  Putting up for EWS.
Comment 108 Filip Pizlo 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.
Comment 109 Filip Pizlo 2012-02-20 09:15:29 PST
Created attachment 127825 [details]
the patch

Rebasing.
Comment 110 Filip Pizlo 2012-02-20 22:55:11 PST
Landed in http://trac.webkit.org/changeset/108309
Comment 111 Csaba Osztrogonác 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'
Comment 112 Csaba Osztrogonác 2012-02-21 00:27:43 PST
.
Comment 113 Filip Pizlo 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?
Comment 114 Filip Pizlo 2012-02-21 01:14:20 PST
Build fix for ELF platforms landed in http://trac.webkit.org/changeset/108323
Comment 115 Filip Pizlo 2012-02-21 01:18:35 PST
Comment on attachment 127932 [details]
fix for elf

Clearing review flag because this was already landed.
Comment 116 Csaba Osztrogonác 2012-02-21 01:23:46 PST
Reopen again, because build works, but tests crash. :-/
Comment 117 Filip Pizlo 2012-02-21 01:33:06 PST
Created attachment 127938 [details]
fix for non-DFG build and DFG crashes
Comment 118 Csaba Osztrogonác 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.
Comment 119 Filip Pizlo 2012-02-21 01:49:57 PST
Fix for non-DFG build and DFG crashed landed in http://trac.webkit.org/changeset/108326
Comment 120 Adam Roben (:aroben) 2012-02-21 09:59:15 PST
I rolled this out in r108358 because it broke the 32-bit Lion build.
Comment 121 Filip Pizlo 2012-02-21 12:15:59 PST
Created attachment 128019 [details]
the patch

Fat binary build fixes.
Comment 122 Filip Pizlo 2012-02-21 21:23:26 PST
Landed with variety of platform fixes in http://trac.webkit.org/changeset/108444