Just a few remarks. Didn't do a thorough review of the details.
https://codereview.chromium.org/573703002/diff/40001/src/compiler/common-operator.h
File src/compiler/common-operator.h (right):
https://codereview.chromium.org/573703002/diff/40001/src/compiler/common-operator.h#newcode35
src/compiler/common-operator.h:35: enum FrameStateType { JS_FRAME,
ARGUMENTS_ADAPTOR };
nit: This needs some comments I think. How about the following?
// The type of stack frame that a FrameState node represents.
enum FrameStateType {
JS_FRAME, // Represents an unoptimized JavaScriptFrame.
ARGUMENTS_ADAPTOR // Represents an ArgumentsAdaptorFrame.
};
https://codereview.chromium.org/573703002/diff/40001/src/compiler/common-operator.h#newcode57
src/compiler/common-operator.h:57: Unique<JSFunction> jsfunction_;
nit: IMHO we should just use Handle<JSFunction> instead of
Unique<JSFunction> here. Not sure what a Unique buys us here.
For posterity only: Also this will not necessarily be a constant
JSFunction in the long run. At some point we want to support inlining of
non-constant functions when we know they have a constant underlying
SharedFunctionInfo. At that point this will become an input operand
instead of a constant parameter. Correct?
https://codereview.chromium.org/573703002/diff/40001/src/compiler/js-inlining.cc
File src/compiler/js-inlining.cc (right):
https://codereview.chromium.org/573703002/diff/40001/src/compiler/js-inlining.cc#newcode311
src/compiler/js-inlining.cc:311: class JSCallFunctionAccessor {
While such a thing would be nice to have, spreading it all over the
place is a slippery slop. In an ideal world such an accessor might even
be generated from a DSL (I know I am dreaming here). I could be
convinced to let this pass if we add a big fat TODO(turbofan) here that
this should be further nailed down and moved into the right place later.
https://codereview.chromium.org/573703002/diff/40001/src/compiler/js-inlining.cc#newcode398
src/compiler/js-inlining.cc:398: info.EnableDeoptimizationSupport();
OMG, we really should try to find a way to do this lazily when we
actually deopt, instead of eagerly throughout the compilation pipeline.
Could we also leave a TODO(turbofan) comment in that regard?
https://codereview.chromium.org/573703002/
--
--
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.
