Bug 90347

Summary:

Inline property storage should not be wasted when it is exhausted

Product:

WebKit

Reporter:

Filip Pizlo <fpizlo>

Component:

JavaScriptCore

Assignee:

Filip Pizlo <fpizlo>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ararunprasad, barraclough, d-r, fpizlo, ggaren, gstephan, gustavo, mhahnenberg, oliver, ossy, prem_x87, thouraya.andolsi, webkit.review.bot, xan.lopez, zherczeg

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

90657, 90658, 90664

Bug Blocks:

Attachments:

Description	Flags
it begins	none
more!	none
it grows!	none
the JIT has been attacked	none
prop storage load patching is implemented	none
a lot more code	none
property name iteration done right	none
wrote some 64-bit thingies	none
fixing some compiler errors	none
it compiles!	none
I proved it correct	none
it might work	gyuyoung.kim: commit-queue-
it works on x86-32, x86-64, and classic interpreter	buildbot: commit-queue-
the patch	none
the patch	none
the patch	buildbot: commit-queue-
the patch	none
the patch	buildbot: commit-queue-
the patch	buildbot: commit-queue-
the patch	none
the patch	buildbot: commit-queue-
the patch	none
the patch	barraclough: review+

Filip Pizlo

Reported 2012-06-30 23:58:03 PDT

Currently JSFinalObjects (i.e. vanilla JS objects created via 'new Object', '{}', or constructor calls to user-defined functions) will have inline property storage, enough for at least 4 properties. This is a great optimization since it means that for most objects we don't need to perform a second allocation to create property storage, and accesses to properties of small objects are fast. But things go wrong when the inline property storage is exhausted: we allocate out-of-line storage and move all properties that were inline into the out-of-line storage. This simplifies property accesses, as it implies that it's always safe to say object->propertyStorage[offset], where object->propertyStorage points to either the inline storage or the out-of-line storage depending on the object size. But this simplicity comes with three costs: first, wasted memory since the inline storage slots are not reclaimed; and second, execution time penalty when the first out-of-line property is stored since all of the inline properties have to be copied to the out-of-line storage; and third, increased execution time cost for accessing the properties that were previously inline. A better approach would be to continue to use the inline property storage even when it is exhausted: the first N properties will always be inline and the remainder will be out-of-line. Since our inline property storage size is tuned for common object sizes, this means that the vast majority of properties in the heap will tend to be inline and will be fast to access. This will be a performance improvement over the current approach, where as soon as any property overflows the inline storage, all properties are evicted and become more expensive to access.

Attachments
it begins (59.20 KB, patch) 2012-07-01 00:02 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
more! (79.84 KB, patch) 2012-07-01 13:20 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
it grows! (101.99 KB, patch) 2012-07-01 15:50 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the JIT has been attacked (121.36 KB, patch) 2012-07-01 16:09 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
prop storage load patching is implemented (128.16 KB, patch) 2012-07-01 16:53 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
a lot more code (172.23 KB, patch) 2012-07-01 18:24 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
property name iteration done right (181.18 KB, patch) 2012-07-02 01:03 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
wrote some 64-bit thingies (199.97 KB, patch) 2012-07-02 21:40 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
fixing some compiler errors (213.95 KB, patch) 2012-07-02 22:40 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
it compiles! (217.31 KB, patch) 2012-07-02 23:10 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
I proved it correct (217.38 KB, patch) 2012-07-02 23:49 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
it might work (237.15 KB, patch) 2012-07-03 15:34 PDT, Filip Pizlo	gyuyoung.kim: commit-queue-	Details Formatted Diff Diff
it works on x86-32, x86-64, and classic interpreter (249.48 KB, patch) 2012-07-03 18:03 PDT, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
the patch (253.99 KB, patch) 2012-07-04 00:47 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (253.55 KB, patch) 2012-07-04 01:02 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (253.40 KB, patch) 2012-07-04 01:14 PDT, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
the patch (256.16 KB, patch) 2012-07-04 12:58 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (256.16 KB, patch) 2012-07-04 12:59 PDT, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
the patch (256.81 KB, patch) 2012-07-04 13:59 PDT, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
the patch (257.68 KB, patch) 2012-07-04 14:41 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (257.70 KB, patch) 2012-07-04 16:34 PDT, Filip Pizlo	buildbot: commit-queue-	Details Formatted Diff Diff
the patch (257.88 KB, patch) 2012-07-04 19:12 PDT, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (258.97 KB, patch) 2012-07-04 19:44 PDT, Filip Pizlo	barraclough: review+	Details Formatted Diff Diff
Show Obsolete (22) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2012-07-01 00:02:18 PDT

