Looks almost good from my side, but I am sure mstarzinger@ will have bunch
of
comments.
https://codereview.chromium.org/573703002/diff/20001/src/compiler/code-generator.cc
File src/compiler/code-generator.cc (right):
https://codereview.chromium.org/573703002/diff/20001/src/compiler/code-generator.cc#newcode330
src/compiler/code-generator.cc:330:
descriptor->outer_state()->GetTotalSize() + i));
How about adding descriptor->outer_state()->GetTotalSize() to
frame_state_offset outside the loop?
https://codereview.chromium.org/573703002/diff/20001/src/compiler/instruction-selector.cc
File src/compiler/instruction-selector.cc (right):
https://codereview.chromium.org/573703002/diff/20001/src/compiler/instruction-selector.cc#newcode1043
src/compiler/instruction-selector.cc:1043: if (descriptor->type() ==
JS_FRAME) {
How about introducing FrameStateDescriptor::HasContext method?
https://codereview.chromium.org/573703002/diff/20001/src/compiler/js-inlining.cc
File src/compiler/js-inlining.cc (right):
https://codereview.chromium.org/573703002/diff/20001/src/compiler/js-inlining.cc#newcode439
src/compiler/js-inlining.cc:439: for (Node* node : visitor.copies()) {
Are we allowed to use C++11? I think not yet.
https://codereview.chromium.org/573703002/diff/20001/src/compiler/node-properties-inl.h
File src/compiler/node-properties-inl.h (right):
https://codereview.chromium.org/573703002/diff/20001/src/compiler/node-properties-inl.h#newcode166
src/compiler/node-properties-inl.h:166: Node* frame_state) {
Perhaps an assert here that the node indeed has frame state input?
https://codereview.chromium.org/573703002/diff/20001/src/frames.cc
File src/frames.cc (right):
https://codereview.chromium.org/573703002/diff/20001/src/frames.cc#newcode937
src/frames.cc:937: if (LookupCode()->is_turbofanned() &&
!FLAG_turbo_deoptimization) {
This is a bold move. We should try to run with run-tests
--extra-flags="--turbo-deoptimization" and see what breaks with this.
Do not the GetInlineCount and GetFunctions methods below need similar
treatment?
https://codereview.chromium.org/573703002/diff/20001/test/cctest/compiler/test-run-inlining.cc
File test/cctest/compiler/test-run-inlining.cc (right):
https://codereview.chromium.org/573703002/diff/20001/test/cctest/compiler/test-run-inlining.cc#newcode95
test/cctest/compiler/test-run-inlining.cc:95: "= 12;"
This is a funky line break. Could you make it a bit more readable?
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.
