Bug 162980

Summary: [DOMJIT] Add DOMJIT::Signature
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cdumez, commit-queue, darin, fpizlo, saam, sam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 163226, 163657, 163900, 164199    
Bug Blocks: 162544, 164356    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch sam: review+

Yusuke Suzuki
Reported 2016-10-05 10:56:54 PDT
We should have two type of DOMJIT functionality. 1. DOMJIT::GetterSetter based inlining system We can define all the DOMJIT operations in DOMJIT::Patchpoint. It is great since we have a chance to inline it completely! While it's quite good for performance, ASM is typically written by hand. I explored the way to generate ASM automatically from IDL compiler, but the accessors we can do so is still limited. For example, even in Node::firstChild, it has special check path for ContainerNode, while nextSibling, parentNode, and previousSibling do not have such a path. And this fact is not exposed in IDL. We manually write ASM in this case. It has scalability & complexity (But CSS Selector JIT already does the similar things for parentNode, siblings) problem. So we will write this for super hot accessors. Like parentNode, firstChild etc. 2. DOMJIT::Signature based type checking elimination On the other hand, this can be applied to many bindings automatically. We support DOMJIT::Signature. That is the way to express DOM function's signature to DFG/FTL. DFG / FTL recognizes and emit Checks instead of binding code. And in the middle of code, we will invoke the thin binding code that does not have any checks.
Attachments
Patch (9.47 KB, patch)
2016-10-29 00:33 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (126.20 KB, patch)
2016-11-01 00:19 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (156.53 KB, patch)
2016-11-01 15:33 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (180.13 KB, patch)
2016-11-01 22:51 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (173.50 KB, patch)
2016-11-01 22:59 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (173.80 KB, patch)
2016-11-01 23:41 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (174.03 KB, patch)
2016-11-01 23:50 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (174.79 KB, patch)
2016-11-02 00:37 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (173.56 KB, patch)
2016-11-02 11:29 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (174.85 KB, patch)
2016-11-02 11:35 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (174.86 KB, patch)
2016-11-02 11:41 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (174.86 KB, patch)
2016-11-02 13:07 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (179.77 KB, patch)
2016-11-02 16:55 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (185.77 KB, patch)
2016-11-02 17:30 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (186.00 KB, patch)
2016-11-02 17:56 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (187.38 KB, patch)
2016-11-02 18:45 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (187.31 KB, patch)
2016-11-02 20:09 PDT, Yusuke Suzuki 		sam: review+
DetailsFormatted DiffDiff
Show Obsolete (16) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2016-10-05 10:59:33 PDT
Second one can be later extended to support DOM Binding interpreter. It emits byte code instead of C++ native code. And it will be executed by DOM Binding interpreter. While it does not pose any perf benefit (or it may cause some regression), it reduces amount of memory used for DOM binding native code.
Yusuke Suzuki
Comment 2 2016-10-11 14:06:07 PDT
And we have a chance to annotate clobber information of methods. And it allows DFG to hoist / eliminate the operation.
Yusuke Suzuki
Comment 3 2016-10-28 22:52:30 PDT
Start working on this. Since we already have DOMJIT::Effect, this should be easy!
Yusuke Suzuki
Comment 4 2016-10-29 00:33:46 PDT
Created attachment 293285 [details] Patch WIP
Yusuke Suzuki
Comment 5 2016-11-01 00:19:53 PDT
Created attachment 293533 [details] Patch WIP
Yusuke Suzuki
Comment 6 2016-11-01 15:33:04 PDT
Created attachment 293605 [details] Patch WIP
Yusuke Suzuki
Comment 7 2016-11-01 22:51:45 PDT
Created attachment 293640 [details] Patch
Yusuke Suzuki
Comment 8 2016-11-01 22:53:16 PDT
PFR
Yusuke Suzuki
Comment 9 2016-11-01 22:53:30 PDT
Oops, rebasing.
Yusuke Suzuki
Comment 10 2016-11-01 22:59:01 PDT
Created attachment 293641 [details] Patch
Yusuke Suzuki
Comment 11 2016-11-01 22:59:25 PDT
OK, rebasing is done. Ready for reviews!
WebKit Commit Bot
Comment 12 2016-11-01 23:00:30 PDT
Attachment 293641 [details] did not pass style-queue: ERROR: Source/WebCore/domjit/DOMJITIDLType.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:42: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:42: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:87: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:35: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:41: More than one command on the same line [whitespace/newline] [4] Total errors found: 16 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 13 2016-11-01 23:41:02 PDT
Created attachment 293642 [details] Patch
WebKit Commit Bot
Comment 14 2016-11-01 23:44:32 PDT
Attachment 293642 [details] did not pass style-queue: ERROR: Source/WebCore/domjit/DOMJITIDLType.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:42: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:42: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:87: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:35: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:41: More than one command on the same line [whitespace/newline] [4] Total errors found: 16 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 15 2016-11-01 23:50:54 PDT
Created attachment 293644 [details] Patch
WebKit Commit Bot
Comment 16 2016-11-01 23:52:57 PDT
Attachment 293644 [details] did not pass style-queue: ERROR: Source/WebCore/domjit/DOMJITIDLType.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:42: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:42: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:87: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:35: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:41: More than one command on the same line [whitespace/newline] [4] Total errors found: 16 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 17 2016-11-02 00:37:25 PDT
Created attachment 293647 [details] Patch
Yusuke Suzuki
Comment 18 2016-11-02 00:38:46 PDT
Attempt to fix internal compiler error in GCC...............
WebKit Commit Bot
Comment 19 2016-11-02 00:39:32 PDT
Attachment 293647 [details] did not pass style-queue: ERROR: Source/WebCore/domjit/DOMJITIDLType.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:42: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:42: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:87: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:35: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:48: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:49: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:50: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:51: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:53: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:54: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:55: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:56: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:57: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:58: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:59: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:60: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:61: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:62: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:63: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:64: More than one command on the same line [whitespace/newline] [4] Total errors found: 33 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 20 2016-11-02 07:35:26 PDT
Comment on attachment 293647 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=293647&action=review > Source/JavaScriptCore/ChangeLog:28 > + LICM in Dromaeo dom-attr test. In the Dromaeo dom-attr getAttribute test, we can see 32x improvement. (134974.8 v.s. 4203.4) Amazing!
Yusuke Suzuki
Comment 21 2016-11-02 11:29:42 PDT
Created attachment 293680 [details] Patch
WebKit Commit Bot
Comment 22 2016-11-02 11:32:21 PDT
Attachment 293680 [details] did not pass style-queue: ERROR: Source/WebCore/domjit/DOMJITIDLType.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:42: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:42: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:87: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:35: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:41: More than one command on the same line [whitespace/newline] [4] Total errors found: 16 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 23 2016-11-02 11:35:27 PDT
Created attachment 293681 [details] Patch
WebKit Commit Bot
Comment 24 2016-11-02 11:37:47 PDT
Attachment 293681 [details] did not pass style-queue: ERROR: Source/WebCore/domjit/DOMJITIDLType.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:42: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:42: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:87: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:35: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:48: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:49: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:50: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:51: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:53: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:54: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:55: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:56: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:57: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:58: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:59: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:60: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:61: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:62: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:63: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:64: More than one command on the same line [whitespace/newline] [4] Total errors found: 33 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 25 2016-11-02 11:41:34 PDT
Created attachment 293682 [details] Patch
WebKit Commit Bot
Comment 26 2016-11-02 11:43:16 PDT
Attachment 293682 [details] did not pass style-queue: ERROR: Source/WebCore/domjit/DOMJITIDLType.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:42: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:42: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:87: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:35: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:48: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:49: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:50: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:51: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:53: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:54: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:55: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:56: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:57: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:58: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:59: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:60: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:61: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:62: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:63: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:64: More than one command on the same line [whitespace/newline] [4] Total errors found: 33 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 27 2016-11-02 11:55:59 PDT
(In reply to comment #20) > Comment on attachment 293647 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=293647&action=review > > > Source/JavaScriptCore/ChangeLog:28 > > + LICM in Dromaeo dom-attr test. In the Dromaeo dom-attr getAttribute test, we can see 32x improvement. (134974.8 v.s. 4203.4) > > Amazing! Yay! It paves the way to annotate C++ functions with effects. I think eventually we can extend this mechanism to the C++ functions that is not related to DOM. Like String.prototype.xxx functions in JSC. It sometimes successfully annotates the function with like "It does not modify the heap!". And it gives us a chance to perform CSE, LICM, and other great DFG optimizations!
Yusuke Suzuki
Comment 28 2016-11-02 13:07:11 PDT
Created attachment 293688 [details] Patch
WebKit Commit Bot
Comment 29 2016-11-02 13:09:58 PDT
Attachment 293688 [details] did not pass style-queue: ERROR: Source/WebCore/domjit/DOMJITIDLType.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:42: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:42: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:87: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:35: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:48: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:49: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:50: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:51: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:53: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:54: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:55: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:56: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:57: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:58: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:59: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:60: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:61: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:62: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:63: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:64: More than one command on the same line [whitespace/newline] [4] Total errors found: 33 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 30 2016-11-02 13:11:21 PDT
Attempt to fix Windows build!
Yusuke Suzuki
Comment 31 2016-11-02 14:18:42 PDT
Comment on attachment 293688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=293688&action=review > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7338 > + silentSpillAllRegisters(resultRegs); > + int argumentCountIncludingThis = signature->argumentCount + 1; > + switch (argumentCountIncludingThis) { > + case 1: > + callOperation(reinterpret_cast<J_JITOperation_EP>(signature->unsafeFunction), extractResult(resultRegs), regs[0]); > + break; > + case 2: > + callOperation(reinterpret_cast<J_JITOperation_EPP>(signature->unsafeFunction), extractResult(resultRegs), regs[0], regs[1]); > + break; > + case 3: > + callOperation(reinterpret_cast<J_JITOperation_EPPP>(signature->unsafeFunction), extractResult(resultRegs), regs[0], regs[1], regs[2]); > + break; > + default: > + RELEASE_ASSERT_NOT_REACHED(); > + break; > + } > + > + silentFillAllRegisters(resultRegs); I'll use flushRegisters() instead later.
Saam Barati
Comment 32 2016-11-02 15:46:04 PDT
Comment on attachment 293688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=293688&action=review r=me Patch looks reasonable to me. It may be worth pinging review from somebody who knows the IDL generator on that part of the patch since I'm not familiar with that code. > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2620 > + if (argumentCountIncludingThis != static_cast<int>(1 + signature->argumentCount)) Can you also add a check for the maximum number of arguments we currently support, since the code in the rest of the DFG doesn't handle arbitrary number of arguments. Maybe also with a FIXME if we ever plan to extend the number of arguments. > Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:2703 > + case SpecBoolean: Can you also add an assert that we don't reach the default case. > Source/JavaScriptCore/dfg/DFGNode.cpp:230 > + for (unsigned i = 1, iz = numChildren(); i < iz; ++i) You don't need iz here, I'd just make the loop i < numChildren(). Also, this function should ASSERT or RELEASE_ASSERT that numChildren() <= 4. > Source/JavaScriptCore/dfg/DFGNode.h:2392 > + // Note that this does not include TailCall thingy intentionally. thingy => node types > Source/JavaScriptCore/dfg/DFGNode.h:2393 > + // CallDOM thingy are always converted from Call. thingy are => node type is > Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:714 > + case CallDOM: { Why would this ever be in the IR at this point? I would put it with the asserts saying it shouldn't be in the IR. > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7270 > + Vector<Variant<SpeculateCellOperand, SpeculateInt32Operand, SpeculateBooleanOperand>> operands; > + Vector<GPRReg> regs; Nit: I'd give tese an inline capacity of 3 or 4 or whatever the max number of arguments for now are. > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7321 > + silentSpillAllRegisters(resultRegs); This should just be a flushRegisters() since you'll never have a non-call path. > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7338 > + silentFillAllRegisters(resultRegs); and then you can remove this. > Source/JavaScriptCore/domjit/DOMJITHeapRange.h:88 > + static_assert(begin <= end, "begin <= end is the invariant of this HeapRange."); I thought begin must be strictly less than end? Or is that only if we're not none()? If so, maybe this can be stricter: (begin < end) || (begin == UINT16_MAX && end == UINT16_MAX) > Source/JavaScriptCore/domjit/DOMJITSignature.h:35 > +// Eventually, we should extend this. But possibly, 2 or 3 can cover typical use cases. I would open a FIXME for this. > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9134 > + int argumentCountIncludingThis = signature->argumentCount + 1; nit: should be unsigned integer.
Yusuke Suzuki
Comment 33 2016-11-02 16:22:12 PDT
Comment on attachment 293688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=293688&action=review Thanks, I'll update the patch wit your comment. And request IDL review. >> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2620 >> + if (argumentCountIncludingThis != static_cast<int>(1 + signature->argumentCount)) > > Can you also add a check for the maximum number of arguments we currently support, since the code in the rest of the DFG doesn't handle arbitrary number of arguments. > Maybe also with a FIXME if we ever plan to extend the number of arguments. Added assertion here. And FIXME exists on DOMJIT::Signature. But we also add the FIXME here. >> Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:2703 >> + case SpecBoolean: > > Can you also add an assert that we don't reach the default case. Sure! Added. >> Source/JavaScriptCore/dfg/DFGNode.cpp:230 >> + for (unsigned i = 1, iz = numChildren(); i < iz; ++i) > > You don't need iz here, I'd just make the loop i < numChildren(). > Also, this function should ASSERT or RELEASE_ASSERT that numChildren() <= 4. Fixed. >> Source/JavaScriptCore/dfg/DFGNode.h:2392 >> + // Note that this does not include TailCall thingy intentionally. > > thingy => node types Fixed. >> Source/JavaScriptCore/dfg/DFGNode.h:2393 >> + // CallDOM thingy are always converted from Call. > > thingy are => node type is Fixed. >> Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:714 >> + case CallDOM: { > > Why would this ever be in the IR at this point? I would put it with the asserts saying it shouldn't be in the IR. Oops, right. It is not necessary. Dropped. Add CallDOM to the DFG_CRASH clause. >> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7270 >> + Vector<GPRReg> regs; > > Nit: I'd give tese an inline capacity of 3 or 4 or whatever the max number of arguments for now are. Sounds nice. Fixed. >>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7338 >>> + silentFillAllRegisters(resultRegs); >> >> I'll use flushRegisters() instead later. > > and then you can remove this. Yeah, right. Dropped. >> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9134 >> + int argumentCountIncludingThis = signature->argumentCount + 1; > > nit: should be unsigned integer. Fixed.
Yusuke Suzuki
Comment 34 2016-11-02 16:23:32 PDT
Comment on attachment 293688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=293688&action=review >> Source/JavaScriptCore/domjit/DOMJITHeapRange.h:88 >> + static_assert(begin <= end, "begin <= end is the invariant of this HeapRange."); > > I thought begin must be strictly less than end? Or is that only if we're not none()? > If so, maybe this can be stricter: > (begin < end) || (begin == UINT16_MAX && end == UINT16_MAX) Yeah, right. I'll make this assertion stricter.
Yusuke Suzuki
Comment 35 2016-11-02 16:24:23 PDT
Comment on attachment 293688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=293688&action=review >> Source/JavaScriptCore/domjit/DOMJITSignature.h:35 >> +// Eventually, we should extend this. But possibly, 2 or 3 can cover typical use cases. > > I would open a FIXME for this. Yeah, opened. https://bugs.webkit.org/show_bug.cgi?id=164346
Yusuke Suzuki
Comment 36 2016-11-02 16:55:48 PDT
Created attachment 293717 [details] Patch
Yusuke Suzuki
Comment 37 2016-11-02 17:30:50 PDT
Created attachment 293724 [details] Patch
WebKit Commit Bot
Comment 38 2016-11-02 17:34:25 PDT
Attachment 293724 [details] did not pass style-queue: ERROR: Source/WebCore/domjit/DOMJITIDLType.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:42: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:42: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:87: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:35: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:41: More than one command on the same line [whitespace/newline] [4] Total errors found: 16 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 39 2016-11-02 17:56:38 PDT
Created attachment 293728 [details] Patch
WebKit Commit Bot
Comment 40 2016-11-02 17:59:07 PDT
Attachment 293728 [details] did not pass style-queue: ERROR: Source/WebCore/domjit/DOMJITIDLType.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:42: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:42: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:87: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:35: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:41: More than one command on the same line [whitespace/newline] [4] Total errors found: 16 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 41 2016-11-02 17:59:30 PDT
Comment on attachment 293728 [details] Patch Oops, rebasing is performed on to the incorrect branch. Fixing...
Yusuke Suzuki
Comment 42 2016-11-02 18:45:17 PDT
Created attachment 293733 [details] Patch
WebKit Commit Bot
Comment 43 2016-11-02 18:47:15 PDT
Attachment 293733 [details] did not pass style-queue: ERROR: Source/WebCore/domjit/DOMJITIDLType.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:42: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:42: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:87: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:35: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:48: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:49: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:50: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:51: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:53: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:54: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:55: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:56: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:57: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:58: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:59: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:60: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:61: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:62: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:63: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:64: More than one command on the same line [whitespace/newline] [4] Total errors found: 33 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yusuke Suzuki
Comment 44 2016-11-02 18:51:44 PDT
OK. I would like to request reviews for IDL code generator part. Could you review this, Chris, Alex, or Sam?
Yusuke Suzuki
Comment 45 2016-11-02 20:09:44 PDT
Created attachment 293738 [details] Patch Small fix for IDL when Atomic string ones are required as an argument
WebKit Commit Bot
Comment 46 2016-11-02 20:13:23 PDT
Attachment 293738 [details] did not pass style-queue: ERROR: Source/WebCore/domjit/DOMJITIDLType.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLType.h:42: More than one command on the same line [whitespace/newline] [4] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:42: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/JavaScriptCore/runtime/NativeExecutable.h:87: The parameter name "function" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:35: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:36: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:37: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:38: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:39: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:40: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:41: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:48: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:49: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:50: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:51: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:52: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:53: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:54: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:55: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:56: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:57: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:58: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:59: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:60: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:61: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:62: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:63: More than one command on the same line [whitespace/newline] [4] ERROR: Source/WebCore/domjit/DOMJITIDLTypeFilter.h:64: More than one command on the same line [whitespace/newline] [4] Total errors found: 33 in 69 files If any of these errors are false positives, please file a bug against check-webkit-style.
Sam Weinig
Comment 47 2016-11-02 20:16:21 PDT
Comment on attachment 293738 [details] Patch r=me on the IDL parts.
Yusuke Suzuki
Comment 48 2016-11-02 20:23:55 PDT
Committed r208320: <http://trac.webkit.org/changeset/208320>
Yusuke Suzuki
Comment 49 2016-11-02 20:34:49 PDT
Will fix CLoop build soon.
Yusuke Suzuki
Comment 50 2016-11-02 21:24:30 PDT
Committed r208323: <http://trac.webkit.org/changeset/208323>
Note You need to log in before you can comment on or make changes to this bug.