Created attachment 150328 [details] it begins Still a lot more to do. So far I've written probably most of the glue necessary for the 32-bit LLInt to work, and have modified most of the runtime. But I still need to find all of the remaining places where the offset logic needs changing, and I still need to tweak the other execution engines. That will probably be a lot of work, and there's no choice but to do it in one go.

Filip Pizlo

Comment 2 2012-07-01 13:20:26 PDT

Created attachment 150335 [details] more!

Filip Pizlo

Comment 3 2012-07-01 15:50:54 PDT

Created attachment 150339 [details] it grows!

Filip Pizlo

Comment 4 2012-07-01 16:09:38 PDT

Created attachment 150340 [details] the JIT has been attacked But I still need to do the horrific load-to-nop patching code. And I still need to write the same code for the old JIT.

Filip Pizlo

Comment 5 2012-07-01 16:53:05 PDT

Created attachment 150346 [details] prop storage load patching is implemented The X86 version of the code to flip the out-of-line storage load between being active and inactive is written. I decided to go for the load-to-lea trick that Gavin suggested.

Filip Pizlo

Comment 6 2012-07-01 18:24:42 PDT

Created attachment 150351 [details] a lot more code Property name iteration ... it's special.

Filip Pizlo

Comment 7 2012-07-02 01:03:02 PDT

Created attachment 150377 [details] property name iteration done right The glue for 32-bit is almost done! Oddly, I'm doing 32-bit first this time; I'll do 64-bit after 32-bit starts working.

Filip Pizlo

Comment 8 2012-07-02 11:22:27 PDT

Zoltan, I've CC'd you because this will require new assembler support: we'll need to be able to turn a loadPtr(Address(register1, constant), register2) into a addPtr(constant, register1, register2) and vice-versa in RepatchBuffer. This patch doesn't build yet and is still in the relatively early stages. I hope to have it finished in a day or so.

Filip Pizlo

Comment 9 2012-07-02 21:40:50 PDT

Created attachment 150530 [details] wrote some 64-bit thingies It still probably doesn't compile.

Filip Pizlo

Comment 10 2012-07-02 22:40:41 PDT

Created attachment 150535 [details] fixing some compiler errors But of course it doesn't compile yet!

Filip Pizlo

Comment 11 2012-07-02 23:10:37 PDT

Created attachment 150540 [details] it compiles! In 64-bit mode anyway.

Filip Pizlo

Comment 12 2012-07-02 23:49:18 PDT

Created attachment 150546 [details] I proved it correct by running "hello world".

Filip Pizlo

Comment 13 2012-07-03 15:34:23 PDT

Created attachment 150680 [details] it might work Still doing some more testing though.

WebKit Review Bot

Comment 14 2012-07-03 15:38:00 PDT

Attachment 150680 [details] did not pass style-queue: Source/JavaScriptCore/runtime/PropertyMapHashTable.h:179: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyMapHashTable.h:181: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:375: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:376: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:396: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "chain" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:399: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:90: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:91: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:92: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PutPropertySlot.h:67: Place brace on its own line for function definitions. [whitespace/braces] [4] Source/JavaScriptCore/runtime/Options.cpp:137: Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:49: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:50: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:50: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:51: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:52: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:52: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:53: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:54: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:55: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:56: The parameteFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 r name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:57: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:58: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:59: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:60: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:60: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:61: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:61: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:62: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:102: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 36 in 50 files If any of these errors are false positives, please file a bug against check-webkit-style.

