Bug 75812 – JSC should be a triple-tier VM

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

This is a work in progress.

[Filip Pizlo]

<rdar://problem/10079694>

[Filip Pizlo]

Created attachment 121601 [details]
work in progress

Doesn't work yet, not ready for review.

[Filip Pizlo]

Created attachment 121607 [details]
work in progress

Added a chunk of the build logic, but not yet all of it.

[Filip Pizlo]

Created attachment 121617 [details]
work in progress

It's starting to build, sort of.

[Sam Weinig]

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

[Filip Pizlo]

(In reply to comment #5)
> Triple? Meh. Let's go to five -- http://onion.com/bWBRid.

I can't argue with you on that one. ;-)

[Filip Pizlo]

Created attachment 121624 [details]
work in progress

Wrote more glue.

[Filip Pizlo]

Created attachment 121744 [details]
work in progress

Getting closer to having it build.

[Filip Pizlo]

Created attachment 121769 [details]
work in progress

It now compiles.  But does not link.

[Filip Pizlo]

Created attachment 121809 [details]
work in progress

It compiles and links but there is still a tiny bit of glue to write.

[Filip Pizlo]

Created attachment 121915 [details]
work in progress

Rebased, and started to work on the yucky glue.

[Filip Pizlo]

Created attachment 121946 [details]
work in progress

Call linking is fixed up.  But still more work to do.

[Filip Pizlo]

Created attachment 122140 [details]
work in progress

[Filip Pizlo]

Created attachment 122163 [details]
work in progress

Did another rebase and wrote most (all?) of the remaining glue that is needed to get this to work.

Not sure if it builds at this point.  And I'm pretty sure that if it does, it'll crash in some humorous way.

[WebKit Review Bot]

Attachment 122163 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

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


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Comment on attachment 122163 [details]
work in progress

Clearing r? because I didn't mean to set it.

[Gyuyoung Kim]

Comment on attachment 122163 [details]
work in progress

Attachment 122163 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11213281

[Early Warning System Bot]

Comment on attachment 122163 [details]
work in progress

Attachment 122163 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/11222238

[Filip Pizlo]

Created attachment 122172 [details]
work in progress

It just ran "hello".

[pizlo@nitroflex OpenSource] WebKitBuild/Debug/jsc hello.js 
hello!
[pizlo@nitroflex OpenSource]

[Filip Pizlo]

Created attachment 122310 [details]
work in progress

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

[Filip Pizlo]

Created attachment 122371 [details]
work in progress

Tons of bug fixes.  It's starting to work, but there are still test failures.

[Filip Pizlo]

Created attachment 122701 [details]
work in progress

Fixed loads more regressions, but there's still more to go!

[Filip Pizlo]

Created attachment 122841 [details]
work in progress

It can now run javascriptcore-tests, SunSpider, V8, and Kraken.  Backing up before I rebase.

[Filip Pizlo]

Created attachment 122872 [details]
work in progress

This now runs SunSpider, V8, and Kraken in release mode.  It turns out that there was a bug in the offline assembler's interaction with the system linker, where the assembler's use of non-local but non-global labels (i.e. labels beginning with '_' but lacking a '.globl') was causing a hilarious bug: an entire snippet of code between two such labels disappeared sometime between when the inline assembly was passed to the C++ compiler and when it was loaded into memory when the binary ran!

Anyway, here's the performance of just the low-level interpreter (LLInt) without tiering support.

Remaining work includes:

- Running layout tests.  Not anticipating anything big there.

- Enabling tiering and enjoying the bug explosion.


Benchmark report for SunSpider, V8, and Kraken on oldmac (MacPro4,1).

VMs tested:
"TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc (r105216)
"LLInt" at /Volumes/Data/pizlo/senary/OpenSource/WebKitBuild/Release/jsc (r105216)
"OldInt" at /Volumes/Data/pizlo/septenary/OpenSource/WebKitBuild/Release/jsc (r105216)

Collected 12 samples per benchmark/VM, with 4 VM invocations per benchmark. Emitted a call to gc() between sample measurements. Used 1
benchmark iteration per VM invocation for warm-up. Used the jsc-specific preciseTime() function to get microsecond-level timing.
Reporting benchmark execution times with 95% confidence intervals in milliseconds.

                                            TipOfTree                 LLInt                   OldInt              OldInt v. TipOfTree    
SunSpider:
   3d-cube                                7.6638+-0.0505    !    20.0177+-0.0229    !    51.2581+-0.0747       ! definitely 6.6884x slower
   3d-morph                              14.0859+-0.0497    !    22.2417+-0.0705    !    63.8518+-0.1799       ! definitely 4.5330x slower
   3d-raytrace                           11.8198+-0.0278    !    32.3048+-0.2806    !    57.2185+-0.0874       ! definitely 4.8409x slower
   access-binary-trees                    2.2516+-0.0116    !    11.5333+-0.0726    !    17.5089+-0.0268       ! definitely 7.7761x slower
   access-fannkuch                       11.4253+-0.0212    !    51.4479+-1.7268    !   119.4145+-0.4435       ! definitely 10.4517x slower
   access-nbody                           6.8891+-0.0643    !    22.1227+-0.0842    !    56.7224+-0.0504       ! definitely 8.2336x slower
   access-nsieve                          3.8053+-0.0326    !    10.4458+-0.0395    !    25.1232+-0.0232       ! definitely 6.6021x slower
   bitops-3bit-bits-in-byte               1.5363+-0.0249    !    20.9110+-0.0530    !    42.8032+-0.0428       ! definitely 27.8613x slower
   bitops-bits-in-byte                    6.0055+-0.0246    !    27.4117+-0.7046    !    44.8535+-0.1989       ! definitely 7.4687x slower
   bitops-bitwise-and                     4.6773+-0.0433    !    22.7415+-0.1085    !    47.1811+-0.0167       ! definitely 10.0871x slower
   bitops-nsieve-bits                     8.2503+-0.0383    !    25.2988+-0.0695    !    60.1579+-0.0419       ! definitely 7.2916x slower
   controlflow-recursive                  3.5350+-0.0092    !    12.4984+-0.0564    !    26.1094+-0.1379       ! definitely 7.3861x slower
   crypto-aes                            11.2509+-0.1880    !    23.7763+-0.1513    !    40.7436+-0.1964       ! definitely 3.6214x slower
   crypto-md5                             3.4951+-0.0344    !    15.9365+-0.1302    !    27.5367+-0.1492       ! definitely 7.8786x slower
   crypto-sha1                            3.0708+-0.0170    !    14.6742+-0.0774    !    27.7720+-0.3279       ! definitely 9.0438x slower
   date-format-tofte                     14.1085+-0.1157    !    31.0783+-0.0933    ^    30.7639+-0.0352       ! definitely 2.1805x slower
   date-format-xparb                     14.4195+-0.1487    !    26.5260+-0.1848    ^    23.1269+-0.0614       ! definitely 1.6039x slower
   math-cordic                           12.1021+-0.0200    !    36.6978+-0.1130    !    70.9342+-0.3044       ! definitely 5.8613x slower
   math-partial-sums                     15.0148+-0.0252    !    22.0465+-0.0671    !    54.6326+-0.0661       ! definitely 3.6386x slower
   math-spectral-norm                     3.2588+-0.0080    !    18.0681+-1.2429    !    42.4210+-0.0473       ! definitely 13.0173x slower
   regexp-dna                            11.1681+-0.0469    ?    11.2181+-0.0782    !   233.8707+-0.7245       ! definitely 20.9410x slower
   string-base64                          5.7609+-0.0243    !    25.8767+-0.1465    !    27.5596+-0.1349       ! definitely 4.7839x slower
   string-fasta                          10.5268+-0.0296    !    24.8933+-0.2741    !    36.4612+-0.0171       ! definitely 3.4637x slower
   string-tagcloud                       16.5318+-0.0611    !    26.0234+-0.3133    !    45.0726+-0.6773       ! definitely 2.7264x slower
   string-unpack-code                    26.5009+-0.1188    !    30.0149+-0.2176    !    64.0745+-0.1974       ! definitely 2.4178x slower
   string-validate-input                  7.6353+-0.0251    !    17.3225+-0.0934    !    23.3600+-0.1823       ! definitely 3.0595x slower

   <arithmetic> *                         9.1073+-0.0249    !    23.1972+-0.1000    !    52.3282+-0.0400       ! definitely 5.7457x slower
   <geometric>                            7.3686+-0.0196    !    21.6519+-0.0729    !    43.7337+-0.0305       ! definitely 5.9351x slower
   <harmonic>                             5.7195+-0.0203    !    20.1591+-0.0641    !    38.5437+-0.0333       ! definitely 6.7391x slower

                                            TipOfTree                 LLInt                   OldInt              OldInt v. TipOfTree    
V8:
   crypto                               120.1883+-0.5165    !  1037.6748+-3.7260    !  2807.2836+-3.1412       ! definitely 23.3574x slower
   deltablue                            226.1341+-0.6206    !  2734.4194+-28.1251   ^  2493.4806+-10.8510      ! definitely 11.0266x slower
   earley-boyer                         143.4873+-1.6203    !   466.2559+-3.5907    !   789.9330+-4.2703       ! definitely 5.5052x slower
   raytrace                              68.8719+-0.2381    !   303.0984+-1.9271    !   353.9877+-5.8820       ! definitely 5.1398x slower
   regexp                               129.1626+-0.3948    !   162.7258+-0.5501    !  1202.7013+-13.1267      ! definitely 9.3115x slower
   richards                             240.3431+-1.1474    !  2640.3920+-12.0460   !  2896.8581+-5.3771       ! definitely 12.0530x slower
   splay                                120.6794+-1.3454    !   246.4478+-1.9810    !   289.9561+-8.3939       ! definitely 2.4027x slower

   <arithmetic>                         149.8381+-0.3944    !  1084.4306+-4.6843    !  1547.7429+-3.4411       ! definitely 10.3294x slower
   <geometric> *                        139.0662+-0.3535    !   636.7670+-2.0252    !  1102.1020+-4.9534       ! definitely 7.9250x slower
   <harmonic>                           128.6159+-0.2863    !   403.3166+-1.4094    !   738.4358+-7.3911       ! definitely 5.7414x slower

                                            TipOfTree                 LLInt                   OldInt              OldInt v. TipOfTree    
Kraken:
   ai-astar                             986.4699+-1.5020    !  5711.2708+-1.5013    ^  5656.3493+-4.4791       ! definitely 5.7339x slower
   audio-beat-detection                 300.4027+-2.5656    !  1544.7168+-1.8202    !  3461.4428+-8.8822       ! definitely 11.5227x slower
   audio-dft                            449.7360+-2.2174    !  1563.2987+-14.5969   !  2986.4066+-8.7263       ! definitely 6.6404x slower
   audio-fft                            188.6152+-0.6885    !  1491.1198+-2.0865    !  3378.8465+-4.2707       ! definitely 17.9140x slower
   audio-oscillator                     531.0049+-1.3694    !  1119.1909+-1.2375    !  2355.1291+-46.6195      ! definitely 4.4352x slower
   imaging-darkroom                     477.9397+-5.2400    !  1952.3063+-16.5109   !  4594.1441+-9.4076       ! definitely 9.6124x slower
   imaging-desaturate                   349.4066+-0.2174    !  3459.0173+-2.5981    !  6846.5305+-2.5816       ! definitely 19.5947x slower
   imaging-gaussian-blur                804.0863+-0.3510    ! 10101.1572+-114.0651  ! 28450.3464+-137.4128     ! definitely 35.3822x slower
   json-parse-financial                  84.1573+-0.1870    ?    84.3888+-0.5073         83.9558+-0.0540       
   json-stringify-tinderbox             127.6504+-0.3823        127.5610+-0.3344    ^   126.5883+-0.5533       ^ definitely 1.0084x faster
   stanford-crypto-aes                  165.0739+-0.5711    !   668.7007+-4.0118    !  1169.5522+-2.7745       ! definitely 7.0850x slower
   stanford-crypto-ccm                  148.1637+-0.5867    !   478.2820+-0.8416    !   805.8471+-1.0524       ! definitely 5.4389x slower
   stanford-crypto-pbkdf2               304.4971+-2.9252    !  1632.6897+-1.5222    !  2874.7453+-3.3609       ! definitely 9.4410x slower
   stanford-crypto-sha256-iterative     131.1675+-1.9117    !   598.6028+-0.7485    !   938.0714+-3.1941       ! definitely 7.1517x slower

   <arithmetic> *                       360.5980+-0.3558    !  2180.8788+-7.8414    !  4551.9968+-7.8917       ! definitely 12.6235x slower
   <geometric>                          280.6399+-0.3003    !  1126.2141+-0.9497    !  1946.9019+-2.5327       ! definitely 6.9374x slower
   <harmonic>                           220.8662+-0.2670    !   474.7984+-1.2485    !   554.6496+-0.7083       ! definitely 2.5112x slower

                                            TipOfTree                 LLInt                   OldInt              OldInt v. TipOfTree    
All benchmarks:
   <arithmetic>                         134.7665+-0.1527    !   823.9669+-1.8847    !  1615.3763+-2.1290       ! definitely 11.9865x slower
   <geometric>                           33.7496+-0.0654    !   116.2493+-0.2059    !   219.0713+-0.1464       ! definitely 6.4911x slower
   <harmonic>                            10.0778+-0.0350    !    35.1643+-0.1067    !    66.2642+-0.0545       ! definitely 6.5752x slower

                                            TipOfTree                 LLInt                   OldInt              OldInt v. TipOfTree    
Geomean of preferred means:
   <scaled-result>                       77.0094+-0.1378    !   318.1832+-0.5270    !   640.2995+-0.7543       ! definitely 8.3146x slower

[Filip Pizlo]

Created attachment 123042 [details]
work in progress

fast/js passes.

[Filip Pizlo]

Created attachment 123055 [details]
work in progress

fast/js, sputnik, and ietestcenter all pass now.

[Filip Pizlo]

Created attachment 123066 [details]
work in progress

Just got LLInt->oldJIT tiering to work.  And oldJIT->DFG tiering still works, too.  Going to do more tests and then it'll be time to commit.

[Filip Pizlo]

Created attachment 123089 [details]
work in progress

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

[Filip Pizlo]

Created attachment 123518 [details]
work in progress

Triple-tiering now works on SunSpider and run-javascript-core tests and does not cause regressions.  Not too shabby, since SunSpider was not the benchmark we were trying to progress on with this change.



Benchmark report for SunSpider on nitroflex (MacBookPro8,2).

VMs tested:
"TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc (r105584)
"LLInt" at /Volumes/Data/pizlo/septenary/OpenSource/WebKitBuild/Release/jsc (r105584)

Collected 12 samples per benchmark/VM, with 4 VM invocations per benchmark. Emitted a call to gc()
between sample measurements. Used 1 benchmark iteration per VM invocation for warm-up. Used the
jsc-specific preciseTime() function to get microsecond-level timing. Reporting benchmark execution
times with 95% confidence intervals in milliseconds.

                                 TipOfTree                 LLInt                                      

3d-cube                        5.6745+-0.1568    ?     5.8361+-0.1208       ? might be 1.0285x slower
3d-morph                      11.6210+-0.1631    ?    12.1069+-0.4708       ? might be 1.0418x slower
3d-raytrace                    8.9877+-0.2091    ?     9.0600+-0.2257       ?
access-binary-trees            1.7678+-0.0542          1.7197+-0.0319         might be 1.0280x faster
access-fannkuch                7.4614+-0.0947    ?     7.6526+-0.2332       ? might be 1.0256x slower
access-nbody                   5.2353+-0.1827    ^     4.7338+-0.1034       ^ definitely 1.1059x faster
access-nsieve                  2.9858+-0.0898          2.9686+-0.0710       
bitops-3bit-bits-in-byte       1.2672+-0.0291    ?     1.2888+-0.0322       ? might be 1.0171x slower
bitops-bits-in-byte            2.9313+-0.0499    ?     3.0038+-0.0783       ? might be 1.0247x slower
bitops-bitwise-and             2.8171+-0.1788    ?     2.9507+-0.2950       ? might be 1.0474x slower
bitops-nsieve-bits             6.5448+-0.3039    ?     6.5532+-0.3514       ?
controlflow-recursive          2.3911+-0.0410          2.3722+-0.0297       
crypto-aes                     8.3165+-0.1755          8.2634+-0.2232       
crypto-md5                     2.7578+-0.1087    !     3.0056+-0.0823       ! definitely 1.0899x slower
crypto-sha1                    2.4191+-0.0348          2.3804+-0.0404         might be 1.0163x faster
date-format-tofte             10.3733+-0.2814         10.2813+-0.1830       
date-format-xparb             11.3328+-0.7074         10.5627+-0.1865         might be 1.0729x faster
math-cordic                    8.0856+-0.0717    ?     8.0955+-0.1207       ?
math-partial-sums              9.6584+-0.2356          9.5466+-0.1533         might be 1.0117x faster
math-spectral-norm             2.4937+-0.0390    ?     2.4996+-0.0752       ?
regexp-dna                     7.8369+-0.1595          7.7539+-0.1160         might be 1.0107x faster
string-base64                  4.9288+-0.1364    ?     5.0067+-0.1325       ? might be 1.0158x slower
string-fasta                   7.7753+-0.1828          7.7097+-0.1765       
string-tagcloud               12.4376+-0.2461    ?    12.4669+-0.2336       ?
string-unpack-code            19.4631+-0.2433    ?    19.9456+-0.3127       ? might be 1.0248x slower
string-validate-input          6.3725+-0.1745          6.3249+-0.1717       

<arithmetic> *                 6.6899+-0.0577    ?     6.6957+-0.0426       ? might be 1.0009x slower
<geometric>                    5.4089+-0.0367    ?     5.4170+-0.0346       ? might be 1.0015x slower
<harmonic>                     4.2462+-0.0267    ?     4.2621+-0.0290       ? might be 1.0037x slower

[Filip Pizlo]

Created attachment 123633 [details]
work in progress

Can now run all major benchmarks in triple-tier mode.

[Filip Pizlo]

Created attachment 123701 [details]
work in progress

It works on 32-bit Mac.

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

[Filip Pizlo]

Created attachment 123783 [details]
work in progress

Made inlining of interpreted functions work.

[Filip Pizlo]

Created attachment 123822 [details]
work in progress

It completely works on 32-bit Mac now.  On to other platforms...

[Filip Pizlo]

Created attachment 123855 [details]
the patch

It's ready.

[WebKit Review Bot]

Attachment 123855 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Last 3072 characters of output:
aScriptCore/llint/LLIntHelpers.cpp:126:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:133:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:228:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:557:  One line control clauses should not use braces.  [whitespace/braces] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1313:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1346:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:43:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:41:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:55:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:57:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:68:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 29 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 123858 [details]
the patch

Fixed the real style errors.  The rest are false positives.

[WebKit Review Bot]

Attachment 123858 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1

Last 3072 characters of output:
se underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:120:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:126:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:133:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:228:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:41:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:55:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:57:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:68:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gavin Barraclough]

