Should be ready to go...
https://codereview.chromium.org/11659022/diff/50002/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://codereview.chromium.org/11659022/diff/50002/src/hydrogen-instructions.cc#newcode2126
src/hydrogen-instructions.cc:2126: bool HLoadKeyed::CanReturnHole()
const {
On 2013/02/01 13:15:08, Michael Starzinger wrote:
This name is kind of disambiguate, it took me a while to figure out
that it's
used by the use-sites of this value. As discussed offline we should
rename this
to something like "UsesMustHandleHole()".
Done.
https://codereview.chromium.org/11659022/diff/50002/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
https://codereview.chromium.org/11659022/diff/50002/src/hydrogen-instructions.h#newcode5415
src/hydrogen-instructions.h:5415: class HTrapAllocationMemento : public
HTemplateInstruction<2> {
On 2013/02/01 13:15:08, Michael Starzinger wrote:
Looks like this instruction only needs one operand. Can we adapt the
template
parameter?
Done.
https://codereview.chromium.org/11659022/diff/50002/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/11659022/diff/50002/src/hydrogen.cc#newcode641
src/hydrogen.cc:641: true_env_ = env->Copy();
On 2013/02/01 13:15:08, Michael Starzinger wrote:
I think it is safer not to preserve these environments in instance
variables
(see comment about body_env_ below).
Done.
https://codereview.chromium.org/11659022/diff/50002/src/hydrogen.cc#newcode688
src/hydrogen.cc:688: body_env_ = env->Copy();
On 2013/02/01 13:15:08, Michael Starzinger wrote:
Likewise.
Done.
https://codereview.chromium.org/11659022/diff/50002/src/hydrogen.cc#newcode749
src/hydrogen.cc:749: body_env_->Push(increment_);
On 2013/02/01 13:15:08, Michael Starzinger wrote:
The body_env_ might be a stale environment in cases where you have
nested loops
or an IfBuilder nested within a loop. I think it is safer to push the
increment
to the current environment (i.e. HGraphBuilder::environment) and not
preserve
the original body-environment in the LoopBuilder instance.
Done.
https://codereview.chromium.org/11659022/diff/50002/src/hydrogen.cc#newcode988
src/hydrogen.cc:988: new(zone) HStoreNamedField(elements,
map_field_name, map_constant,
On 2013/02/01 13:15:08, Michael Starzinger wrote:
Strictly speaking this store needs to have the kChangesMaps
side-effect. I think
in this case it is safe without that flag, but in general all
HStoreNamedField
instructions that target the map-field should have that side-effect
set. Maybe
this justifies a separate builder function for HStoreNamedField
instructions
that target map-fields.
Done.
https://codereview.chromium.org/11659022/diff/50002/src/hydrogen.cc#newcode1005
src/hydrogen.cc:1005: void HGraphBuilder::BuildCopyElements(HContext*
context,
On 2013/02/01 13:15:08, Michael Starzinger wrote:
Beautifully simple, me gusta!
Done.
https://codereview.chromium.org/11659022/
--
--
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/groups/opt_out.
