RESOLVED FIXED Bug 164108
Optimize SharedArrayBuffer in the DFG+FTL
https://bugs.webkit.org/show_bug.cgi?id=164108
Summary Optimize SharedArrayBuffer in the DFG+FTL
Filip Pizlo
Reported 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.
Attachments
it's a start (20.92 KB, patch)
2017-04-08 18:45 PDT, Filip Pizlo
no flags
a bit more (26.88 KB, patch)
2017-04-08 19:18 PDT, Filip Pizlo
no flags
more (49.01 KB, patch)
2017-04-13 15:20 PDT, Filip Pizlo
no flags
almost done (76.31 KB, patch)
2017-04-13 17:20 PDT, Filip Pizlo
no flags
a bit more (83.04 KB, patch)
2017-04-13 17:38 PDT, Filip Pizlo
no flags
it compiles! (91.71 KB, patch)
2017-04-14 21:17 PDT, Filip Pizlo
no flags
it crashes! (103.48 KB, patch)
2017-04-15 09:09 PDT, Filip Pizlo
no flags
it ran! (104.54 KB, patch)
2017-04-15 09:37 PDT, Filip Pizlo
no flags
starting to work! (106.05 KB, patch)
2017-04-15 21:01 PDT, Filip Pizlo
no flags
pretty sure it's right (112.75 KB, patch)
2017-04-15 21:30 PDT, Filip Pizlo
buildbot: commit-queue-
Archive of layout-test-results from ews107 for mac-elcapitan-wk2 (898.82 KB, application/zip)
2017-04-15 23:03 PDT, Build Bot
no flags
the patch (116.34 KB, patch)
2017-04-18 11:13 PDT, Filip Pizlo
saam: review+
buildbot: commit-queue-
maybe patch for landing (140.77 KB, patch)
2017-04-18 16:58 PDT, Filip Pizlo
buildbot: commit-queue-
Archive of layout-test-results from ews112 for mac-elcapitan (1.69 MB, application/zip)
2017-04-18 18:39 PDT, Build Bot
no flags
patch for landing (141.45 KB, patch)
2017-04-18 18:45 PDT, Filip Pizlo
no flags
patch for landing (150.91 KB, patch)
2017-04-20 10:03 PDT, Filip Pizlo
no flags
Filip Pizlo
Comment 1 2017-04-08 18:45:37 PDT
Created attachment 306590 [details] it's a start
Filip Pizlo
Comment 2 2017-04-08 19:18:05 PDT
Created attachment 306594 [details] a bit more
Filip Pizlo
Comment 3 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.
Filip Pizlo
Comment 4 2017-04-13 17:20:03 PDT
Created attachment 307054 [details] almost done
Filip Pizlo
Comment 5 2017-04-13 17:38:17 PDT
Created attachment 307058 [details] a bit more
Filip Pizlo
Comment 6 2017-04-14 21:17:02 PDT
Created attachment 307183 [details] it compiles!
Filip Pizlo
Comment 7 2017-04-15 09:09:56 PDT
Created attachment 307198 [details] it crashes!
Filip Pizlo
Comment 8 2017-04-15 09:37:49 PDT
Filip Pizlo
Comment 9 2017-04-15 21:01:00 PDT
Created attachment 307215 [details] starting to work! It passes the SharedArrayBuffer-opt test in DFG.
Filip Pizlo
Comment 10 2017-04-15 21:30:13 PDT
Created attachment 307216 [details] pretty sure it's right
Build Bot
Comment 11 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.
Build Bot
Comment 12 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
Build Bot
Comment 13 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
Build Bot
Comment 14 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
Filip Pizlo
Comment 15 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.
Build Bot
Comment 16 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.
Filip Pizlo
Comment 17 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
Build Bot
Comment 18 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
Saam Barati
Comment 19 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".
Filip Pizlo
Comment 20 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.
Saam Barati
Comment 21 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.
Filip Pizlo
Comment 22 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.
Filip Pizlo
Comment 23 2017-04-18 16:58:31 PDT
Created attachment 307436 [details] maybe patch for landing
Filip Pizlo
Comment 24 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.
Build Bot
Comment 25 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.
Build Bot
Comment 26 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
Build Bot
Comment 27 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
Filip Pizlo
Comment 28 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.
Filip Pizlo
Comment 29 2017-04-18 18:45:52 PDT
Created attachment 307449 [details] patch for landing
Build Bot
Comment 30 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.
Filip Pizlo
Comment 31 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.
Build Bot
Comment 32 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.
Filip Pizlo
Comment 33 2017-04-20 10:56:18 PDT
Note You need to log in before you can comment on or make changes to this bug.