LGTM, just minor comments.
https://codereview.chromium.org/526953004/diff/1/src/compiler/js-generic-lowering.cc
File src/compiler/js-generic-lowering.cc (right):
https://codereview.chromium.org/526953004/diff/1/src/compiler/js-generic-lowering.cc#newcode314
src/compiler/js-generic-lowering.cc:314: DCHECK(!has_frame_state);
Ouch, this is getting complex, but we can always clean it up later. But
could we preserve the comment about why we add effect and control for
pure nodes?
https://codereview.chromium.org/526953004/diff/1/src/compiler/js-generic-lowering.cc#newcode383
src/compiler/js-generic-lowering.cc:383: Operator* new_op =
common()->Call(desc);
nit: Please keep this consistent with the other ReplaceWithFoo helpers,
let's either hoist the operator into a local variable everywhere or undo
this change here.
https://codereview.chromium.org/526953004/diff/1/src/compiler/verifier.cc
File src/compiler/verifier.cc (right):
https://codereview.chromium.org/526953004/diff/1/src/compiler/verifier.cc#newcode78
src/compiler/verifier.cc:78:
CHECK(NodeProperties::GetFrameStateInput(node)->op()->opcode() ==
nit: There should be Node::opcode() directly IIRC.
https://codereview.chromium.org/526953004/diff/1/src/compiler/verifier.cc#newcode79
src/compiler/verifier.cc:79: IrOpcode::kFrameState);
suggestion: Do you think it makes sense to check that def-use and
use-def chains are present (i.e. Is[DefUse|UseDef]ChainLinkPresent)?
https://codereview.chromium.org/526953004/diff/1/test/cctest/cctest.status
File test/cctest/cctest.status (left):
https://codereview.chromium.org/526953004/diff/1/test/cctest/cctest.status#oldcode105
test/cctest/cctest.status:105: 'test-deoptimization/DeoptimizeCompare':
[PASS, NO_VARIANTS],
Nice!
https://codereview.chromium.org/526953004/
--
--
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.
