On 2014/08/19 11:31:19, sigurds wrote:

https://codereview.chromium.org/453833003/diff/40001/src/compiler/js-inlining.cc
File src/compiler/js-inlining.cc (right):


https://codereview.chromium.org/453833003/diff/40001/src/compiler/js-inlining.cc#newcode27
src/compiler/js-inlining.cc:27: static void ReplaceEffectfulWithValue(Node*
node, Node* value) {
On 2014/08/14 10:24:31, titzer wrote:
> Can you find a place for this routine? I think this is the third copy now.
Also,
> you updated the one in context specialization to remove all inputs but not
this
> one.

Done.


https://codereview.chromium.org/453833003/diff/40001/src/compiler/js-inlining.cc#newcode48
src/compiler/js-inlining.cc:48: explicit InlinerVisitor(JSInliner* spec) :
spec_(spec) {}
On 2014/08/14 10:36:22, Michael Starzinger wrote:
> nit: s/spec/inliner/

Done.


https://codereview.chromium.org/453833003/diff/40001/src/compiler/js-inlining.cc#newcode62
src/compiler/js-inlining.cc:62: JSInliner* spec_;
On 2014/08/14 10:36:22, Michael Starzinger wrote:
> nit: s/spec_/inliner_/

Done.


https://codereview.chromium.org/453833003/diff/40001/src/compiler/js-inlining.h
File src/compiler/js-inlining.h (right):


https://codereview.chromium.org/453833003/diff/40001/src/compiler/js-inlining.h#newcode10
src/compiler/js-inlining.h:10: #include "src/contexts.h"
On 2014/08/14 10:36:22, Michael Starzinger wrote:
> nit: Both the graph-reducer.h and the contexts.h include should not be
needed
I
> think.

Done.

https://codereview.chromium.org/453833003/diff/40001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):


https://codereview.chromium.org/453833003/diff/40001/src/compiler/pipeline.cc#newcode199
src/compiler/pipeline.cc:199: SourcePositionTable::Scope
pos_(&source_positions,
On 2014/08/14 10:36:22, Michael Starzinger wrote:
> nit: Let's drop the trialing underscore from "pos" here and a few lines
above
in
> line 190 as well.

Done.

https://codereview.chromium.org/453833003/diff/40001/src/flag-definitions.h
File src/flag-definitions.h (right):


https://codereview.chromium.org/453833003/diff/40001/src/flag-definitions.h#newcode341
src/flag-definitions.h:341: DEFINE_STRING(turbo_inlining_filter, "*",
On 2014/08/14 10:36:22, Michael Starzinger wrote:
> As discussed offline: Let's drop the filter flag again, since it is not
(yet)
> needed and also it is intuitively unclear whether the filter should apply to
the
> surrounding function or the inlinee.

Done.

LGTM

https://codereview.chromium.org/453833003/

--
--
v8-dev mailing list
[email protected]
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, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to