[v8-dev] Re: [turbofan] Fix type feedback vector for stub compilation. (issue 1320623004 by mstarzin...@chromium.org)

2016-06-13 Thread mstarzinger
This is no longer needed now that the compilation pipeline has been straightened out. Also, stub compilation no longer goes through the AST graph builder. Closing. https://codereview.chromium.org/1320623004/ -- -- v8-dev mailing list v8-dev@googlegroups.com

[v8-dev] Re: Add a html-based visualizer for TurboFan graphs (issue 729913004 by da...@chromium.org)

2016-05-13 Thread mstarzinger
LGTM (rubber-stamped). https://codereview.chromium.org/729913004/diff/580001/tools/turbolizer/README File tools/turbolizer/README (right): https://codereview.chromium.org/729913004/diff/580001/tools/turbolizer/README#newcode1 tools/turbolizer/README:1: Graph visualization and manipulation

[v8-dev] Re: [turbofan] Enable concurrent (re)compilation. (issue 1179393008 by bmeu...@chromium.org)

2016-04-27 Thread mstarzinger
LGTM on the new version as well. https://codereview.chromium.org/1179393008/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and

[v8-dev] Re: Unify all four JSConstructStub generators into one. (issue 1221803004 by mstarzin...@chromium.org)

2015-11-18 Thread mstarzinger
Closing. This is being obsoleteted by: https://codereview.chromium.org/1448933002/ https://codereview.chromium.org/1221803004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups

[v8-dev] Re: ES6: Array.prototype.slice and friends should use ToLength instead of ToUint32 (issue 1309243003 by ape...@igalia.com)

2015-09-16 Thread mstarzinger
LGTM (rubber-stamp for heap). https://codereview.chromium.org/1309243003/diff/60001/src/bootstrapper.cc File src/bootstrapper.cc (right): https://codereview.chromium.org/1309243003/diff/60001/src/bootstrapper.cc#newcode1857 src/bootstrapper.cc:1857: Handle flag(FLAG_harmony_tolength ?

[v8-dev] Re: ES6: Array.prototype.slice and friends should use ToLength instead of ToUint32 (issue 1309243003 by ape...@igalia.com)

2015-09-16 Thread mstarzinger
LGTM (rubber-stamp for heap). https://codereview.chromium.org/1309243003/diff/60001/src/bootstrapper.cc File src/bootstrapper.cc (right): https://codereview.chromium.org/1309243003/diff/60001/src/bootstrapper.cc#newcode1857 src/bootstrapper.cc:1857: Handle flag(FLAG_harmony_tolength ?

[v8-dev] Re: When parsing inner functions, try to allocate in a temporary Zone (issue 1304923004 by conr...@chromium.org)

2015-09-10 Thread mstarzinger
Looking good, just one suggestion. https://codereview.chromium.org/1304923004/diff/11/src/parser.cc File src/parser.cc (right): https://codereview.chromium.org/1304923004/diff/11/src/parser.cc#newcode4241 src/parser.cc:4241: factory()->set_zone(outer_zone); Suggestion as discsussed

[v8-dev] Re: When parsing inner functions, try to allocate in a temporary Zone (issue 1304923004 by conr...@chromium.org)

2015-09-10 Thread mstarzinger
LGTM from my end. https://codereview.chromium.org/1304923004/diff/180001/src/ast.h File src/ast.h (right): https://codereview.chromium.org/1304923004/diff/180001/src/ast.h#newcode3696 src/ast.h:3696: // HeapObjects which need to persist until scope analysis must be allocated in nit:

[v8-dev] Re: [Interpreter] Add support for property store operations. (issue 1319833004 by rmcil...@chromium.org)

2015-09-09 Thread mstarzinger
LGTM. https://codereview.chromium.org/1319833004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: [turbofan] Handle stack overflow exceptions in JSInliner. (issue 1322203005 by mstarzin...@chromium.org)

2015-09-09 Thread mstarzinger
Thanks for review. Regression test added. Landing. https://codereview.chromium.org/1322203005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe

[v8-dev] Re: [heap] Fix MemoryChunk::kHeaderSize computation and add some assertions. (issue 1302423007 by mlippa...@chromium.org)

2015-09-08 Thread mstarzinger
LGTM, just nits and a suggestion. https://codereview.chromium.org/1302423007/diff/20001/src/heap/spaces.h File src/heap/spaces.h (right): https://codereview.chromium.org/1302423007/diff/20001/src/heap/spaces.h#newcode563 src/heap/spaces.h:563: kPointerSize // intptr_t

[v8-dev] Re: [Interpreter] Ensure that implicit return undefined is generated. (issue 1308693014 by rmcil...@chromium.org)

2015-09-08 Thread mstarzinger
I am not an owner of anything in this CL, but I am happy to offer my thoughts ... https://codereview.chromium.org/1308693014/diff/1/src/interpreter/bytecode-generator.cc File src/interpreter/bytecode-generator.cc (right):