Comment on attachment 123858 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=123858&action=review

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:-62
> -#if !defined(NDEBUG) || ENABLE(OPCODE_SAMPLING)

This is enabling dumping in release builds? - I don't think we'll want all these strings in a release binary, this change should be reverted.

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1535
> +#if ENABLE(LLINT)    

I'm saddened to see yet another build flag – though this is clearly necessary until LLInt is ported to all platforms.

Looking toward the future, we may need to think about rationalizing down – typing together LLINT + JIT + DFG + MACRO_ASSEMBLER + YARR_JIT into a single brave-new-world-of-fast-JS-with-JITs switch.

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1754
> +    if (!interpreter->enabled()) {

This confuses me a little - it looks like if LLInt is being used, interpreter->enabled() will be true, does LLInt not use the structure slots embedded within the instruction stream? – a comment here would probably be useful. :-)

[Early Warning System Bot]

Comment on attachment 123858 [details]
the patch

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

[Gyuyoung Kim]

Comment on attachment 123858 [details]
the patch

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

[Filip Pizlo]

(In reply to comment #38)
> (From update of attachment 123858 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=123858&action=review
> 
> > Source/JavaScriptCore/bytecode/CodeBlock.cpp:-62
> > -#if !defined(NDEBUG) || ENABLE(OPCODE_SAMPLING)
> 
> This is enabling dumping in release builds? - I don't think we'll want all these strings in a release binary, this change should be reverted.

Should we really bothered by 824 bytes worth of strings and code?  The way C++ inlines template code like crazy, probably any change we make introduces that much.

Here's the size of a release build of the JavaScriptCore framework in ToT: 3578504 bytes
Here's the size of a release build with CodeBlock dumping compiled in: 3579328 bytes

So compiling in the dumping code leads to 824 bytes *total* increase in the framework size.  That's strings and code, combined.  Another way to put it is that it's a 0.02% increase - hardly noticable.

It's pretty much routine for me at this point to have to enable this dumping thingy in release builds when dealing with release-only crashes.  It's really annoying, and the NDEBUG checks are super annoying.  I agree with your comment below that we should strive to reduce the amount of #if madness in our code.

Are we really willing to have #if NDEBUG statements in a bunch of places to protect against having a 0.02% increase in code size?

> 
> > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1535
> > +#if ENABLE(LLINT)    
> 
> I'm saddened to see yet another build flag – though this is clearly necessary until LLInt is ported to all platforms.
> 
> Looking toward the future, we may need to think about rationalizing down – typing together LLINT + JIT + DFG + MACRO_ASSEMBLER + YARR_JIT into a single brave-new-world-of-fast-JS-with-JITs switch.
> 
> > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1754
> > +    if (!interpreter->enabled()) {
> 
> This confuses me a little - it looks like if LLInt is being used, interpreter->enabled() will be true, does LLInt not use the structure slots embedded within the instruction stream? – a comment here would probably be useful. :-)

interpreter->enabled() means that LLInt was compiled in but we ended up using the old interpreter instead, a mode that I still support because there's probably a

[Filip Pizlo]

Created attachment 123880 [details]
the patch

Attempt to fix Qt/EFL/GTK builds.

[Filip Pizlo]

(In reply to comment #42)
> Created an attachment (id=123880) [details]
> the patch
> 
> Attempt to fix Qt/EFL/GTK builds.

And also addressed Gavin's comment about not having comments in the !interpreter->enabled() check.

[WebKit Review Bot]

Attachment 123880 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
se underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:120:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:126:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:133:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:228:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:41:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:55:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:57:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:68:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 97 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 123880 [details]
the patch

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

[Early Warning System Bot]

Comment on attachment 123880 [details]
the patch

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

[Gustavo Noronha (kov)]

Comment on attachment 123880 [details]
the patch

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

[Filip Pizlo]

Created attachment 123970 [details]
the patch

Fixed bunch of builds.

[WebKit Review Bot]

Attachment 123970 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
se underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:120:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:126:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:133:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:228:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:41:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:55:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:57:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:68:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 99 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 123970 [details]
the patch

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

[Filip Pizlo]

Created attachment 124199 [details]
the patch

Fix build on EFL, Windows.

[WebKit Review Bot]

Attachment 124199 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:120:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:126:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:133:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:228:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:41:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:55:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:57:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:68:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 101 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 124201 [details]
the patch

Cleaned up the comment style that I used in offlineasm.

[WebKit Review Bot]

Attachment 124201 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:120:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:126:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:133:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:228:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:41:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:55:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:57:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:68:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 101 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 124201 [details]
the patch

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

[Filip Pizlo]

Created attachment 124225 [details]
the patch

Trying to fix the build more.

[Filip Pizlo]

Created attachment 124229 [details]
the patch

Rebased.

[WebKit Review Bot]

Attachment 124229 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:120:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:126:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:133:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:228:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:65:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:46:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:48:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:54:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:67:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 104 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 124248 [details]
the patch

Another rebase!

[WebKit Review Bot]

Attachment 124248 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:120:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:126:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:133:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:228:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:64:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:46:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:48:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:54:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:67:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 103 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 124248 [details]
the patch

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

[Filip Pizlo]

Created attachment 124360 [details]
the patch

Again trying to fix EFL and Windows build.  Maybe I got it this time? ;-)

[WebKit Review Bot]

Attachment 124360 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:120:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:126:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:133:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:228:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:64:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:48:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:50:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:69:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:33:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 105 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 124360 [details]
the patch

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

[Filip Pizlo]

Created attachment 124372 [details]
the patch

- More EFL build fixes.

- Made the offline assembler capable of hashing itself for depedency tracking.  So if you change the offline assembler, then it will realize that it must rebuild LLInt.

- Made the offline assembler use InlineASM.h's macros.

[Filip Pizlo]

Created attachment 124373 [details]
LLIntAssembly.h

Generated assembly code with latest offlineasm changes.

[Filip Pizlo]

(In reply to comment #66)
> Created an attachment (id=124373) [details]
> LLIntAssembly.h
> 
> Generated assembly code with latest offlineasm changes.

Hajime, Simon, can you check that the LLIntAssembly.h file does the symbol magic consistently with the changes you guys are making?

Note that I had to introduce a new macro in InlineASM.h to create local labels.  Currently I only have this working on Mac, but it should be easy to expand support for this to other platforms in future.

[Filip Pizlo]

Created attachment 124374 [details]
the patch

Another rebase!

[WebKit Review Bot]

Attachment 124374 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:120:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:126:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:133:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:228:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1312:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1345:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:64:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 108 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Hajime Morrita]

(In reply to comment #67)
> (In reply to comment #66)
> > Created an attachment (id=124373) [details] [details]
> > LLIntAssembly.h
> > 
> > Generated assembly code with latest offlineasm changes.
> 
> Hajime, Simon, can you check that the LLIntAssembly.h file does the symbol magic consistently with the changes you guys are making?

Filip, Thanks for caring.
It looks good to me although Simon should know more.
Because the generated code is using same machinery as existing code, it should work fine.
And the code generator looks straightforward enough to fix possible problem later
without significant impact.

[Filip Pizlo]

Created attachment 124432 [details]
the patch, sort of

This patch may be slightly polluted by debug garbage, as well as a bugfix that is to be reviewed separately.  I will "remove" that bugfix from this patch by rebasing after that one lands.

[WebKit Review Bot]

Attachment 124432 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:120:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:126:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:133:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:228:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1316:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1349:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:64:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 112 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 124478 [details]
the patch

Includes beginnings of proper ARMv7 support in the offlineasm backend. This is done by doing a series of micropasses that break down the larger operations (like say "addi 5000, 4[foo, bar, 8]") into smaller ones ("move 4, tmp1; addp foo, tmp1; loadi [tmp1, bar, 8], tmp2; move 5000, tmp3; addi tmp3, tmp2; storei tmp2, [tmp1, bar, 8]").

Also includes a build fix in tools/; I'll land that build fix separately and exclude it from this patch in the next rebase.

[WebKit Review Bot]

Attachment 124478 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:120:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:126:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:133:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:228:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1316:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1349:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:64:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 111 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 124496 [details]
the patch

Rabased, and added more ARMv7 code

[WebKit Review Bot]

Attachment 124496 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:120:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:126:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:133:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:228:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1316:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1349:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:64:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:34:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 110 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Simon Hausmann]

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

Looks good AFAICS :)

Simon

[Filip Pizlo]

Created attachment 124608 [details]
the patch

- Rebased for new way of dealing with getters and setters.

- More ARMv7 stuff.

[WebKit Review Bot]

Attachment 124608 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:121:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:128:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:136:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:229:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1322:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1355:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 110 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 124619 [details]
the patch

- complete (though untested) implementation of ARMv7 and another rebase

[WebKit Review Bot]

Attachment 124619 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:121:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:127:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:128:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:134:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:135:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:136:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:229:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1322:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1355:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 110 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 124646 [details]
work in progress

Doing a bunch of hacking to get ARMv7 to work.  I'm almost there.  Backing up.

[Filip Pizlo]

Created attachment 124872 [details]
the patch

ARM is starting to work.

[WebKit Review Bot]

Attachment 124872 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:123:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:129:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:130:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:136:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:137:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:138:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:231:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1412:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1449:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 111 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 124875 [details]
the patch

Realized that the previous patch had some unnecessary printf's.

Other than that - still working on fleshing out ARM support.

[WebKit Review Bot]

Attachment 124875 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:123:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:129:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:130:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:136:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:137:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:138:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:231:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1412:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1449:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 111 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 124888 [details]
the patch

ARM works in LLInt-only config.  Haven't tried LLInt+JIT yet, but I'm guessing it'll just work.

[Filip Pizlo]

Created attachment 125035 [details]
the patch

Rebased again.  We can now run LLInt+JIT on ARM.

[WebKit Review Bot]

Attachment 125035 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:123:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:129:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:130:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:136:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:137:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:138:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:231:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1412:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1449:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:42:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:51:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:60:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:71:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 111 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 125085 [details]
the patch

Made SunSpider, V8, and Kraken run on ARM.

To do this I had to make more debug printing work in release builds. :-(

[WebKit Review Bot]

Attachment 125085 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:123:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:129:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:130:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:136:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:137:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:138:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:231:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1413:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1450:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:69:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 111 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

(In reply to comment #90)
> Created an attachment (id=125085) [details]
> the patch
> 
> Made SunSpider, V8, and Kraken run on ARM.
> 
> To do this I had to make more debug printing work in release builds. :-(

Never mind, bad patch.  I left in some horrible debug magic.  I will retest without it and repost.

[Gyuyoung Kim]

Comment on attachment 125085 [details]
the patch

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

[Filip Pizlo]

Created attachment 125091 [details]
the patch

- Fixed windows/efl, hopefully.

- Removed some really dumb debug stuff.

[WebKit Review Bot]

Attachment 125091 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:123:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:129:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:130:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:136:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:137:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:138:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:231:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1413:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1450:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:69:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 112 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 125186 [details]
the patch

Rebased to include the release debugging changes.

[WebKit Review Bot]

Attachment 125186 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:123:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:129:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:130:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:136:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:137:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:138:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:231:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1413:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1450:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:69:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 111 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 125194 [details]
the patch

Optimized generate_offset_extractor.rb.  It now runs 10x faster.

[Filip Pizlo]

Created attachment 125715 [details]
the patch

Another rebase.

[WebKit Review Bot]

Attachment 125715 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:123:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:129:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:130:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:136:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:137:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:138:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:231:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1413:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1450:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:69:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 112 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 125735 [details]
the patch

Rebased and switched to using 4 space indentation in Ruby code.

[WebKit Review Bot]

Attachment 125735 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1

Last 3072 characters of output:
e underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:123:  __ct_globalData is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:129:  __cce_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:130:  __cce_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:136:  __cr_exec is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:137:  __cr_pc is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:138:  __cr_callTarget is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:231:  The parameter name "]" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1413:  The parameter name "v" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/llint/LLIntHelpers.cpp:1450:  The parameter name "!" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/StructureChain.h:40:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/runtime/JSTypeInfo.h:37:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:70:  Alphabetical sorting problem.  [build/include_order] [4]
Source/JavaScriptCore/runtime/JSVariableObject.h:41:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:40:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:47:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:49:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:56:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:58:  Extra space before ( in function call  [whitespace/parens] [4]
Source/JavaScriptCore/jit/HostCallReturnValue.cpp:69:  The parameter name """" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/JavaScriptCore/runtime/Structure.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 27 in 112 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 127001 [details]
the patch

Rebased again.

[WebKit Review Bot]

Attachment 127001 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
	M	ChangeLog
	A	LayoutTests/fast/css/style-scoped/registering-shadowroot-expected.txt
	A	LayoutTests/fast/css/style-scoped/registering-shadowroot.html
	M	LayoutTests/ChangeLog
	M	Source/WebCore/WebCore.exp.in
	M	Source/WebCore/testing/Internals.cpp
	M	Source/WebCore/testing/Internals.h
	M	Source/WebCore/testing/Internals.idl
	M	Source/WebCore/dom/ShadowRootList.h
	M	Source/WebCore/dom/ShadowRootList.cpp
	M	Source/WebCore/dom/Element.cpp
	M	Source/WebCore/dom/Node.h
	M	Source/WebCore/dom/Element.h
	M	Source/WebCore/dom/NodeRareData.h
	M	Source/WebCore/dom/Node.cpp
	M	Source/WebCore/dom/ElementRareData.h
	M	Source/WebCore/ChangeLog
	M	Source/WebCore/platform/graphics/FontCache.cpp
	M	Source/WebCore/html/HTMLStyleElement.cpp
	M	Source/autotools/symbols.filter
	M	Source/WebKit2/ChangeLog
	M	Source/WebKit2/win/WebKit2CFLite.def
	M	Source/WebKit2/win/WebKit2.def
r107794 = c59b9dab74417b86c7b5b60a4408ab3e5d1659a8 (refs/remotes/origin/master)
First, rewinding head to replay your work on top of it...
Applying: [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets
Using index info to reconstruct a base tree...
<stdin>:1578: trailing whitespace.
        
<stdin>:1647: trailing whitespace.
    
<stdin>:1657: trailing whitespace.
    
<stdin>:1672: trailing whitespace.
        return 0;        
<stdin>:1674: trailing whitespace.
    
warning: squelched 7 whitespace errors
warning: 12 lines add whitespace errors.
Falling back to patching base and 3-way merge...
warning: too many files (created: 168755 deleted: 3), skipping inexact rename detection
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/wk2/Skipped
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Auto-merging Source/WebCore/css/CSSCalculationValue.cpp
Auto-merging Source/WebCore/css/CSSCalculationValue.h
Auto-merging Source/WebCore/css/CSSParser.cpp
Auto-merging Source/WebKit/mac/ChangeLog
CONFLICT (content): Merge conflict in Source/WebKit/mac/ChangeLog
Auto-merging Source/WebKit2/ChangeLog
CONFLICT (content): Merge conflict in Source/WebKit2/ChangeLog
Auto-merging Tools/ChangeLog
CONFLICT (content): Merge conflict in Tools/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 [Mac][Win][WK2] Switch to RFC 6455 protocol for WebSockets

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gavin Barraclough]

Comment on attachment 127001 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=127001&action=review

Okay, this basically all looks good.  Couple small of issues:

Please revert JSActivation.h & Heap.cpp (whitespace only changes).

Please rename Interpreter::enabled() (to make it clear which interpreter this is enabling - e.g. 'classicEnabled()' would be fine.)

Per our discussion, I think 'Helpers' isn't a great name, since it isn't descriptive of what it helps.  I'm happy with your suggested 'SlowPaths'.

It's a shame that LLIntOffsetsExtractor is LLInt specific – it seems this pattern could be generalized to provide offsets into classes for use by the JITs, too, and reduce the friendliness of the codebase.  But I'm not asking for this change in this patch, it's fine as is.

> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:70
> +# These declarations must match interpreter/RegisterFile.h.

Are these constants defended anywhere by compile time ASSERTs?

[Filip Pizlo]

(In reply to comment #105)
> (From update of attachment 127001 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=127001&action=review
> 
> Okay, this basically all looks good.  Couple small of issues:

Thanks! :-)

> 
> Please revert JSActivation.h & Heap.cpp (whitespace only changes).

Ack.

> 
> Please rename Interpreter::enabled() (to make it clear which interpreter this is enabling - e.g. 'classicEnabled()' would be fine.)

Good idea.

> 
> Per our discussion, I think 'Helpers' isn't a great name, since it isn't descriptive of what it helps.  I'm happy with your suggested 'SlowPaths'.

Will do.

> 
> It's a shame that LLIntOffsetsExtractor is LLInt specific – it seems this pattern could be generalized to provide offsets into classes for use by the JITs, too, and reduce the friendliness of the codebase.  But I'm not asking for this change in this patch, it's fine as is.

Agree.

> 
> > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:70
> > +# These declarations must match interpreter/RegisterFile.h.
> 
> Are these constants defended anywhere by compile time ASSERTs?

Unfortunately not. :-(  I will add these to LLIntData.cpp, since LLInt initialization funnels through there.

I'd like to come up with a way of having these constants get extracted similarly to how we do offsets and sizes.  I'm not there yet. :-(

[Filip Pizlo]

Created attachment 127706 [details]
the patch

Rebasing and slowly incorporating Gavin's comments.  Putting up for EWS.

[Filip Pizlo]

Created attachment 127707 [details]
the patch

Addressed Gavin's concerns.  Will have to do more testing before landing.  Putting up for EWS to let it simmer a bit.

[Filip Pizlo]

Created attachment 127825 [details]
the patch

Rebasing.

[Filip Pizlo]

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

[Csaba Osztrogonác]

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

[Csaba Osztrogonác]

.

[Filip Pizlo]

Created attachment 127932 [details]
fix for elf

Fix for ELF.  Can Qt/Gtk people check if this fixes things?

[Filip Pizlo]

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

[Filip Pizlo]

Comment on attachment 127932 [details]
fix for elf

Clearing review flag because this was already landed.

[Csaba Osztrogonác]

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

[Filip Pizlo]

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

[Csaba Osztrogonác]

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

It fixed the crashes on Qt Linux debug, and fixes at leasat the non-DFG Qt Windows build.

[Filip Pizlo]

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

[Adam Roben (:aroben)]

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

[Filip Pizlo]

Created attachment 128019 [details]
the patch

Fat binary build fixes.

[Filip Pizlo]

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