Bug 164108

Summary: Optimize SharedArrayBuffer in the DFG+FTL
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, buildbot, cdumez, cmarcelo, dbates, keith_miller, mark.lam, msaboff, rniwa, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 163986    
Bug Blocks:    
Attachments:
Description Flags
it's a start
none
a bit more
none
more
none
almost done
none
a bit more
none
it compiles!
none
it crashes!
none
it ran!
none
starting to work!
none
pretty sure it's right
buildbot: commit-queue-
Archive of layout-test-results from ews107 for mac-elcapitan-wk2
none
the patch
saam: review+, buildbot: commit-queue-
maybe patch for landing
buildbot: commit-queue-
Archive of layout-test-results from ews112 for mac-elcapitan
none
patch for landing
none
patch for landing none

Description Filip Pizlo 2016-10-27 21:15:11 PDT 
This bug covers teaching the DFG and FTL about the atomics intrinsics and doing any other clean-ups that would teach the DFG how to fully take advantage of shared array buffers.
Comment 1 Filip Pizlo 2017-04-08 18:45:37 PDT 
Created attachment 306590 [details]
it's a start
Comment 2 Filip Pizlo 2017-04-08 19:18:05 PDT 
Created attachment 306594 [details]
a bit more
Comment 3 Filip Pizlo 2017-04-13 15:20:56 PDT 
Created attachment 307032 [details]
more

I wrote the DFG backend.  I think I just need to write the FTL lowering now.
Comment 4 Filip Pizlo 2017-04-13 17:20:03 PDT 
Created attachment 307054 [details]
almost done
Comment 5 Filip Pizlo 2017-04-13 17:38:17 PDT 
Created attachment 307058 [details]
a bit more
Comment 6 Filip Pizlo 2017-04-14 21:17:02 PDT 
Created attachment 307183 [details]
it compiles!
Comment 7 Filip Pizlo 2017-04-15 09:09:56 PDT 
Created attachment 307198 [details]
it crashes!
Comment 8 Filip Pizlo 2017-04-15 09:37:49 PDT 
Created attachment 307199 [details]
it ran!
Comment 9 Filip Pizlo 2017-04-15 21:01:00 PDT 
Created attachment 307215 [details]
starting to work!

It passes the SharedArrayBuffer-opt test in DFG.
Comment 10 Filip Pizlo 2017-04-15 21:30:13 PDT 
Created attachment 307216 [details]
pretty sure it's right
Comment 11 Build Bot 2017-04-15 21:31:58 PDT 
Attachment 307216 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:376:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:377:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:378:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:379:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:380:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:381:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:382:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11886:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11892:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11894:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11900:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11906:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11909:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11914:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11920:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11926:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11930:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11937:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:49:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:50:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:51:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:52:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:53:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:54:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:55:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:56:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:57:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:58:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 28 in 31 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Build Bot 2017-04-15 22:12:26 PDT 
Comment on attachment 307216 [details]
pretty sure it's right

Attachment 307216 [details] did not pass jsc-ews (mac):
Output: http://webkit-queues.webkit.org/results/3543691

New failing tests:
stress/SharedArrayBuffer-opt.js.ftl-no-cjit-no-inline-validate
stress/SharedArrayBuffer-opt.js.ftl-eager
wasm.yaml/wasm/function-tests/memory-multiagent.js.default-wasm
Comment 13 Build Bot 2017-04-15 23:02:59 PDT 
Comment on attachment 307216 [details]
pretty sure it's right

Attachment 307216 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/3543785

New failing tests:
webrtc/multi-video.html
Comment 14 Build Bot 2017-04-15 23:03:00 PDT 
Created attachment 307219 [details]
Archive of layout-test-results from ews107 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 15 Filip Pizlo 2017-04-18 11:13:56 PDT 
Created attachment 307396 [details]
the patch

I may have to work through some build failures, but it basically looks right.
Comment 16 Build Bot 2017-04-18 11:16:23 PDT 
Attachment 307396 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:376:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:377:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:378:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:379:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:380:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:381:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:382:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11886:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11892:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11894:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11900:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11906:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11909:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11914:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11920:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11926:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11930:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11937:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:49:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:50:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:51:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:52:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:53:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:54:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:55:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:56:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:57:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:58:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 28 in 34 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 Filip Pizlo 2017-04-18 11:43:36 PDT 
I investigated the JS test failure.  I don't think it's related.  See https://bugs.webkit.org/show_bug.cgi?id=170958
Comment 18 Build Bot 2017-04-18 11:55:28 PDT 
Comment on attachment 307396 [details]
the patch

Attachment 307396 [details] did not pass jsc-ews (mac):
Output: http://webkit-queues.webkit.org/results/3558464