[v8-dev] Re: [Interpreter] Ensure that implicit return undefined is generated. (issue 1308693014 by rmcil...@chromium.org)

2015-09-08 Thread mstarzinger
LGTM (rubber-stamped). https://codereview.chromium.org/1308693014/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop

[v8-dev] Fix AstPrinter::VisitCallRuntime to not print garbage. (issue 1329133002 by mstarzin...@chromium.org)

2015-09-08 Thread mstarzinger
Reviewers: Benedikt Meurer, Yang OOO until mid October, Message: Benedikt: PTAL Yang: FYI Description: Fix AstPrinter::VisitCallRuntime to not print garbage. R=bmeu...@chromium.org Please review this at https://codereview.chromium.org/1329133002/ Base URL:

[v8-dev] Re: [heap] Remove obsolete DisallowAllocationFailure scope. (issue 1319153006 by mstarzin...@chromium.org)

2015-09-08 Thread mstarzinger
https://codereview.chromium.org/1319153006/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it,

[v8-dev] [heap] Remove obsolete DisallowAllocationFailure scope. (issue 1319153006 by mstarzin...@chromium.org)

2015-09-08 Thread mstarzinger
Reviewers: Michael Lippautz, Description: [heap] Remove obsolete DisallowAllocationFailure scope. This removes the DisallowAllocationFailure assertion scope which mostly coincided with the AlwaysAllocateScope anyways. Access to the bitfield in the Isolate was not synchronized and hence the

[v8-dev] [heap] Prevent leakage of GCCallbacksScope outside of heap. (issue 1314543014 by mstarzin...@chromium.org)

2015-09-08 Thread mstarzinger
Reviewers: Michael Lippautz, Description: [heap] Prevent leakage of GCCallbacksScope outside of heap. R=mlippa...@chromium.org Please review this at https://codereview.chromium.org/1314543014/ Base URL: https://chromium.googlesource.com/v8/v8.git@local_cleanup-remove-alloc-failure-scope

[v8-dev] Use baseline code to compute message locations. (issue 1331603002 by mstarzin...@chromium.org)

2015-09-08 Thread mstarzinger
Reviewers: Benedikt Meurer, Description: Use baseline code to compute message locations. This switches Isolate::ComputeLocation to use baseline code when computing message locations. This unifies locations between optimized and non-optimized code by always going through the FrameSummary for

[v8-dev] [turbofan] Handle stack overflow exceptions in JSInliner. (issue 1322203005 by mstarzin...@chromium.org)

2015-09-08 Thread mstarzinger
Reviewers: Benedikt Meurer, Message: Still cleaning up the repro to make a regression test. Description: [turbofan] Handle stack overflow exceptions in JSInliner. R=bmeu...@chromium.org BUG=chromium:527364 LOG=n Please review this at https://codereview.chromium.org/1322203005/ Base URL:

[v8-dev] Vector ICs: The Oracle needs to report feedback for the count op. (issue 1321993004 by mvstan...@chromium.org)

2015-09-07 Thread mstarzinger
Conceptually I like this change, if memory footprint is acceptable. https://codereview.chromium.org/1321993004/diff/40001/src/ast.h File src/ast.h (right): https://codereview.chromium.org/1321993004/diff/40001/src/ast.h#newcode1429 src/ast.h:1429: int ic_slot_count_; As discussed offline,

[v8-dev] [turbofan] Clarify comment about Parameter indexing. (issue 1329043002 by mstarzin...@chromium.org)

2015-09-07 Thread mstarzinger
Reviewers: oth, rmcilroy, Description: [turbofan] Clarify comment about Parameter indexing. This calrifies a comments in the AstGraphBuilder that has lead to confusion about what "parameter index" refers to. The off-by-one is confusing and a terribly phrased comment doesn't make it any better.

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-07 Thread mstarzinger
https://codereview.chromium.org/1291693004/diff/180001/src/compiler/bytecode-graph-builder.cc File src/compiler/bytecode-graph-builder.cc (right): https://codereview.chromium.org/1291693004/diff/180001/src/compiler/bytecode-graph-builder.cc#newcode46 src/compiler/bytecode-graph-builder.cc:46:

[v8-dev] [presubmit] Enable build/c++11 linter checking. (issue 1317463007 by mstarzin...@chromium.org)

2015-09-07 Thread mstarzinger
de "src/compiler.h" Index: tools/presubmit.py diff --git a/tools/presubmit.py b/tools/presubmit.py index 58c130e95f9ad8d9f39fde9b3eb5d8ef2dc62f4e..c11e89ddb872ae978c8bf92cd17c3754f08dfa0a 100755 --- a/tools/presubmit.py +++ b/tools/presubmit.py @@ -53,7 +53,6 @@ from subprocess import PIPE

[v8-dev] Re: [stubs] Unify the various versions of [[Call]] with CallCallableStub. (issue 1311013008 by bmeu...@chromium.org)

2015-09-07 Thread mstarzinger
LGTM. https://codereview.chromium.org/1311013008/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: Vector ICs: The Oracle needs to report feedback for the count op. (issue 1321993004 by mvstan...@chromium.org)