Gyuyoung Kim

Comment 15 2012-07-03 15:41:08 PDT

Comment on attachment 150680 [details] it might work Attachment 150680 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13124661

Gustavo Noronha (kov)

Comment 16 2012-07-03 15:44:03 PDT

Comment on attachment 150680 [details] it might work Attachment 150680 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13139304

Build Bot

Comment 17 2012-07-03 15:56:42 PDT

Comment on attachment 150680 [details] it might work Attachment 150680 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13127629

Early Warning System Bot

Comment 18 2012-07-03 16:03:18 PDT

Comment on attachment 150680 [details] it might work Attachment 150680 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13126593

Early Warning System Bot

Comment 19 2012-07-03 16:11:07 PDT

Comment on attachment 150680 [details] it might work Attachment 150680 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13131598

Build Bot

Comment 20 2012-07-03 16:53:21 PDT

Comment on attachment 150680 [details] it might work Attachment 150680 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13133548

Filip Pizlo

Comment 21 2012-07-03 18:03:35 PDT

Created attachment 150701 [details] it works on x86-32, x86-64, and classic interpreter Still working on other backends.

WebKit Review Bot

Comment 22 2012-07-03 18:07:20 PDT

Attachment 150701 [details] did not pass style-queue: Source/JavaScriptCore/runtime/PropertyMapHashTable.h:179: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyMapHashTable.h:181: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:375: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:376: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:396: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "chain" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:399: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:90: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:91: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:92: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PutPropertySlot.h:67: Place brace on its own line for function definitions. [whitespace/braces] [4] Source/JavaScriptCore/runtime/Options.cpp:137: Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:49: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:50: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:50: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:51: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:52: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:52: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:53: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:54: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:55: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:56: The parameteFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 r name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:57: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:58: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:59: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:60: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:60: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:61: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:61: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:62: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:102: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 36 in 56 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 23 2012-07-03 18:27:37 PDT

Comment on attachment 150701 [details] it works on x86-32, x86-64, and classic interpreter Attachment 150701 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/13125663

Build Bot

Comment 24 2012-07-03 18:28:03 PDT

Comment on attachment 150701 [details] it works on x86-32, x86-64, and classic interpreter Attachment 150701 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13133579

Filip Pizlo

Comment 25 2012-07-04 00:47:29 PDT

Created attachment 150729 [details] the patch

WebKit Review Bot

Comment 26 2012-07-04 00:52:45 PDT

Attachment 150729 [details] did not pass style-queue: Source/JavaScriptCore/runtime/PropertyMapHashTable.h:179: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyMapHashTable.h:181: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:375: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:376: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:396: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "chain" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:399: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:90: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:91: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:92: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PutPropertySlot.h:67: Place brace on its own line for function definitions. [whitespace/braces] [4] Source/JavaScriptCore/runtime/Options.cpp:137: Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:49: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:50: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:50: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:51: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:52: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:52: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:53: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:54: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:55: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:56: The parameteFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 r name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:57: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:58: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:59: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:60: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:60: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:61: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:61: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:62: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:102: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 36 in 60 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 27 2012-07-04 01:02:42 PDT

Created attachment 150732 [details] the patch

WebKit Review Bot

Comment 28 2012-07-04 01:07:21 PDT

Attachment 150732 [details] did not pass style-queue: Source/JavaScriptCore/runtime/PropertyMapHashTable.h:179: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyMapHashTable.h:181: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:375: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:376: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:396: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "chain" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:399: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:90: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:91: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Structure.h:92: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PutPropertySlot.h:67: Place brace on its own line for function definitions. [whitespace/braces] [4] Source/JavaScriptCore/runtime/Options.cpp:137: Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:49: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:50: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:50: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:51: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:52: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:52: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:53: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:54: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:55: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:56: The parameteFailed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 r name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:57: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:58: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:59: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:60: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:60: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:61: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:61: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/PropertyOffset.h:62: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:102: The parameter name "offset" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 36 in 59 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 29 2012-07-04 01:14:32 PDT

