Thanks for the comments. Rebasing now.
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.
On 2015/07/18 20:52:43, Igor Sheludko wrote:
// Set new context ...
Done.
https://codereview.chromium.org/1238143002/diff/20001/src/compiler/js-generic-lowering.cc#newcode344
src/compiler/js-generic-lowering.cc:344: node->RemoveInput(3);
On 2015/07/18 20:52:43, Igor Sheludko wrote:
// ... instead of old one.
Done.
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.
On 2015/07/18 20:52:43, Igor Sheludko wrote:
// Set new context ...
Done.
https://codereview.chromium.org/1238143002/diff/20001/src/compiler/js-generic-lowering.cc#newcode404
src/compiler/js-generic-lowering.cc:404: node->RemoveInput(4);
On 2015/07/18 20:52:43, Igor Sheludko wrote:
// ... instead of old one.
Done.
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;
On 2015/07/18 20:52:43, Igor Sheludko wrote:
Same comments as for x64.
Acknowledged.
https://codereview.chromium.org/1238143002/diff/20001/src/ia32/code-stubs-ia32.cc#newcode5130
src/ia32/code-stubs-ia32.cc:5130: __ Pop(result_reg);
On 2015/07/18 20:52:43, Igor Sheludko wrote:
// Pop return address.
Done.
https://codereview.chromium.org/1238143002/diff/20001/src/ia32/code-stubs-ia32.cc#newcode5240
src/ia32/code-stubs-ia32.cc:5240: __ Pop(cell_reg);
On 2015/07/18 20:52:43, Igor Sheludko wrote:
// Pop return address.
Done.
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();
On 2015/07/18 20:52:43, Igor Sheludko wrote:
Remove this too.
Done.
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());
On 2015/07/18 20:52:44, Igor Sheludko wrote:
GC mole will complain about this change.
Done.
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());
On 2015/07/18 20:52:44, Igor Sheludko wrote:
GC mole will complain about this change.
Done.
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;
I explicitly changed it to not use those functions because that would
make the register allocation less readable. Now everything is explicit
by just reading the function.
https://codereview.chromium.org/1238143002/diff/20001/src/x64/code-stubs-x64.cc#newcode5050
src/x64/code-stubs-x64.cc:5050: context_reg = rdi;
On 2015/07/18 20:52:44, Igor Sheludko wrote:
Nice!
Acknowledged.
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;
See above.
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
On 2015/07/18 20:52:44, Igor Sheludko wrote:
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.
Acknowledged.
https://codereview.chromium.org/1238143002/diff/20001/src/x64/code-stubs-x64.cc#newcode5169
src/x64/code-stubs-x64.cc:5169: __ j(equal, &fast_case);
On 2015/07/20 09:01:16, Igor Sheludko wrote:
On 2015/07/19 20:29:29, Igor Sheludko wrote:
> I think it is better to write field and RecordWriteField(...,
OMIT_SMI_CHECK)
> here instead of jumping to fast case that does a smi check in
RecordWriteField()
> again.
Even better idea: we could add an explicit smi check before the above
RecordWriteField(), which we can then call as RecordWriteField(...,
OMIT_SMI_CHECK) and jump there from here after storing to the cell.
Done.
https://codereview.chromium.org/1238143002/diff/20001/src/x64/code-stubs-x64.cc#newcode5169
src/x64/code-stubs-x64.cc:5169: __ j(equal, &fast_case);
On 2015/07/19 20:29:29, Igor Sheludko wrote:
I think it is better to write field and RecordWriteField(...,
OMIT_SMI_CHECK)
here instead of jumping to fast case that does a smi check in
RecordWriteField()
again.
Acknowledged.
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.