2015-09-07 Thread mstarzinger
LGTM on TurboFan, only nits, didn't look at the rest. https://codereview.chromium.org/1321993004/diff/60001/src/ast.h File src/ast.h (right): https://codereview.chromium.org/1321993004/diff/60001/src/ast.h#newcode1510 src/ast.h:1510: // Expression* setter; nit: Still look like a left-over.

[v8-dev] Re: Add template parameter and unittests to atomic utils. (issue 1324153003 by mlippa...@chromium.org)

2015-09-04 Thread mstarzinger
https://codereview.chromium.org/1324153003/diff/20001/src/atomic-utils.h File src/atomic-utils.h (right): https://codereview.chromium.org/1324153003/diff/20001/src/atomic-utils.h#newcode119 src/atomic-utils.h:119: class AtomicFlag { On 2015/09/04 11:09:32, Michael Lippautz wrote: Maybe another

[v8-dev] [heap] Separate scavenger functionality into own file. (issue 1323993004 by mstarzin...@chromium.org)

2015-09-04 Thread mstarzinger
Reviewers: Hannes Payer (OOO), Michael Lippautz, Description: [heap] Separate scavenger functionality into own file. This moves scavenging functionality into a separate component to that neither the scavenger nor objects-visiting need to be exposed outside the heap.

[v8-dev] [heap] No leakage of objects-visiting.h outside of heap. (issue 1328003002 by mstarzin...@chromium.org)

2015-09-04 Thread mstarzinger
Reviewers: Hannes Payer (OOO), Michael Lippautz, Description: [heap] No leakage of objects-visiting.h outside of heap. This prevents the internal objects-visiting.h to be usable outisde of the "heap" directory. The static object visitation is only usefull within the GC and is now properly

[v8-dev] Remove obsolete DEBUG and NDEBUG macro dance. (issue 1315063007 by mstarzin...@chromium.org)

2015-09-04 Thread mstarzinger
Reviewers: ulan, Description: Remove obsolete DEBUG and NDEBUG macro dance. The original intention of this seemed to have been to enable DEBUG when NDEBUG was not defined within Google3. Everything since then was just added to avoid the "#error" below checking for consistency from firing.

[v8-dev] Re: Remove obsolete DEBUG and NDEBUG macro dance. (issue 1315063007 by mstarzin...@chromium.org)

2015-09-04 Thread mstarzinger
For posterity, the following is what this section looked like originally. The intended semantics were different back then ... +// Google3 uses NDEBUG. +#if defined(GOOGLE3) && !defined(NDEBUG) +#define DEBUG +#endif + +// V8 only uses DEBUG, but included external files +// may use NDEBUG -

[v8-dev] Re: Add template parameter and unittests to atomic utils. (issue 1324153003 by mlippa...@chromium.org)

2015-09-04 Thread mstarzinger
LGTM. https://codereview.chromium.org/1324153003/diff/10005/src/atomic-utils.h File src/atomic-utils.h (right): https://codereview.chromium.org/1324153003/diff/10005/src/atomic-utils.h#newcode67 src/atomic-utils.h:67: struct cast_helper { Got a rhyme for you: You sneaky C++ wiz, I am fine

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-04 Thread mstarzinger
https://codereview.chromium.org/1291693004/diff/160001/src/compiler/linkage.h File src/compiler/linkage.h (right): https://codereview.chromium.org/1291693004/diff/160001/src/compiler/linkage.h#newcode333 src/compiler/linkage.h:333: static const int kInterpreterReceiverParameter = -1; On

[v8-dev] Re: [heap] Separate scavenger functionality into own file. (issue 1323993004 by mstarzin...@chromium.org)

2015-09-04 Thread mstarzinger
Thanks. Yes, I definitely want to get Hannes' opinion on this too. https://codereview.chromium.org/1323993004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group.

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-04 Thread mstarzinger
https://codereview.chromium.org/1291693004/diff/180001/src/compiler/bytecode-graph-builder.cc File src/compiler/bytecode-graph-builder.cc (right): https://codereview.chromium.org/1291693004/diff/180001/src/compiler/bytecode-graph-builder.cc#newcode46 src/compiler/bytecode-graph-builder.cc:46:

[v8-dev] Re: [runtime] Introduce a dedicated JSIteratorResult type. (issue 1302173007 by bmeu...@chromium.org)

