LGTM with nits for patch set 2. Thanks for improving this!
Please land this after https://codereview.chromium.org/1228113008/ which
will
require rebasing.
https://codereview.chromium.org/1238143002/diff/20001/src/compiler/js-generic-lowering.cc
File src/compiler/js-generic-lowering.cc (right):
https://codereview.chromium.org/1238143002/diff/20001/src/compiler/js-generic-lowering.cc#newcode343
src/compiler/js-generic-lowering.cc:343: node->ReplaceInput(2,
script_context); // Replace old context.
// Set new context ...
https://codereview.chromium.org/1238143002/diff/20001/src/compiler/js-generic-lowering.cc#newcode344
src/compiler/js-generic-lowering.cc:344: node->RemoveInput(3);
// ... instead of old one.
https://codereview.chromium.org/1238143002/diff/20001/src/compiler/js-generic-lowering.cc#newcode403
src/compiler/js-generic-lowering.cc:403: node->ReplaceInput(3,
script_context); // Replace old context.
// Set new context ...
https://codereview.chromium.org/1238143002/diff/20001/src/compiler/js-generic-lowering.cc#newcode404
src/compiler/js-generic-lowering.cc:404: node->RemoveInput(4);
// ... instead of old one.
https://codereview.chromium.org/1238143002/diff/20001/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):
https://codereview.chromium.org/1238143002/diff/20001/src/ia32/code-stubs-ia32.cc#newcode5106
src/ia32/code-stubs-ia32.cc:5106: Register context_reg = esi;
Same comments as for x64.
https://codereview.chromium.org/1238143002/diff/20001/src/ia32/code-stubs-ia32.cc#newcode5130
src/ia32/code-stubs-ia32.cc:5130: __ Pop(result_reg);
// Pop return address.
https://codereview.chromium.org/1238143002/diff/20001/src/ia32/code-stubs-ia32.cc#newcode5240
src/ia32/code-stubs-ia32.cc:5240: __ Pop(cell_reg);
// Pop return address.
https://codereview.chromium.org/1238143002/diff/20001/src/interface-descriptors.h
File src/interface-descriptors.h (right):
https://codereview.chromium.org/1238143002/diff/20001/src/interface-descriptors.h#newcode446
src/interface-descriptors.h:446: static const Register DepthRegister();
Remove this too.
https://codereview.chromium.org/1238143002/diff/20001/src/runtime/runtime-object.cc
File src/runtime/runtime-object.cc (right):
https://codereview.chromium.org/1238143002/diff/20001/src/runtime/runtime-object.cc#newcode444
src/runtime/runtime-object.cc:444: script_context->set(slot,
*it.GetPropertyCell());
GC mole will complain about this change.
https://codereview.chromium.org/1238143002/diff/20001/src/runtime/runtime-object.cc#newcode475
src/runtime/runtime-object.cc:475: script_context->set(slot,
*it.GetPropertyCell());
GC mole will complain about this change.
https://codereview.chromium.org/1238143002/diff/20001/src/x64/code-stubs-x64.cc
File src/x64/code-stubs-x64.cc (right):
https://codereview.chromium.org/1238143002/diff/20001/src/x64/code-stubs-x64.cc#newcode5041
src/x64/code-stubs-x64.cc:5041: Register context_reg = rsi;
What about using LoadGlobalViaContextDescriptor::SlotRegister() and
friends here?
https://codereview.chromium.org/1238143002/diff/20001/src/x64/code-stubs-x64.cc#newcode5050
src/x64/code-stubs-x64.cc:5050: context_reg = rdi;
Nice!
https://codereview.chromium.org/1238143002/diff/20001/src/x64/code-stubs-x64.cc#newcode5074
src/x64/code-stubs-x64.cc:5074: Register context_reg = rsi;
Same here.
https://codereview.chromium.org/1238143002/diff/20001/src/x64/code-stubs-x64.cc#newcode5131
src/x64/code-stubs-x64.cc:5131: // This can only be true for Constant
and ConstantType cells, because we
We could probably get here for PropertyCellType::kUndefined if we store
an undefined value. I think it's ok just to check that KindField part is
kData.
https://codereview.chromium.org/1238143002/
--
--
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.
