LGTM, I like it.
https://codereview.chromium.org/1244583003/diff/20001/src/compiler/js-context-relaxation.cc
File src/compiler/js-context-relaxation.cc (right):
https://codereview.chromium.org/1244583003/diff/20001/src/compiler/js-context-relaxation.cc#newcode30
src/compiler/js-context-relaxation.cc:30: return NoChange();
Could we just break when we hit a context-changing frame-state? IIUC we
can walk up the frame-state chain as long as the inlined call is
non-context-changing. Once we hit a context-changing call, we can still
walk the context-extension chain in smaller jumps.
https://codereview.chromium.org/1244583003/diff/20001/src/compiler/js-context-relaxation.h
File src/compiler/js-context-relaxation.h (right):
https://codereview.chromium.org/1244583003/diff/20001/src/compiler/js-context-relaxation.h#newcode8
src/compiler/js-context-relaxation.h:8: #include "src/code-factory.h"
nit: The code-factory include shouldn't be necessary (at least not in
the header).
https://codereview.chromium.org/1244583003/diff/20001/src/compiler/js-context-relaxation.h#newcode17
src/compiler/js-context-relaxation.h:17: // builder. This makes it
possible to use they operations with context
nit: s/they/these/
https://codereview.chromium.org/1244583003/diff/20001/src/compiler/js-context-relaxation.h#newcode19
src/compiler/js-context-relaxation.h:19: // be embedded in generated
code thus causing leaks.
nit: ... and potentially using the wrong native context (i.e. stubs are
shared between native contexts).
https://codereview.chromium.org/1244583003/diff/20001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):
https://codereview.chromium.org/1244583003/diff/20001/src/compiler/pipeline.cc#newcode718
src/compiler/pipeline.cc:718: AddReducer(data, &graph_reducer,
&relaxing);
nit: s/relxing/context_relaxing/
https://codereview.chromium.org/1244583003/diff/20001/test/unittests/compiler/js-context-relaxation-unittest.cc
File test/unittests/compiler/js-context-relaxation-unittest.cc (right):
https://codereview.chromium.org/1244583003/diff/20001/test/unittests/compiler/js-context-relaxation-unittest.cc#newcode6
test/unittests/compiler/js-context-relaxation-unittest.cc:6: #include
"src/compiler/diamond.h"
nit: The diamond.h and the access-builder.h include shouldn't be
necessary.
https://codereview.chromium.org/1244583003/
--
--
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.