Created attachment 150736 [details] the patch Fix style issues.

WebKit Review Bot

Comment 30 2012-07-04 01:16:46 PDT

Attachment 150736 [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 Source/JavaScriptCore/jit/JIT.h:375: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:376: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:396: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "chain" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:399: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Options.cpp:137: Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Total errors found: 11 in 59 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 31 2012-07-04 01:26:01 PDT

Zoltan: I've finalized the approach of repatching loadPtr(). There's now a convertibleLoadPtr(), which can be converted into an address computation. It should be a straight-forward API to implement, though it is necessarily gross.

Build Bot

Comment 32 2012-07-04 01:51:15 PDT

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

Build Bot

Comment 33 2012-07-04 02:00:40 PDT

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

Filip Pizlo

Comment 34 2012-07-04 12:58:23 PDT

Created attachment 150835 [details] the patch Fixing build errors.

Filip Pizlo

Comment 35 2012-07-04 12:59:43 PDT

Created attachment 150836 [details] the patch Just rebasing.

WebKit Review Bot

Comment 36 2012-07-04 13:04:14 PDT

Attachment 150836 [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 Source/JavaScriptCore/jit/JIT.h:375: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:376: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:396: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "chain" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:399: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Options.cpp:137: Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Total errors found: 11 in 60 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 37 2012-07-04 13:23:33 PDT

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

Build Bot

Comment 38 2012-07-04 13:58:13 PDT

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

Filip Pizlo

Comment 39 2012-07-04 13:59:18 PDT

Created attachment 150841 [details] the patch More build fixes.

WebKit Review Bot

Comment 40 2012-07-04 14:01:12 PDT

Attachment 150841 [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 Source/JavaScriptCore/jit/JIT.h:375: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:376: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:396: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "chain" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:399: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Options.cpp:138: Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Total errors found: 11 in 60 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 41 2012-07-04 14:28:34 PDT

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

Filip Pizlo

Comment 42 2012-07-04 14:41:24 PDT

Created attachment 150843 [details] the patch Even more build fixes.

WebKit Review Bot

Comment 43 2012-07-04 14:45:12 PDT

Attachment 150843 [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 Source/JavaScriptCore/jit/JIT.h:375: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:376: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:396: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "chain" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:399: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Options.cpp:138: Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Total errors found: 11 in 61 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 44 2012-07-04 16:34:26 PDT

Created attachment 150851 [details] the patch More build fixes. Apparently, the compiler the Mac build bot uses has a whole bunch of pedantry I've magically managed to avoid on my install.

WebKit Review Bot

Comment 45 2012-07-04 16:36:47 PDT

Attachment 150851 [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 Source/JavaScriptCore/jit/JIT.h:375: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:376: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:396: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "chain" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:399: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Options.cpp:138: Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Total errors found: 11 in 61 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 46 2012-07-04 17:15:27 PDT

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

Filip Pizlo

Comment 47 2012-07-04 19:12:03 PDT

Created attachment 150858 [details] the patch Ugh more build fixes.

WebKit Review Bot

Comment 48 2012-07-04 19:15:54 PDT

Attachment 150858 [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 Source/JavaScriptCore/jit/JIT.h:375: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:376: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:396: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "chain" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:399: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Options.cpp:138: Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Total errors found: 11 in 61 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 49 2012-07-04 19:44:30 PDT

Created attachment 150861 [details] the patch Maybe it'll actually build this time!

WebKit Review Bot

Comment 50 2012-07-04 19:51:15 PDT

Attachment 150861 [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 Source/JavaScriptCore/jit/JIT.h:375: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:376: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:394: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:396: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "chain" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:397: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:398: The parameter name "callFrame" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/jit/JIT.h:399: The parameter name "returnAddress" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/Options.cpp:138: Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Total errors found: 11 in 61 files If any of these errors are false positives, please file a bug against check-webkit-style.

Zoltan Herczeg

Comment 51 2012-07-04 20:52:14 PDT

I''l handle the ARM port when the patch is landed.

Gavin Barraclough

Comment 52 2012-07-05 14:18:54 PDT

Comment on attachment 150861 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=150861&action=review Agreed that that it would be nice to consider a class in the future, and even though it's not from code you added it would be good to fix the preexisting style issues re duplicate names. > Source/JavaScriptCore/runtime/Options.cpp:121 > +#define FOR_EACH_OPTION(type_, name_, defaultValue_) \ extra whitespace! r-! >> Source/JavaScriptCore/runtime/Options.cpp:138 >> +#endif > > Line contains only semicolon. If this should be an empty statement, use { } instead. [whitespace/semicolon] [5] Technically we don't allow #if 0 code, so this is on pretty shaky ground, but Im prepared to look the other way if this helps.

Filip Pizlo

Comment 53 2012-07-05 15:56:55 PDT

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

Csaba Osztrogonác

Comment 54 2012-07-05 21:58:27 PDT

(In reply to comment #53) > Landed in http://trac.webkit.org/changeset/121925 FYI: I filed a bug for ARM fix - https://bugs.webkit.org/show_bug.cgi?id=90657 , a bug for 32 bit regression - https://bugs.webkit.org/show_bug.cgi?id=90658 and landed a 64 bit buildfix - https://trac.webkit.org/changeset/121943

Filip Pizlo

Comment 55 2012-07-06 00:03:21 PDT

(In reply to comment #54) > (In reply to comment #53) > > Landed in http://trac.webkit.org/changeset/121925 > > FYI: I filed a bug for ARM fix - https://bugs.webkit.org/show_bug.cgi?id=90657 , > a bug for 32 bit regression - https://bugs.webkit.org/show_bug.cgi?id=90658 and > landed a 64 bit buildfix - https://trac.webkit.org/changeset/121943 Thanks for the 64 bit build fix! The 32-bit regression should be fixed as of http://trac.webkit.org/changeset/121946. Good luck with the ARM side and let me know if you need clarification of the new assembler functionality.

Csaba Osztrogonác

Comment 56 2012-07-06 01:35:55 PDT

I filed one more bug for PerformanceTests/Dromaeo/cssquery-prototype.html crash - https://bugs.webkit.org/show_bug.cgi?id=90664

Dominik Röttsches (drott)

Comment 57 2012-07-06 03:34:57 PDT

If I am not mistaken, the changes to JSObject.h in this page cause a lot of warnings in debug builds, like: Source/JavaScriptCore/runtime/JSObject.h: In member function ‘void JSC::JSFinalObject::finishCreation(JSC::JSGlobalData&)’: Source/JavaScriptCore/runtime/JSObject.h:419:61: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] Source/JavaScriptCore/runtime/JSObject.h:420:67: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

Dominik Röttsches (drott)

Comment 58 2012-07-06 03:35:19 PDT

(In reply to comment #57) > page => patch

Arunprasad

Comment 59 2012-07-18 01:42:46 PDT

This breaks SH4 JIT build.

Arunprasad

Comment 60 2012-07-18 01:44:31 PDT

QT SH4 bot seems to be dead. If it was active might have reported this build break.

Premkumar Subramanian

Comment 61 2012-07-20 03:09:59 PDT

This patch breaks Qt Mips JIT build as well. (In reply to comment #60) > QT SH4 bot seems to be dead. If it was active might have reported this build break.

STEPHAN Gael

Comment 62 2012-08-24 08:25:45 PDT

If you don't mind Thouraya i added you here so you maybe can check out the SH4 part of the JIT ( I cannot build the latest releases of webkit because of the changes in the JIT not ported to the SH4 JIT) Thank you!!

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