New failing tests:
wasm.yaml/wasm/function-tests/memory-multiagent.js.default-wasm
Comment 19 Saam Barati 2017-04-18 13:09:13 PDT 
Comment on attachment 307396 [details]
the patch

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

r=me

> Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:706
> +            if (jump.isSet())
> +                m_jumps.append(jump);

This seems like it would usually be a bug in the caller.

> Source/JavaScriptCore/assembler/CPU.h:76
> +inline bool is64Bit()
> +{
> +    return sizeof(void*) == 8;
> +}
> +
> +inline bool is32Bit()
> +{
> +    return !is64Bit();
> +}

Why not just use JSVALUE64 for these?

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:555
> +    case AtomicsIsLockFree: {
> +        if (node->child1().useKind() != Int32Use)
> +            clobberWorld(node->origin.semantic, clobberLimit);
> +        forNode(node).setType(SpecBoolInt32);
> +        break;
> +    }

Maybe it's worth having a constant folding rule here? I could see this often being used with a constant literal by users of the API.

> Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:943
> +            fixEdge<KnownInt32Use>(index);

Is this KnownInt32 b/c blessArrayOperation?

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:3194
> +        Edge argEdges[2];
> +        for (unsigned i = numExtraArgs; i--;)

Can you RELEASE_ASSERT here that numExtraArgs <= 2?
Also, it might be worth giving the literal "2" a name, since you use the literal 2 in a few places.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:3296
> +        // OOPS! need bug.

Please create and link here.

Also, I don't understand why this is needed. Can you explain?

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:2967
> +        Edge argEdges[2];
> +        for (unsigned i = numExtraArgs; i--;)

Ditto about adding an assertion and also perhaps using a name for "2".
Comment 20 Filip Pizlo 2017-04-18 13:16:05 PDT 
(In reply to Saam Barati from comment #19)
> Comment on attachment 307396 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=307396&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/assembler/AbstractMacroAssembler.h:706
> > +            if (jump.isSet())
> > +                m_jumps.append(jump);
> 
> This seems like it would usually be a bug in the caller.

It made some code in this patch cleaner.  I don't think it hurts other callers.  I'm not sure if crash-while-patching-a-jump is a common crash.  It doesn't ring a bell.

> 
> > Source/JavaScriptCore/assembler/CPU.h:76
> > +inline bool is64Bit()
> > +{
> > +    return sizeof(void*) == 8;
> > +}
> > +
> > +inline bool is32Bit()
> > +{
> > +    return !is64Bit();
> > +}
> 
> Why not just use JSVALUE64 for these?

You mean, inside them, or instead of them?

This code is just moved from B3Common.h.  Using "if (is64Bit())" is cleaner than #if's.

> 
> > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:555
> > +    case AtomicsIsLockFree: {
> > +        if (node->child1().useKind() != Int32Use)
> > +            clobberWorld(node->origin.semantic, clobberLimit);
> > +        forNode(node).setType(SpecBoolInt32);
> > +        break;
> > +    }
> 
> Maybe it's worth having a constant folding rule here? I could see this often
> being used with a constant literal by users of the API.

Yeah, I can add one.  I didn't add it because B3 constant folds it.

> 
> > Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:943
> > +            fixEdge<KnownInt32Use>(index);
> 
> Is this KnownInt32 b/c blessArrayOperation?

No, that's a bug!  I'll add a test.

> 
> > Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:3194
> > +        Edge argEdges[2];
> > +        for (unsigned i = numExtraArgs; i--;)
> 
> Can you RELEASE_ASSERT here that numExtraArgs <= 2?
> Also, it might be worth giving the literal "2" a name, since you use the
> literal 2 in a few places.
> 
> > Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:3296
> > +        // OOPS! need bug.
> 
> Please create and link here.
> 
> Also, I don't understand why this is needed. Can you explain?

The MacroAssembler has a register allocation validater that tries to prevent you from doing register allocation inside control flow.  Unfortunately, since we do a label() here and we just finished doing register allocation, the validater assumes that the register allocation happened at the label, rather than before it.  Adding the nop creates a gap, so the register allocation validater correctly notes that the register allocation happens before the loop jump destination.

> 
> > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:2967
> > +        Edge argEdges[2];
> > +        for (unsigned i = numExtraArgs; i--;)
> 
> Ditto about adding an assertion and also perhaps using a name for "2".

OK, I'll look into that.
Comment 21 Saam Barati 2017-04-18 13:22:28 PDT 
Comment on attachment 307396 [details]
the patch

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

>>> Source/JavaScriptCore/assembler/CPU.h:76
>>> +}
>> 
>> Why not just use JSVALUE64 for these?
> 
> You mean, inside them, or instead of them?
> 
> This code is just moved from B3Common.h.  Using "if (is64Bit())" is cleaner than #if's.

