Bug 164108 – Optimize SharedArrayBuffer in the DFG+FTL

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Filip Pizlo]

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.

[Filip Pizlo]

Created attachment 306590 [details]
it's a start

[Filip Pizlo]

Created attachment 306594 [details]
a bit more

[Filip Pizlo]

Created attachment 307032 [details]
more

I wrote the DFG backend.  I think I just need to write the FTL lowering now.

[Filip Pizlo]

Created attachment 307054 [details]
almost done

[Filip Pizlo]

Created attachment 307058 [details]
a bit more

[Filip Pizlo]

Created attachment 307183 [details]
it compiles!

[Filip Pizlo]

Created attachment 307198 [details]
it crashes!

[Filip Pizlo]

Created attachment 307199 [details]
it ran!

[Filip Pizlo]

Created attachment 307215 [details]
starting to work!

It passes the SharedArrayBuffer-opt test in DFG.

[Filip Pizlo]

Created attachment 307216 [details]
pretty sure it's right

[Build Bot]

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.

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Filip Pizlo]

Created attachment 307396 [details]
the patch

I may have to work through some build failures, but it basically looks right.

[Build Bot]

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.

[Filip Pizlo]

I investigated the JS test failure.  I don't think it's related.  See https://bugs.webkit.org/show_bug.cgi?id=170958

[Build Bot]

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

[Saam Barati]

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".

[Filip Pizlo]

(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.

[Saam Barati]

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.

[Filip Pizlo]

(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.

[Filip Pizlo]

Created attachment 307436 [details]
maybe patch for landing

[Filip Pizlo]

(In reply to Filip Pizlo from comment #23)
> Created attachment 307436 [details]
> maybe patch for landing

Need to see what happens on EWS.

[Build Bot]

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.

[Build Bot]

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

[Build Bot]

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

[Filip Pizlo]

(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.

[Filip Pizlo]

Created attachment 307449 [details]
patch for landing

[Build Bot]

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.

[Filip Pizlo]

Created attachment 307597 [details]
patch for landing

Ran tests on ARM64.  Found a few goofs in the masm.

[Build Bot]

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.

[Filip Pizlo]

Landed in https://trac.webkit.org/changeset/215565/webkit