2015-09-03 Thread mstarzinger
LGTM (rubber-stamp for "heap", didn't look at the rest at all). https://codereview.chromium.org/1302173007/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-03 Thread mstarzinger
LGTM. Just nits and a suggestions. https://codereview.chromium.org/1291693004/diff/140001/src/compiler/bytecode-graph-builder.cc File src/compiler/bytecode-graph-builder.cc (right): https://codereview.chromium.org/1291693004/diff/140001/src/compiler/bytecode-graph-builder.cc#newcode25

[v8-dev] Re: [heap] Make AlwaysAllocateScope thread-safe. (issue 1325173003 by mlippa...@chromium.org)

2015-09-03 Thread mstarzinger
LGTM. Just for posterity, because it took me a while to figure out this race: This is PagedSpace::SlowAllocateRaw running on the sub-thread when allocating in the CompactionSpace reading the count, racing against, any AlwaysAllocateScope on the main-thread mutating the count.

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-03 Thread mstarzinger
https://codereview.chromium.org/1291693004/diff/140001/src/compiler/bytecode-graph-builder.cc File src/compiler/bytecode-graph-builder.cc (right): https://codereview.chromium.org/1291693004/diff/140001/src/compiler/bytecode-graph-builder.cc#newcode48 src/compiler/bytecode-graph-builder.cc:48:

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-03 Thread mstarzinger
https://codereview.chromium.org/1291693004/diff/140001/src/compiler/bytecode-graph-builder.cc File src/compiler/bytecode-graph-builder.cc (right): https://codereview.chromium.org/1291693004/diff/140001/src/compiler/bytecode-graph-builder.cc#newcode48 src/compiler/bytecode-graph-builder.cc:48:

[v8-dev] Re: [Interpreter] Add support for property load operations. (issue 1309843007 by rmcil...@chromium.org)

2015-09-02 Thread mstarzinger
Looking good, just one issue about evaluation order. https://codereview.chromium.org/1309843007/diff/60001/src/interpreter/bytecode-generator.cc File src/interpreter/bytecode-generator.cc (right):

[v8-dev] [heap] Move ObjectStatsVisitor into the proper component. (issue 1310953008 by mstarzin...@chromium.org)

2015-09-02 Thread mstarzinger
Reviewers: Michael Lippautz, Description: [heap] Move ObjectStatsVisitor into the proper component. This is a follow-up to a previous change and moved object statistics tracking into its own component. It is no longer intertwinded with the normal marking logic, but separated out into

[v8-dev] Re: Simplify object stats tracker a bit. (issue 10802032)

2015-09-02 Thread mstarzinger
Reviewers: danno, Message: Closing. Superseded by: https://codereview.chromium.org/1310953008/ Description: Simplify object stats tracker a bit. R=da...@chromium.org Please review this at https://codereview.chromium.org/10802032/ Base URL:

[v8-dev] Re: [Interpreter] Add support for property load operations. (issue 1309843007 by rmcil...@chromium.org)

2015-09-02 Thread mstarzinger
LGTM from my end. https://codereview.chromium.org/1309843007/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving

[v8-dev] [presubmit] Fix build/include linter violations. (issue 1318863004 by mstarzin...@chromium.org)

2015-09-02 Thread mstarzinger
Reviewers: Benedikt Meurer, Description: [presubmit] Fix build/include linter violations. R=bmeu...@chromium.org Please review this at https://codereview.chromium.org/1318863004/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+94, -22 lines): M include/v8.h

[v8-dev] [presubmit] Fix whitespace/empty_loop_body linter violations. (issue 1327523003 by mstarzin...@chromium.org)

2015-09-02 Thread mstarzinger
Reviewers: Benedikt Meurer, Description: [presubmit] Fix whitespace/empty_loop_body linter violations. R=bmeu...@chromium.org Please review this at https://codereview.chromium.org/1327523003/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+1, -3 lines): M

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-02 Thread mstarzinger
Looks sensible from a high-level point of view. No concrete actionable feedback at this point. I like it. Just left a couple of random ramblings. https://codereview.chromium.org/1291693004/diff/11/src/compiler/bytecode-graph-builder.cc File src/compiler/bytecode-graph-builder.cc (right):

[v8-dev] Re: [Interpreter] Skeleton bytecode graph builder [NOT FOR COMMIT] (issue 1291693004 by o...@chromium.org)

2015-09-02 Thread mstarzinger
https://codereview.chromium.org/1291693004/diff/80001/src/compiler/bytecode-graph-builder.h File src/compiler/bytecode-graph-builder.h (right): https://codereview.chromium.org/1291693004/diff/80001/src/compiler/bytecode-graph-builder.h#newcode22 src/compiler/bytecode-graph-builder.h:22:

[v8-dev] [heap] Separate ObjectStats out into its own class. (issue 1326793002 by mstarzin...@chromium.org)

2015-09-02 Thread mstarzinger
Reviewers: Michael Lippautz, Description: [heap] Separate ObjectStats out into its own class. Note that this is only pulling out the bookkeeping side of things, the marking visitor that actually records the statistics should also move into the ObjectStats class. That will be done as a

[v8-dev] Re: [heap] Throw OOM upon failing to expand a PagedSpace above old gen limits. (issue 1316183004 by mlippa...@chromium.org)

2015-09-01 Thread mstarzinger
LGTM. https://codereview.chromium.org/1316183004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: [futex] Allow debugger to break in the middle of a futexWait (issue 1321543004 by bi...@chromium.org)

2015-09-01 Thread mstarzinger
On 2015/08/31 22:37:01, binji wrote: > Michael is right. The current solution is not ideal either. How about this: instead of threading this through execution.cc, we could set a bool on the Debug object via a scope similar to IgnoreBreaks (I think thats how its called). That would contain

[v8-dev] Re: Make type-feedback-vector.h usable without objects-inl.h header (and others). (issue 1309303008 by mvstan...@chromium.org)

2015-09-01 Thread mstarzinger
Awesome. Thanks! LGTM with one comment to address. https://codereview.chromium.org/1309303008/diff/1/src/type-feedback-vector.cc File src/type-feedback-vector.cc (left): https://codereview.chromium.org/1309303008/diff/1/src/type-feedback-vector.cc#oldcode5 src/type-feedback-vector.cc:5:

[v8-dev] Re: MIPS: Fix QuietSignalingNaNs on mips32r6. (issue 1311473007 by akos.pa...@imgtec.com)

2015-09-01 Thread mstarzinger
LGTM (rubber-stamped). https://codereview.chromium.org/1311473007/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop

[v8-dev] Re: [heap,cctest] Fix CodeRange tests that call AllocateRawMemory directly. (issue 1325853003 by mlippa...@chromium.org)

2015-09-01 Thread mstarzinger
LGTM. Nice catch. https://codereview.chromium.org/1325853003/diff/40001/src/base/platform/platform-linux.cc File src/base/platform/platform-linux.cc (right): https://codereview.chromium.org/1325853003/diff/40001/src/base/platform/platform-linux.cc#newcode313

[v8-dev] Make presubmit.py rules differential. (issue 1325833005 by mstarzin...@chromium.org)

2015-09-01 Thread mstarzinger
space/indent -whitespace/labels -whitespace/line_length -whitespace/newline -whitespace/operators -whitespace/parens -whitespace/tab -whitespace/todo -""".split() - +# named "string" and "map" assuming that you needed to include STL headers. # TODO(bmeurer):

[v8-dev] Re: Make presubmit.py rules differential. (issue 1325833005 by mstarzin...@chromium.org)

2015-09-01 Thread mstarzinger
Thanks! Yep did so, ran "rm .cpplint-cache; ./tools/presubmit.py" locally. https://codereview.chromium.org/1325833005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev"

[v8-dev] [presubmit] Fix runtime/indentation_namespace linter violations. (issue 1302413007 by mstarzin...@chromium.org)

2015-09-01 Thread mstarzinger
Reviewers: Michael Achenbach, Description: [presubmit] Fix runtime/indentation_namespace linter violations. R=machenb...@chromium.org Please review this at https://codereview.chromium.org/1302413007/ Base URL: https://chromium.googlesource.com/v8/v8.git@local_fix-presubmit-rules Affected

[v8-dev] Re: [heap] Fix recursive GCs caused by adjusting externally allocated memory (issue 1325643002 by mlippa...@chromium.org)

2015-08-31 Thread mstarzinger
LGTM. Thanks! https://codereview.chromium.org/1325643002/diff/1/src/api.cc File src/api.cc (right): https://codereview.chromium.org/1325643002/diff/1/src/api.cc#newcode6876 src/api.cc:6876: DCHECK(heap->gc_state() == i::Heap::NOT_IN_GC); nit: DCHECK_EQ?

[v8-dev] Make isolate.h usable without objects-inl.h header. (issue 1322883002 by mstarzin...@chromium.org)

2015-08-31 Thread mstarzinger
Reviewers: Benedikt Meurer, Message: Benedikt: PTAL. Igor: Please provide "ic" OWNER review. Also, I am looking for a "regexp" OWNER that is _not_ on vacation right now. :( Description: Make isolate.h usable without objects-inl.h header. This CL us a pure refactoring that makes an empty

[v8-dev] [turbofan] Remove obsolete unique.h includes in TurboFan. (issue 1321223002 by mstarzin...@chromium.org)

2015-08-31 Thread mstarzinger
mpiler/node-matchers.h" #include "src/compiler/node-properties.h" #include "src/compiler/operator-properties.h" +#include "src/counters.h" +#include "src/objects-inl.h" namespace v8 { namespace internal { Index: src/compiler/js-operator.cc diff -

[v8-dev] Re: [heap] Properly decrement amount of externally allocated memory (issue 1329493002 by mlippa...@chromium.org)

2015-08-31 Thread mstarzinger
LGTM. Totally slipped my review. Kudos to Fedor Indutny for noticing. https://codereview.chromium.org/1329493002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev"

[v8-dev] Re: [futex] Allow debugger to break in the middle of a futexWait (issue 1321543004 by bi...@chromium.org)

2015-08-31 Thread mstarzinger
Some concerns. Also adding Jaro who might have an opinion about this. https://codereview.chromium.org/1321543004/diff/20001/src/execution.h File src/execution.h (right): https://codereview.chromium.org/1321543004/diff/20001/src/execution.h#newcode212 src/execution.h:212:

[v8-dev] Make unsafe Unique constructor private. (issue 1326493002 by mstarzin...@chromium.org)

2015-08-31 Thread mstarzinger
Reviewers: titzer, Description: Make unsafe Unique constructor private. The constructor taking an artificial raw address was only used as a workaround in TurboFan. It should only be accessible by constructor functions internal to Unique. R=tit...@chromium.org Please review this at

[v8-dev] Re: [builtins] Pass correct number of arguments after adapting arguments. (issue 1306423003 by bmeu...@chromium.org)

2015-08-31 Thread mstarzinger
LGTM. https://codereview.chromium.org/1306423003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Drop ambiguous MaybeHandle comparison and hashing ops. (issue 1319383002 by mstarzin...@chromium.org)

2015-08-31 Thread mstarzinger
Reviewers: Benedikt Meurer, Description: Drop ambiguous MaybeHandle comparison and hashing ops. The default equality comparison operators and hashing functions for Handles are ambiguous. The intended semantics might have either been based on Handle locations or on object identity. This is why

[v8-dev] Re: [runtime] Use utils.InstallFunctions for Symbol.prototype[@@toPrimitive]. (issue 1310163004 by bmeu...@chromium.org)

2015-08-31 Thread mstarzinger
LGTM. https://codereview.chromium.org/1310163004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Make frames.h usable without handles-inl.h header. (issue 1319423003 by mstarzin...@chromium.org)

2015-08-31 Thread mstarzinger
Reviewers: Igor Sheludko, Description: Make frames.h usable without handles-inl.h header. This CL us a pure refactoring that makes an empty compilation unit including just "frames.h" but not "handles-inl.h" compile without warnings or errors. This is needed to further reduce the header

[v8-dev] Re: [es6] Implement Date.prototype[@@toPrimitive] as C++ builtin. (issue 1324713002 by bmeu...@chromium.org)

2015-08-31 Thread mstarzinger
LGTM. https://codereview.chromium.org/1324713002/diff/1/src/objects.h File src/objects.h (right): https://codereview.chromium.org/1324713002/diff/1/src/objects.h#newcode176 src/objects.h:176: // Valid hints for the abstract operations OrdinaryToPrimitive, nit: s/operations/operation/

[v8-dev] Re: [es6] Implement spec compliant ToPrimitive in the runtime. (issue 1306303003 by bmeu...@chromium.org)

2015-08-28 Thread mstarzinger
LGTM modulo spec changes, didn't verify those. https://codereview.chromium.org/1306303003/diff/1/src/objects.cc File src/objects.cc (right): https://codereview.chromium.org/1306303003/diff/1/src/objects.cc#newcode6017 src/objects.cc:6017: HandleObject exoticToPrim; nit: not likey camelCase

[v8-dev] Re: Improve handling of debug name in CompilationInfo. (issue 1320103002 by tit...@chromium.org)

2015-08-28 Thread mstarzinger
LGTM. https://codereview.chromium.org/1320103002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] [heap] Move IdentityMap data structure out of heap. (issue 1320503004 by mstarzin...@chromium.org)

2015-08-28 Thread mstarzinger
Reviewers: titzer, rmcilroy, Message: Ben: PTAL. Ross: FYI. Description: [heap] Move IdentityMap data structure out of heap. This data structure uses the public heap API only and is not specific to any heap internals. It should be usable throughout V8 and inclusion of the header file should

[v8-dev] Re: [Interpreter] Add support for loading literals from the constant pool. (issue 1321663003 by rmcil...@chromium.org)

2015-08-28 Thread mstarzinger
Looking good. Just a few comments. https://codereview.chromium.org/1321663003/diff/40001/src/DEPS File src/DEPS (right): https://codereview.chromium.org/1321663003/diff/40001/src/DEPS#newcode8 src/DEPS:8: +src/heap/identity-map.h, I have moved the IdentityMap out of the heap directory. This

[v8-dev] Re: [runtime] Add %ToString and %_ToString and remove the TO_STRING builtin. (issue 1319973007 by bmeu...@chromium.org)

2015-08-28 Thread mstarzinger
LGTM on TurboFan, didn't look at the rest. https://codereview.chromium.org/1319973007/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this

[v8-dev] [turbofan] Remove usage of UniqueT from graph. (issue 1314473007 by mstarzin...@chromium.org)

2015-08-28 Thread mstarzinger
Reviewers: *Benedikt Meurer, *titzer, https://codereview.chromium.org/1314473007/diff/1/src/compiler/common-operator.h File src/compiler/common-operator.h (right): https://codereview.chromium.org/1314473007/diff/1/src/compiler/common-operator.h#newcode10 src/compiler/common-operator.h:10:

[v8-dev] Re: Add test-run-native-calls tests for mixed parameters. (issue 1314973004 by tit...@chromium.org)

2015-08-28 Thread mstarzinger
LGTM (rubber-stamped). https://codereview.chromium.org/1314973004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop

[v8-dev] Re: [Interpreter] Add support for loading literals from the constant pool. (issue 1321663003 by rmcil...@chromium.org)

2015-08-28 Thread mstarzinger
LGTM. https://codereview.chromium.org/1321663003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: [turbofan] Remove usage of UniqueT from graph. (issue 1314473007 by mstarzin...@chromium.org)

2015-08-28 Thread mstarzinger
OK, after a tirade of fixes for VC++ I got this to compile on all platforms. This should also fix many UBSan issues because we now perform correct casting in OpParameter throughout the codebase. Now that we are using HandleT, which does not provide an equality comparison operator, all the

[v8-dev] Re: [turbofan] Remove obsolete BuildLoadBuiltinsObject. (issue 1305163008 by yang...@chromium.org)

2015-08-27 Thread mstarzinger
LGTM. Thanks! https://codereview.chromium.org/1305163008/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving

[v8-dev] Re: [wasm] Move the (conditional) installation of the WASM api into bootstrapper.cc. (issue 1319003002 by tit...@chromium.org)

2015-08-27 Thread mstarzinger
LGTM. https://codereview.chromium.org/1319003002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: [heap] Get rid of dead code in HeapIterator. (issue 1319953003 by mlippa...@chromium.org)

2015-08-27 Thread mstarzinger
LGTM. https://codereview.chromium.org/1319953003/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] [heap] Remove raw unchecked root set accessors. (issue 1305163007 by mstarzin...@chromium.org)

2015-08-27 Thread mstarzinger
Reviewers: Hannes Payer (slow OOO soon), Description: [heap] Remove raw unchecked root set accessors. R=hpa...@chromium.org BUG=v8:1490 LOG=n Please review this at https://codereview.chromium.org/1305163007/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+17,

[v8-dev] [turbofan] Fix type feedback vector for stub compilation. (issue 1320623004 by mstarzin...@chromium.org)

2015-08-27 Thread mstarzinger
Reviewers: mvstanton, Description: [turbofan] Fix type feedback vector for stub compilation. This makes sure that stubs never gets to see a type feedback vector, which would be an empty fixed array anyways but takes whatever the given CompilationInfo provides. R=mvstan...@chromium.org Please

[v8-dev] Re: Ensure hole checks take place in switch statement scopes (issue 1312613003 by little...@chromium.org)

2015-08-27 Thread mstarzinger
Just pinging on one of my comments so that it doesn't drown in the noise. Still LGTM after that of course. https://codereview.chromium.org/1312613003/diff/60001/src/full-codegen/full-codegen.cc File src/full-codegen/full-codegen.cc (right):

[v8-dev] Re: Remove CompilationInfo::MayUseThis() and replace it with what we really want to know: MustReplaceUn… (issue 1312713004 by tit...@chromium.org)

2015-08-27 Thread mstarzinger
LGTM. https://codereview.chromium.org/1312713004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Re: [Interpreter] Add support for loading literals from the constant pool. (issue 1321663003 by rmcil...@chromium.org)

2015-08-27 Thread mstarzinger
Just replying to comment. Will do review tomorrow. https://codereview.chromium.org/1321663003/diff/40001/test/cctest/interpreter/test-bytecode-generator.cc File test/cctest/interpreter/test-bytecode-generator.cc (right):

[v8-dev] Re: Reduce the number of entrypoints to the compiler pipeline by one. Always require caller to provide … (issue 1317113004 by tit...@chromium.org)

2015-08-27 Thread mstarzinger
LGTM. Love it. https://codereview.chromium.org/1317113004/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving

[v8-dev] Re: [Interpreter] Add support for loading literals from the constant pool. (issue 1321663003 by rmcil...@chromium.org)

2015-08-27 Thread mstarzinger
https://codereview.chromium.org/1321663003/diff/40001/test/cctest/interpreter/test-bytecode-generator.cc File test/cctest/interpreter/test-bytecode-generator.cc (right): https://codereview.chromium.org/1321663003/diff/40001/test/cctest/interpreter/test-bytecode-generator.cc#newcode257

[v8-dev] Re: [es6] Initial steps towards a correct implementation of IsCallable. (issue 1316933002 by bmeu...@chromium.org)

2015-08-27 Thread mstarzinger
LGTM if all the changes to the .js file are working as intended from a specification perspective, didn't check those. https://codereview.chromium.org/1316933002/diff/80001/src/execution.h File src/execution.h (right):

[v8-dev] Re: [heap] Limit friendship of the Heap class to essentials. (issue 1320843002 by mstarzin...@chromium.org)

2015-08-27 Thread mstarzinger
Addressed comments. https://codereview.chromium.org/1320843002/diff/1/src/heap/heap.h File src/heap/heap.h (right): https://codereview.chromium.org/1320843002/diff/1/src/heap/heap.h#newcode1231 src/heap/heap.h:1231: void public_set_code_stub_context(Object* value) { On 2015/08/27 10:11:00,

[v8-dev] Re: [interpreter] Fix gcmole error after r30404. (issue 1319943002 by rmcil...@chromium.org)

2015-08-27 Thread mstarzinger
LGTM. https://codereview.chromium.org/1319943002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] [heap] Limit friendship of the Heap class to essentials. (issue 1320843002 by mstarzin...@chromium.org)

2015-08-27 Thread mstarzinger
Reviewers: Michael Lippautz, Yang, Message: Yang: PTAL at bootstrapper and deserializer. Mike: PTAL. Description: [heap] Limit friendship of the Heap class to essentials. This makes it clear that only components within the heap directory should be friends with the Heap class. The two notable

[v8-dev] Re: [heap] Limit friendship of the Heap class to essentials. (issue 1320843002 by mstarzin...@chromium.org)

2015-08-27 Thread mstarzinger
https://codereview.chromium.org/1320843002/diff/1/src/heap/heap.h File src/heap/heap.h (right): https://codereview.chromium.org/1320843002/diff/1/src/heap/heap.h#newcode1231 src/heap/heap.h:1231: void public_set_code_stub_context(Object* value) { On 2015/08/27 09:25:02, Hannes Payer wrote:

[v8-dev] Re: PPC: Make Simulator respect C stack limits as well. (issue 1309303005 by mbra...@us.ibm.com)

2015-08-27 Thread mstarzinger
LGTM. https://codereview.chromium.org/1309303005/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups v8-dev group. To unsubscribe from this group and stop receiving emails from

[v8-dev] Move runtime helper for ToName conversion onto Object. (issue 1306043003 by mstarzin...@chromium.org)

2015-08-27 Thread mstarzinger
Reviewers: Benedikt Meurer, Description: Move runtime helper for ToName conversion onto Object. R=bmeu...@chromium.org Please review this at https://codereview.chromium.org/1306043003/ Base URL: https://chromium.googlesource.com/v8/v8.git@local_cleanup-heap-friends Affected files (+30,

[v8-dev] Re: Call JS functions via native context instead of js builtins object. (issue 1306993003 by yang...@chromium.org)

2015-08-26 Thread mstarzinger
LGTM if all comments are addressed from my end. Please also wait on Benedikt's sign-off. I like this a lot. Thanks! https://codereview.chromium.org/1306993003/diff/20001/src/bootstrapper.cc File src/bootstrapper.cc (right):

[v8-dev] [turbofan] Fix broken dynamic TDZ check for let and const. (issue 1318693002 by mstarzin...@chromium.org)

2015-08-26 Thread mstarzinger
Reviewers: rossberg, Dan Ehrenberg, Message: Andreas: PTAL. Dan: FYI. Description: [turbofan] Fix broken dynamic TDZ check for let and const. This fixes broken dynamic hole-checks for the temporal dead zone of non-initializing assignments to {let} and {const} declared variables. Also note that

[v8-dev] Remove named load from builtin in default super call. (issue 1314493006 by mstarzin...@chromium.org)

2015-08-26 Thread mstarzinger
Reviewers: Yang, Description: Remove named load from builtin in default super call. R=yang...@chromium.org Please review this at https://codereview.chromium.org/1314493006/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+1, -7 lines): M

[v8-dev] Re: [runtime] Remove the redundant %_IsObject intrinsic. (issue 1313903003 by bmeu...@chromium.org)

2015-08-26 Thread mstarzinger
LGTM. LOL. https://codereview.chromium.org/1313903003/diff/1/test/cctest/compiler/test-run-jscalls.cc File test/cctest/compiler/test-run-jscalls.cc (left): https://codereview.chromium.org/1313903003/diff/1/test/cctest/compiler/test-run-jscalls.cc#oldcode160

[v8-dev] Re: Ensure hole checks take place in switch statement scopes (issue 1312613003 by little...@chromium.org)

2015-08-26 Thread mstarzinger
LGTM on full codegen, just nits. But one concern about scope serialization, adding Andreas for that. https://codereview.chromium.org/1312613003/diff/60001/src/full-codegen/full-codegen.cc File src/full-codegen/full-codegen.cc (right):

[v8-dev] Move runtime healper for JSSet and JSMap onto objects. (issue 1312413002 by mstarzin...@chromium.org)

2015-08-26 Thread mstarzinger
Reviewers: rossberg, Description: Move runtime healper for JSSet and JSMap onto objects. R=rossb...@chromium.org Please review this at https://codereview.chromium.org/1312413002/ Base URL: https://chromium.googlesource.com/v8/v8.git@master Affected files (+42, -49 lines): M src/api.cc M

[v8-dev] Move runtime helper for JSWeakCollection onto objects. (issue 1314053003 by mstarzin...@chromium.org)

2015-08-26 Thread mstarzinger
Reviewers: rossberg, Description: Move runtime helper for JSWeakCollection onto objects. R=rossb...@chromium.org Please review this at https://codereview.chromium.org/1314053003/ Base URL: https://chromium.googlesource.com/v8/v8.git@local_cleanup-runtime-helpers-1 Affected files (+67, -78

  1   2   3   4   5   6   7   8   9   10   >