I'm going to introduce a new term: "heap variable". This is a variable that is anywhere but on the stack (i.e. local). It may be global or it may be captured. Hence the hierarchy of variables is:
Variable:
Local Variable: it's on the stack according to bytecode
Heap Variable: it's not a Local Variable
Captured Variable: it's in some function's activation, may be my own activation
Global Variable: it's in some global object
We currently use VirtualRegister to effectively track heap variables because of some history. The use of the VirtualRegister class for this purpose makes it difficult to then remove the hole we have in the stack that corresponds to local variables that got captured. Once we ensure that VirtualRegister is no longer used for heap variables, we'll be able to remove the hole.
Comment on attachment 245967[details]
it begins!
View in context: https://bugs.webkit.org/attachment.cgi?id=245967&action=review
I really like this patch. It makes some stuff that confused me when I first got started hacking on JSC more intuitive.
> Source/JavaScriptCore/runtime/SymbolTable.cpp:140
> +SymbolTable* SymbolTable::cloneIgnoringStack(VM& vm)
I kind of like the function name: "cloneHeapNames" or "cloneHeapVariables", cloneIgnoringStack confused me at first. But that's just my style.
(In reply to comment #6)
> Created attachment 246976[details]
> a bit more
>
> I believe that the next step is to remove the
> JSLexicalEnvironment::m_registers field.
It is, but it has to happen as part of this patch. m_registers currently serves the perverse purpose of allowing VirtualRegister to be used as a scope offset.
Created attachment 247195[details]
more
Taking a pause while I explore what defineOwnProperty is really supposed to do for arguments. I'm pretty sure that what we're doing now is technically right but unnecessarily roundabout.
I just realized that my ScopedArguments object needs room for overflow arguments - those arguments that were not in the formal parameter list. Shouldn't be a hard change, just more code.
But other than that, the BytecodeGenerator stuff is materializing nicely.
Created attachment 247539[details]
a bit more
The bytecode generator changes are coming along nicely, but I'll have to shift focus a bit and give ScopedArguments the guts it needs for dealing with overflow arguments.
Created attachment 247643[details]
bytecode generation initialization is done
Now I just have to:
- change NodesCodegen to use BytecodeGenerator::variable() instead of BytecodeGenerator::local()
- finish changing all of the execution engines to handle the new arguments- and activation-related operations.
- redo ArgumentsSimplificationPhase
Created attachment 247650[details]
let the carnage begin!
In preparation for adding all of the new stuff, I'm starting to remove the captured variable cruft. There is no longer such a thing! After bytecode is generated, all "captured" variables are just heap accesses, as they should be.
Created attachment 247981[details]
the bulk of functionality is done, just cleaning up now
Of course this doesn't include 32-bit or the the new arguments simplification. The things left are to get this to the point where it compiles and where varargs calls aren't totally messed up.
(In reply to comment #42)
> Ossy: this doesn't look like it will involve platform-specifics, at least
> not yet. But there is still another 100k worth of code to write...
Thanks for the heads-up.
Created attachment 248291[details]
passing lots of tests
Looks like there are still corner cases that are broken, but lots works!
I also still need to write tests specifically for the bizarre things that arise with OutOfBandArguments and DirectArguments.
Created attachment 248480[details]
it might pass all tests
Next step: get it to perform.
After that: port to 32-bit.
I want to make sure that this performs before I write the 32-bit codegen, since my performance findings may change my mind about how I generate code.
Created attachment 248568[details]
more
Arguments elimination phase now has all of the required analyses. I need to think for a bit about whether I like how they're written.
Created attachment 248791[details]
latest
Trying to get the new stuff to compile. Turns out I need to make some more changes to promoted heap locations...
(In reply to comment #68)
> Created attachment 249036[details]
> passing all tests
>
> on 64-bit only, of course. haven't run perf tests yet.
It still crashes raytrace - but only running in the benchmark harness. Gotta investigate what's up with that.
(In reply to comment #69)
> (In reply to comment #68)
> > Created attachment 249036[details]
> > passing all tests
> >
> > on 64-bit only, of course. haven't run perf tests yet.
>
> It still crashes raytrace - but only running in the benchmark harness.
> Gotta investigate what's up with that.
Looks like a GC bug, like a reference that isn't being marked or barriered. But when it does run, it's as fast as trunk!
OTOH, box2d is still regressed, albeit less. I believe that benchmark will need some form of varargs forwarding (i.e. limited arguments elimination just for varargs) in the third-tier DFG.
The new code with the third-tier varargs forwarding still has some issues:
** The following JSC stress test failures have been introduced:
internal-js-tests.yaml/V8v7/raytrace.js.default-ftl
internal-js-tests.yaml/V8v7/raytrace.js.ftl-no-cjit-no-simple-opt
internal-js-tests.yaml/V8v7/raytrace.js.ftl-no-cjit-osr-validation
internal-js-tests.yaml/V8v7/raytrace.js.ftl-no-cjit-validate
jsc-layout-tests.yaml/js/script-tests/function-apply-many-args.js.layout-dfg-eager-no-cjit
v8-v6/v8-raytrace.js.default-ftl
v8-v6/v8-raytrace.js.dfg-eager-no-cjit-validate
v8-v6/v8-raytrace.js.ftl-eager-no-cjit
v8-v6/v8-raytrace.js.ftl-no-cjit-validate
Results for JSC stress tests:
9 failures found.
OK, I fixed that. It turned out to be a dumb bug in argumentsInvolveStackSlot(): we thought that a store to 'this' interferes with the stack slots used by the arguments object.
(In reply to comment #80)
> OK, I fixed that. It turned out to be a dumb bug in
> argumentsInvolveStackSlot(): we thought that a store to 'this' interferes
> with the stack slots used by the arguments object.
And by "fixed", I mean that box2d now has a speed-up with this patch:
box2d x2 12.74121+-0.02937 ^ 12.21960+-0.13325 ^ definitely 1.0427x faster
(In reply to comment #82)
> Created attachment 249182[details]
> couple of bugfixes
>
> And some new tests!
This passes a lot of tests, but hilariously, the VarargsForwardingPhase causes the FTL to do less optimizations. I need to investigate.
Created attachment 249219[details]
so close to done
I just need to write the code for the super-annoying CreateScopedArguments/CreateOutOfBadArguments slow call argument marshaling for platforms with fewer arguments registers.
Attachment 249240[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:712: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecode/ValueRecovery.h:30: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ScopedArguments.h:111: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/ScopedArguments.h:111: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:43: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:107: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:113: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:206: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:237: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:298: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:318: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:45: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:46: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:47: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:48: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:49: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:697: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/ScopedArgumentsTable.h:72: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/ScopedArgumentsTable.h:72: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/ScopedArgumentsTable.h:76: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/ScopedArgumentsTable.h:76: The parameter name "globalObject" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsUtilities.h:36: The parameter name "reg" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsUtilities.h:37: The parameter name "reg" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsUtilities.h:40: The parameter name "origin" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGGraph.cpp:863: Use 'WTF::move()' instead of 'std::move()'. [runtime/wtf_move] [4]
ERROR: Source/JavaScriptCore/dfg/DFGGraph.cpp:881: Use 'WTF::move()' instead of 'std::move()'. [runtime/wtf_move] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:118: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:173: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:253: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/OutOfBandArguments.h:53: The parameter name "structure" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/GenericArgumentsInlines.h:130: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:275: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:282: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:261: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:621: The parameter name "pattern" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:52: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2535: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/ScopedArgumentsTable.cpp:82: Use 'WTF::move()' instead of 'std::move()'. [runtime/wtf_move] [4]
ERROR: Source/JavaScriptCore/bytecode/BytecodeKills.h:63: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4384: Multi line control clauses should use braces. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4482: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4570: Use 'WTF::move()' instead of 'std::move()'. [runtime/wtf_move] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4669: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4672: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4673: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4674: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4676: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4681: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4698: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4699: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4700: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4702: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4708: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:155: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:171: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:196: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:207: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/SymbolTable.h:630: The parameter name "locker" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SymbolTable.h:630: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SymbolTable.h:631: The parameter name "locker" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SymbolTable.h:631: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 61 in 249 files
If any of these errors are false positives, please file a bug against check-webkit-style.
EFL build can be fixed easily on x86_64: (all JSC stress tests pass)
- Source/JavaScriptCore/runtime/GenericOffset.h needs a limits.h include
- Source/JavaScriptCore/CMakeLists.txt needs runtime/ScopeOffset.cpp too
I'll check the ARM Linux build and test soon.
Attachment 249245[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:712: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecode/ValueRecovery.h:30: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ScopedArguments.h:111: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/ScopedArguments.h:111: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:43: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:107: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:113: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:206: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:237: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:298: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:318: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:45: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:46: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:47: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:48: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:49: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:697: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/ScopedArgumentsTable.h:72: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/ScopedArgumentsTable.h:72: The parameter name "i" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/ScopedArgumentsTable.h:76: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/ScopedArgumentsTable.h:76: The parameter name "globalObject" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsUtilities.h:36: The parameter name "reg" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsUtilities.h:37: The parameter name "reg" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsUtilities.h:40: The parameter name "origin" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGGraph.cpp:863: Use 'WTF::move()' instead of 'std::move()'. [runtime/wtf_move] [4]
ERROR: Source/JavaScriptCore/dfg/DFGGraph.cpp:881: Use 'WTF::move()' instead of 'std::move()'. [runtime/wtf_move] [4]
ERROR: Source/JavaScriptCore/runtime/JSGlobalObject.cpp:118: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:173: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:253: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/OutOfBandArguments.h:53: The parameter name "structure" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/GenericArgumentsInlines.h:130: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:275: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:282: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:261: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:621: The parameter name "pattern" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGOperations.cpp:52: Alphabetical sorting problem. [build/include_order] [4]
ERROR: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:2535: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/ScopedArgumentsTable.cpp:82: Use 'WTF::move()' instead of 'std::move()'. [runtime/wtf_move] [4]
ERROR: Source/JavaScriptCore/bytecode/BytecodeKills.h:63: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4384: Multi line control clauses should use braces. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4482: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4570: Use 'WTF::move()' instead of 'std::move()'. [runtime/wtf_move] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4669: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4672: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4673: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4674: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4676: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4681: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4698: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4699: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4700: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4702: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4708: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:155: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:171: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:196: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:207: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/runtime/SymbolTable.h:630: The parameter name "locker" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SymbolTable.h:630: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SymbolTable.h:631: The parameter name "locker" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/SymbolTable.h:631: The parameter name "vm" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 61 in 249 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 249249[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:712: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:107: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:113: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:206: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:237: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:298: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:318: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:45: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:46: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:47: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:48: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:49: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:697: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:173: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:253: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:275: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:282: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:261: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/BytecodeKills.h:63: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4384: Multi line control clauses should use braces. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4669: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4672: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4673: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4674: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4676: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4681: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4698: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4699: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4700: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4702: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4708: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:155: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:171: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:196: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:207: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 35 in 251 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #98)
> Created attachment 249249[details]
> very latest
>
> Totally works on x86-64, mostly works on x86-32, probably won't build yet on
> the ARMs.
Only a small patch was needed for this version to make ARM Thumb2 Linux happy:
@@ -497,6 +513,13 @@
return registerForIndex[index];
}
+ static GPRReg toArgumentRegister(unsigned index)
+ {
+ ASSERT(index < numberOfArgumentRegisters);
+ static const GPRReg registerForIndex[numberOfArgumentRegisters] = { argumentGPR0, argumentGPR1, argumentGPR2, argumentGPR3 };
+ return registerForIndex[index];
+ }
+
static unsigned toIndex(GPRReg reg)
{
ASSERT(reg != InvalidGPRReg);
(In reply to comment #100)
With this buildfix there are only 8 failures on ARM Thumb2 Linux:
** The following JSC stress test failures have been introduced:
regress/script-tests/varargs-construct-inline.js.dfg-eager
regress/script-tests/varargs-construct-inline.js.dfg-eager-no-cjit-validate
v8-v6/v8-raytrace.js.always-trigger-copy-phase
v8-v6/v8-raytrace.js.default
v8-v6/v8-raytrace.js.dfg-eager
v8-v6/v8-raytrace.js.dfg-eager-no-cjit-validate
v8-v6/v8-raytrace.js.no-cjit-validate-phases
v8-v6/v8-raytrace.js.no-llint
Results for JSC stress tests:
8 failures found.
Comment on attachment 249245[details]
even more build fixes
View in context: https://bugs.webkit.org/attachment.cgi?id=249245&action=review
Stopped half way through as i have to do other things :-(
> Source/JavaScriptCore/bytecode/BytecodeKills.h:129
> + uintptr_t m_word;
could you make this a union instead of just having casts? It will save the bitwise_cast ugliness and make the code generally clearer
Created attachment 249257[details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Created attachment 249258[details]
Archive of layout-test-results from ews100 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-mavericks Platform: Mac OS X 10.9.5
(In reply to comment #101)
> (In reply to comment #100)
>
> With this buildfix there are only 8 failures on ARM Thumb2 Linux:
>
> ** The following JSC stress test failures have been introduced:
> regress/script-tests/varargs-construct-inline.js.dfg-eager
>
> regress/script-tests/varargs-construct-inline.js.dfg-eager-no-cjit-validate
> v8-v6/v8-raytrace.js.always-trigger-copy-phase
> v8-v6/v8-raytrace.js.default
> v8-v6/v8-raytrace.js.dfg-eager
> v8-v6/v8-raytrace.js.dfg-eager-no-cjit-validate
> v8-v6/v8-raytrace.js.no-cjit-validate-phases
> v8-v6/v8-raytrace.js.no-llint
>
> Results for JSC stress tests:
> 8 failures found.
Same failures on ARM traditional, and it seems
bug141193 will gone away with this patch.
(In reply to comment #102)
> Are these worth investigating?
>
> call-spread-apply 16.3509+-0.5089 !
> 31.5406+-1.6610 ! definitely 1.9290x slower
> call-spread-call 6.1589+-0.1679 !
> 25.1263+-1.5951 ! definitely 4.0797x slower
> destructuring-arguments 5.1739+-0.0185 !
> 16.8056+-0.7282 ! definitely 3.2482x slower
> direct-arguments-getbyval 0.9358+-0.0600 !
> 1.2183+-0.0840 ! definitely 1.3019x slower
> infer-closure-const-then-reenter 23.1630+-1.9191 !
> 124.7979+-2.0891 ! definitely 5.3878x slower
> inline-arguments-access 1.4097+-0.0432 !
> 4.2397+-0.0864 ! definitely 3.0076x slower
> inline-arguments-aliased-access 1.8209+-0.2001 !
> 4.2587+-0.0750 ! definitely 2.3388x slower
> tear-off-arguments-simple 1.9683+-0.1093 !
> 3.2880+-0.0646 ! definitely 1.6705x slower
> tear-off-arguments 2.7358+-0.0353 !
> 4.4778+-0.0407 ! definitely 1.6368x slower
From what I remember, these tests don't make it to FTL (don't run long enough, coverage, etc) and test arguments elimination. We expect those tests to slow down, since the new arguments elimination in DFG is intended just for handling foo.apply(bar, arguments).
But, I will look at those some more to make sure it's nothing horrible.
Attachment 249267[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:712: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:107: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:113: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:206: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:237: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:298: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:318: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:45: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:46: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:47: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:48: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:49: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:697: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:173: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:253: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:275: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:282: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:261: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/BytecodeKills.h:63: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4384: Multi line control clauses should use braces. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4669: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4672: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4673: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4674: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4676: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4681: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4698: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4699: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4700: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4702: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4708: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:155: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:171: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:196: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:207: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 35 in 251 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #106)
> Comment on attachment 249249[details]
> very latest
>
> Attachment 249249[details] did not pass mac-ews (mac):
> Output: http://webkit-queues.appspot.com/results/6510621394731008
>
> New failing tests:
> js/dom/const.html
Should be an easy fix. I remember there were a couple of const-related places where it felt like I was making an arbitrary decision. Probably I decided wrong, somewhere.
What's weird is that this test is part of js/dom. It shouldn't be. I'll fix that.
Created attachment 249286[details]
Archive of layout-test-results from ews100 for mac-mavericks
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-mavericks Platform: Mac OS X 10.9.5
Created attachment 249287[details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Created attachment 249313[details]
more fixes
ARM64, ARMv7, x86-32, and x86-64 should all work.
js/const is still regressed. I still haven't looked at it in detail, but at least it's not a DOM test anymore.
Attachment 249342[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:712: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:107: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:113: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:206: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:237: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:298: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:318: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:45: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:46: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:47: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:48: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:49: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:697: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:173: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:253: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:275: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:282: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:261: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/BytecodeKills.h:63: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4384: Multi line control clauses should use braces. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4669: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4672: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4673: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4674: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4676: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4681: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4698: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4699: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4700: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4702: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4708: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:155: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:171: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:196: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:207: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 35 in 251 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 249342[details]
the patch
View in context: https://bugs.webkit.org/attachment.cgi?id=249342&action=review
r=me with issues. I need a nap now.
Does object allocation sinking work for function objects? If not, I think we want a follow-up bug for that, since bytecode used to sink those allocations through conditional logic on the stack. I won't defend the old way of doing this, but I will say it's worth some speed on Octane.
> Source/JavaScriptCore/bytecode/BytecodeKills.h:92
> + Vector<unsigned>* vector = new Vector<unsigned>();
This is a leak. KillSet needs a destructor to delete m_word if & 1.
Also might be nice to isolate the encode/decode of m_word into helper functions: isEmpty(), isSingleItem(), singleItem(), setSingleItem(), setVector(), vector().
> Source/JavaScriptCore/bytecode/CodeBlock.cpp:-2824
> -bool CodeBlock::isCaptured(VirtualRegister operand, InlineCallFrame* inlineCallFrame) const
I will not miss this function.
> Source/JavaScriptCore/bytecode/CodeBlock.cpp:-2850
> -int CodeBlock::framePointerOffsetToGetActivationRegisters(int machineCaptureStart)
Nor this one.
> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:-1919
> - if (m_codeBlock->usesArguments() && m_codeBlock->numParameters() != 1 && !isStrictMode()) {
I will not miss this code.
> Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:331
> + void transform()
Would be nice to transform GetById(node, "callee") as well, to support anonymous functions that recurse by way of arguments.callee. Perhaps this is rare, though.
> Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp:-111
> -#if CPU(ARM64)
> - m_jit.pushToSave(scratch1);
> - m_jit.pushToSave(scratch2);
> -#else
> m_jit.push(scratch1);
> m_jit.push(scratch2);
> -#endif
A comment and a CRASH macro in MacroAssemblerARM64.h indicate that we need this special pushToSave behavior on ARM64. Why is it OK to remove this?
> Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp:-146
> -#if CPU(ARM64)
> - m_jit.pushToSave(scratch);
> -#else
Ditto.
> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4641
> +void SpeculativeJIT::compilePutToArguments(Node* node)
Do we need some write barrier action here?
> Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:850
> +void JIT::emit_op_put_to_arguments(Instruction* currentInstruction)
Do we need some write barrier action here?
> Source/JavaScriptCore/jit/JITPropertyAccess.cpp:821
> +void JIT::emit_op_put_to_arguments(Instruction* currentInstruction)
Do we need some write barrier action here?
> Source/JavaScriptCore/runtime/ArgumentsMode.h:31
> +enum ArgumentsMode {
Let's use enum class here.
> Source/JavaScriptCore/runtime/CapturedArgumentsOffset.h:35
> +// function. It only comes into play it the arguments aren't also lifted into the activation.
"play it" => "play if"
> Source/JavaScriptCore/runtime/CapturedArgumentsOffset.h:38
> +// If they were then accesses to the arguments would resolve to a ScopeOffset and not a
> +// CapturedArgumentsOffset.
> +class CapturedArgumentsOffset : public GenericOffset<CapturedArgumentsOffset> {
I'm confused by this name. Usually, "captured" means "it's in the activation". But here it means "it's not in the activation".
I guess by "captured" you meant "captured by the arguments object".
I think this offset is always an offset into a DirectArguments object. Right? If so, I suggest calling this "DirectArgumentsOffset". I think that would help disambiguate the capturing.
> Source/JavaScriptCore/runtime/OutOfBandArguments.h:41
> +class OutOfBandArguments : public JSNonFinalObject {
How about calling this CopiedArguments or ClonedArguments or SnapshottedArguments or StrictArguments?
"Out of band" didn't really speak to me because I don't normally think of any kind of arguments as being in-band, and in strict mode, these arguments are not really out of band.
> Source/JavaScriptCore/runtime/ScopedArguments.h:40
> +class ScopedArguments : public GenericArguments<ScopedArguments> {
How about calling this CapturedArguments on IndirectArguments or AliasedArguments?
We use "scope" in get_from_scope and put_to_scope to mean "a subclass of JSScope". But "scoped arguments" is a bit of an awkward phrase, partly because all arguments exist in some lexical scope and so there are no non-scoped arguments. I like "captured" because it's more specific about what triggered this behavior (a function that captured an argument), or I like "indirect" because it's the opposite of "direct" and it conveys that we forward requests somewhere else (even though we don't specify where), or I like "aliased" because it conveys sharing between the named reference and the indexed reference.
> Source/JavaScriptCore/runtime/VarOffset.h:36
> +enum VarKind : uint8_t {
enum class
> Source/JavaScriptCore/runtime/VarOffset.h:40
> + CapturedArgumentKind
Same comment about the term "captured" here. Can we call this DirectArgumentKind?
(In reply to comment #120)
> Comment on attachment 249342[details]
> the patch
>
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=249342&action=review
>
> r=me with issues. I need a nap now.
>
> Does object allocation sinking work for function objects? If not, I think we
> want a follow-up bug for that, since bytecode used to sink those allocations
> through conditional logic on the stack. I won't defend the old way of doing
> this, but I will say it's worth some speed on Octane.
>
> > Source/JavaScriptCore/bytecode/BytecodeKills.h:92
> > + Vector<unsigned>* vector = new Vector<unsigned>();
>
> This is a leak. KillSet needs a destructor to delete m_word if & 1.
Good catch!
>
> Also might be nice to isolate the encode/decode of m_word into helper
> functions: isEmpty(), isSingleItem(), singleItem(), setSingleItem(),
> setVector(), vector().
I'll try that.
>
> > Source/JavaScriptCore/bytecode/CodeBlock.cpp:-2824
> > -bool CodeBlock::isCaptured(VirtualRegister operand, InlineCallFrame* inlineCallFrame) const
>
> I will not miss this function.
>
> > Source/JavaScriptCore/bytecode/CodeBlock.cpp:-2850
> > -int CodeBlock::framePointerOffsetToGetActivationRegisters(int machineCaptureStart)
>
> Nor this one.
>
> > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:-1919
> > - if (m_codeBlock->usesArguments() && m_codeBlock->numParameters() != 1 && !isStrictMode()) {
>
> I will not miss this code.
>
> > Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:331
> > + void transform()
>
> Would be nice to transform GetById(node, "callee") as well, to support
> anonymous functions that recurse by way of arguments.callee. Perhaps this is
> rare, though.
I will add a FIXME and file a bug. I agree that this is worthwhile.
(I plan to make all new FIXMEs reference a bug.)
>
> > Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp:-111
> > -#if CPU(ARM64)
> > - m_jit.pushToSave(scratch1);
> > - m_jit.pushToSave(scratch2);
> > -#else
> > m_jit.push(scratch1);
> > m_jit.push(scratch2);
> > -#endif
>
> A comment and a CRASH macro in MacroAssemblerARM64.h indicate that we need
> this special pushToSave behavior on ARM64. Why is it OK to remove this?
Because this is 32_64 code, and so it'll never ARM64.
>
> > Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp:-146
> > -#if CPU(ARM64)
> > - m_jit.pushToSave(scratch);
> > -#else
>
> Ditto.
Ditto.
>
> > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4641
> > +void SpeculativeJIT::compilePutToArguments(Node* node)
>
> Do we need some write barrier action here?
FixupPhase says:
case PutClosureVar:
case PutToArguments: {
fixEdge<KnownCellUse>(node->child1());
insertStoreBarrier(m_indexInBlock, node->child1(), node->child2());
break;
}
So, this is fine.
>
> > Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp:850
> > +void JIT::emit_op_put_to_arguments(Instruction* currentInstruction)
>
> Do we need some write barrier action here?
It's a bug! I'll fix.
>
> > Source/JavaScriptCore/jit/JITPropertyAccess.cpp:821
> > +void JIT::emit_op_put_to_arguments(Instruction* currentInstruction)
>
> Do we need some write barrier action here?
Ditto.
>
> > Source/JavaScriptCore/runtime/ArgumentsMode.h:31
> > +enum ArgumentsMode {
>
> Let's use enum class here.
OK.
>
> > Source/JavaScriptCore/runtime/CapturedArgumentsOffset.h:35
> > +// function. It only comes into play it the arguments aren't also lifted into the activation.
>
> "play it" => "play if"
OK.
>
> > Source/JavaScriptCore/runtime/CapturedArgumentsOffset.h:38
> > +// If they were then accesses to the arguments would resolve to a ScopeOffset and not a
> > +// CapturedArgumentsOffset.
> > +class CapturedArgumentsOffset : public GenericOffset<CapturedArgumentsOffset> {
>
> I'm confused by this name. Usually, "captured" means "it's in the
> activation". But here it means "it's not in the activation".
>
> I guess by "captured" you meant "captured by the arguments object".
Right.
>
> I think this offset is always an offset into a DirectArguments object.
> Right?
Correct.
> If so, I suggest calling this "DirectArgumentsOffset". I think that
> would help disambiguate the capturing.
OK.
>
> > Source/JavaScriptCore/runtime/OutOfBandArguments.h:41
> > +class OutOfBandArguments : public JSNonFinalObject {
>
> How about calling this CopiedArguments or ClonedArguments or
> SnapshottedArguments or StrictArguments?
>
> "Out of band" didn't really speak to me because I don't normally think of
> any kind of arguments as being in-band, and in strict mode, these arguments
> are not really out of band.
Maybe SnapshottedArguments is the best.
>
> > Source/JavaScriptCore/runtime/ScopedArguments.h:40
> > +class ScopedArguments : public GenericArguments<ScopedArguments> {
>
> How about calling this CapturedArguments on IndirectArguments or
> AliasedArguments?
If we get rid of the term CapturedArgumentsOffset, the notion of "capture" doesn't really come into play anymore. IndirectArguments doesn't say why it's indirect. AliasedArguments is not quite right, since DirectArguments are also aliased - just they are aliased to the stack not to the scope.
I actually think that ScopedArguments is the best name - more on that below.
>
> We use "scope" in get_from_scope and put_to_scope to mean "a subclass of
> JSScope".
I actually think we use it to mean "in the scope". It's "get_from_scope" because the value is in the scope.
For this reason, ScopedArguments makes sense to me. I would agree with your argument if I had called it "ArgumentsScope" - clearly that's wrong, since this object isn't a scope. But "Scoped" here is used to describe the way the arguments are actually stored when we use this object, and in that regard, it's an accurate description - the arguments *are* in a subclass of JSScope.
> But "scoped arguments" is a bit of an awkward phrase, partly
> because all arguments exist in some lexical scope and so there are no
> non-scoped arguments. I like "captured" because it's more specific about
> what triggered this behavior (a function that captured an argument), or I
> like "indirect" because it's the opposite of "direct" and it conveys that we
> forward requests somewhere else (even though we don't specify where), or I
> like "aliased" because it conveys sharing between the named reference and
> the indexed reference.
Hmmm.
I think that of your proposed names, I like CapturedArguments the best, but I don't like it as much as ScopedArguments. I think I understand why you don't like ScopedArguments - the word "scope" means lexical scope and so using the term to describe arguments is a tautology. But, after bytecode generation the word "scope" is only used to mean that the value is in the heap.
In this patch we use the word "scope" this way in a bunch of places including:
- ScopeOffset, as opposed to VirtualRegister (aka StackOffset after I do the renaming), means that it's in the heap in a scope.
- VarOffset::isScope() means that it's in the heap in a scope.
Prior to this patch we already used the word "scope" this way:
- get_from_scope doesn't broadly mean that the variable is in a lexical scope; it means specifically that the variable is in the heap, in a scope object.
For that reason, ScopedArguments feels consistent with the way we usually use the word "scope": a variable is "scoped" or "in a scope" when it is in a scope object that is allocated in the heap.
Do you buy this argument for the name ScopedArguments?
>
> > Source/JavaScriptCore/runtime/VarOffset.h:36
> > +enum VarKind : uint8_t {
>
> enum class
OK.
>
> > Source/JavaScriptCore/runtime/VarOffset.h:40
> > + CapturedArgumentKind
>
> Same comment about the term "captured" here. Can we call this
> DirectArgumentKind?
Yup, that makes sense.
> For that reason, ScopedArguments feels consistent with the way we usually
> use the word "scope": a variable is "scoped" or "in a scope" when it is in a
> scope object that is allocated in the heap.
>
> Do you buy this argument for the name ScopedArguments?
Yeah, I'm OK with this.
OK, so the renamings will be like so:
What the patch previously called CapturedArgumentsOffset/CapturedArgumentKind will now be called DirectArgumentsOffset/DirectArgumentKind.
What the patch previously called OutOfBandArguments will now be called ArgumentsSnapshot. This is because I like the name SnapshottedArguments, but I don't like using the word "snapshot" as a verb and then making it past tense. It's awkward to say.
(In reply to comment #123)
> OK, so the renamings will be like so:
>
> What the patch previously called
> CapturedArgumentsOffset/CapturedArgumentKind will now be called
> DirectArgumentsOffset/DirectArgumentKind.
>
> What the patch previously called OutOfBandArguments will now be called
> ArgumentsSnapshot. This is because I like the name SnapshottedArguments,
> but I don't like using the word "snapshot" as a verb and then making it past
> tense. It's awkward to say.
No, this feels even weirder. I'm going with ClonedArguments.
Attachment 249470[details] did not pass style-queue:
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:712: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:107: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:113: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:208: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:239: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:300: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGArgumentsEliminationPhase.cpp:320: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:45: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:46: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:47: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:48: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:49: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:697: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:173: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGVarargsForwardingPhase.cpp:253: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:275: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:282: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:261: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/BytecodeKills.h:63: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4384: Multi line control clauses should use braces. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4669: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4672: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4673: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4674: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4676: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4681: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4698: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4699: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4700: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4702: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:4708: More than one command on the same line [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:155: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:171: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:196: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/dfg/DFGForAllKills.h:207: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 35 in 262 files
If any of these errors are false positives, please file a bug against check-webkit-style.
This patch is still appearing to fail some tests. Those failures don't reproduce locally. The issue is that we need to rm -rf WebKitBuild on all of the bots.
This patch passes all stress tests on Win32, but seems to cause an empty message view, when trying to view messages on www.hotmail.com. Is this the case on Mac 32/64 bit as well?
(In reply to comment #143)
> This patch passes all stress tests on Win32, but seems to cause an empty
> message view, when trying to view messages on www.hotmail.com. Is this the
> case on Mac 32/64 bit as well?
FYI, there's a bug that was introduced in r181993 that may cause similar symptoms. That bug is being tracked in https://bugs.webkit.org/show_bug.cgi?id=143407.
2015-02-03 14:16 PST, Filip Pizlo
2015-02-18 20:44 PST, Filip Pizlo
2015-02-18 20:45 PST, Filip Pizlo
2015-02-19 11:00 PST, Filip Pizlo
2015-02-20 10:25 PST, Filip Pizlo
2015-02-23 15:53 PST, Filip Pizlo
2015-02-23 20:37 PST, Filip Pizlo
2015-02-24 10:22 PST, Filip Pizlo
2015-02-24 12:39 PST, Filip Pizlo
2015-02-26 13:26 PST, Filip Pizlo
2015-02-26 16:57 PST, Filip Pizlo
2015-02-26 20:19 PST, Filip Pizlo
2015-02-27 08:25 PST, Filip Pizlo
2015-02-27 13:15 PST, Filip Pizlo
2015-02-28 20:10 PST, Filip Pizlo
2015-03-01 12:46 PST, Filip Pizlo
2015-03-01 16:44 PST, Filip Pizlo
2015-03-01 18:38 PST, Filip Pizlo
2015-03-01 20:35 PST, Filip Pizlo
2015-03-01 20:48 PST, Filip Pizlo
2015-03-02 12:41 PST, Filip Pizlo
2015-03-02 17:05 PST, Filip Pizlo
2015-03-02 19:45 PST, Filip Pizlo
2015-03-03 11:51 PST, Filip Pizlo
2015-03-03 22:57 PST, Filip Pizlo
2015-03-04 22:20 PST, Filip Pizlo
2015-03-05 12:40 PST, Filip Pizlo
2015-03-05 13:50 PST, Filip Pizlo
2015-03-05 14:31 PST, Filip Pizlo
2015-03-06 18:02 PST, Filip Pizlo
2015-03-06 20:07 PST, Filip Pizlo
2015-03-07 08:43 PST, Filip Pizlo
2015-03-07 11:52 PST, Filip Pizlo
2015-03-07 17:20 PST, Filip Pizlo
2015-03-07 19:55 PST, Filip Pizlo
2015-03-08 23:17 PDT, Filip Pizlo
2015-03-09 12:27 PDT, Filip Pizlo
2015-03-09 16:55 PDT, Filip Pizlo
2015-03-10 11:46 PDT, Filip Pizlo
2015-03-10 17:19 PDT, Filip Pizlo
2015-03-10 19:06 PDT, Filip Pizlo
2015-03-10 20:38 PDT, Filip Pizlo
2015-03-11 15:59 PDT, Filip Pizlo
2015-03-11 20:08 PDT, Filip Pizlo
2015-03-12 20:40 PDT, Filip Pizlo
2015-03-12 22:05 PDT, Filip Pizlo
2015-03-13 12:10 PDT, Filip Pizlo
2015-03-13 15:20 PDT, Filip Pizlo
2015-03-13 17:11 PDT, Filip Pizlo
2015-03-13 19:19 PDT, Filip Pizlo
2015-03-13 19:49 PDT, Filip Pizlo
2015-03-16 20:09 PDT, Filip Pizlo
2015-03-17 10:53 PDT, Filip Pizlo
2015-03-17 12:14 PDT, Filip Pizlo
2015-03-17 13:58 PDT, Filip Pizlo
2015-03-17 17:20 PDT, Filip Pizlo
2015-03-17 21:15 PDT, Filip Pizlo
2015-03-18 11:52 PDT, Filip Pizlo
2015-03-18 15:09 PDT, Filip Pizlo
2015-03-18 15:52 PDT, Filip Pizlo
2015-03-19 07:48 PDT, Filip Pizlo
2015-03-20 18:19 PDT, Filip Pizlo
2015-03-20 19:58 PDT, Filip Pizlo
2015-03-21 08:44 PDT, Filip Pizlo
2015-03-21 10:33 PDT, Filip Pizlo
2015-03-21 11:09 PDT, Filip Pizlo
2015-03-21 18:27 PDT, Filip Pizlo
2015-03-21 20:33 PDT, Filip Pizlo
2015-03-22 13:11 PDT, Filip Pizlo
2015-03-22 18:00 PDT, Filip Pizlo
2015-03-22 19:27 PDT, Filip Pizlo
2015-03-22 21:32 PDT, Filip Pizlo
2015-03-23 11:14 PDT, Filip Pizlo
2015-03-23 11:38 PDT, Filip Pizlo
2015-03-23 11:43 PDT, Filip Pizlo
2015-03-23 12:16 PDT, Filip Pizlo
2015-03-23 13:35 PDT, Build Bot
2015-03-23 13:41 PDT, Build Bot
2015-03-23 14:09 PDT, Filip Pizlo
2015-03-23 15:23 PDT, Build Bot
2015-03-23 15:31 PDT, Build Bot
2015-03-23 22:51 PDT, Filip Pizlo
2015-03-24 13:17 PDT, Filip Pizlo
2015-03-25 20:04 PDT, Filip Pizlo
2015-03-25 20:51 PDT, Filip Pizlo