I mean inside. 
I'm cool with this being a function.
Might as well make it constexpr while you're at it.
Comment 22 Filip Pizlo 2017-04-18 13:23:39 PDT 
(In reply to Saam Barati from comment #21)
> Comment on attachment 307396 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=307396&action=review
> 
> >>> Source/JavaScriptCore/assembler/CPU.h:76
> >>> +}
> >> 
> >> Why not just use JSVALUE64 for these?
> > 
> > You mean, inside them, or instead of them?
> > 
> > This code is just moved from B3Common.h.  Using "if (is64Bit())" is cleaner than #if's.
> 
> I mean inside. 
> I'm cool with this being a function.
> Might as well make it constexpr while you're at it.

Gotcha!  I can do that.
Comment 23 Filip Pizlo 2017-04-18 16:58:31 PDT 
Created attachment 307436 [details]
maybe patch for landing
Comment 24 Filip Pizlo 2017-04-18 16:58:50 PDT 
(In reply to Filip Pizlo from comment #23)
> Created attachment 307436 [details]
> maybe patch for landing

Need to see what happens on EWS.
Comment 25 Build Bot 2017-04-18 17:01:11 PDT 
Attachment 307436 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:376:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:377:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:378:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:379:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:380:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:381:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:382:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3026:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3031:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3036:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3041:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3046:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3051:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3056:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3061:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3066:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3071:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3076:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3081:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3086:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3091:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3096:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3101:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3106:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3111:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11895:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11901:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11903:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11909:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11915:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11918:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11923:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11929:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11935:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11939:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11946:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:49:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:50:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:51:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:52:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:53:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:54:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:55:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:56:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:57:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:58:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1438:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1443:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1448:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1453:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1458:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1463:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 52 in 43 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 26 Build Bot 2017-04-18 18:39:39 PDT 
Comment on attachment 307436 [details]
maybe patch for landing

Attachment 307436 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/3559951

New failing tests:
media/modern-media-controls/macos-fullscreen-media-controls/macos-fullscreen-media-controls-buttons-styles.html
workers/sab/cascade_lock.html
Comment 27 Build Bot 2017-04-18 18:39:41 PDT 
Created attachment 307448 [details]
Archive of layout-test-results from ews112 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 28 Filip Pizlo 2017-04-18 18:41:12 PDT 
(In reply to Build Bot from comment #27)
> Created attachment 307448 [details]
> Archive of layout-test-results from ews112 for mac-elcapitan
> 
> The attached test failures were seen while running run-webkit-tests on the
> mac-debug-ews.
> Bot: ews112  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

This failure looks unrelated.  I added an expected file for my new test.
Comment 29 Filip Pizlo 2017-04-18 18:45:52 PDT 
Created attachment 307449 [details]
patch for landing
Comment 30 Build Bot 2017-04-18 18:47:21 PDT 
Attachment 307449 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:376:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:377:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:378:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:379:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:380:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:381:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:382:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3026:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3031:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3036:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3041:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3046:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3051:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3056:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3061:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3066:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3071:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3076:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3081:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3086:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3091:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3096:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3101:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3106:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3111:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11895:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11901:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11903:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11909:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11915:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11918:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11923:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11929:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11935:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11939:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11946:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:49:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:50:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:51:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:52:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:53:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:54:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:55:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:56:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:57:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:58:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1438:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1443:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1448:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1453:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1458:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1463:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 52 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 31 Filip Pizlo 2017-04-20 10:03:55 PDT 
Created attachment 307597 [details]
patch for landing

Ran tests on ARM64.  Found a few goofs in the masm.
Comment 32 Build Bot 2017-04-20 10:06:39 PDT 
Attachment 307597 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:376:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:377:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:378:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:379:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:380:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:381:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/ftl/FTLOutput.h:382:  The parameter name "pointer" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3026:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3031:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3036:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3041:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3046:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3051:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3056:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3061:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3066:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3071:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3076:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3081:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3086:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3091:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3096:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3101:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3106:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h:3111:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11895:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11901:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11903:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11909:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11915:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11918:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11923:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11929:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11935:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11939:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:11946:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:49:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:50:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:51:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:52:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:53:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:54:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:55:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:56:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:57:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/AtomicsObject.h:58:  The parameter name "exec" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1438:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1443:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1448:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1453:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1458:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/assembler/MacroAssemblerX86_64.h:1463:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 52 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 33 Filip Pizlo 2017-04-20 10:56:18 PDT 
Landed in https://trac.webkit.org/changeset/215565/webkit