I think I have addressed everything...
https://codereview.chromium.org/12208013/diff/1/src/hydrogen-instructions.cc
File src/hydrogen-instructions.cc (right):
https://codereview.chromium.org/12208013/diff/1/src/hydrogen-instructions.cc#newcode818
src/hydrogen-instructions.cc:818: HBoundsCheck* HBoundsCheck::AddAfter(
On 2013/02/06 13:43:59, Jakob wrote:
I don't see this used anywhere.
Removed.
https://codereview.chromium.org/12208013/diff/1/src/hydrogen-instructions.cc#newcode842
src/hydrogen-instructions.cc:842: HDeoptimizeIfTaggedIsNotSmi*
checked_length =
On 2013/02/06 13:43:59, Jakob wrote:
Didn't we agree that HBoundsCheck is only ever used for arrays with
FAST
elements, and those always have a Smi as their length? So I don't
think we need
this check.
Done.
https://codereview.chromium.org/12208013/diff/1/src/hydrogen-instructions.cc#newcode2973
src/hydrogen-instructions.cc:2973: HInstruction::Verify();
On 2013/02/06 13:43:59, Jakob wrote:
This is the default implementation, no need to override it.
The linker complains if I don't override it (and implement it).
https://codereview.chromium.org/12208013/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
https://codereview.chromium.org/12208013/diff/1/src/hydrogen-instructions.h#newcode2616
src/hydrogen-instructions.h:2616: class HDeoptimizeIfTaggedIsNotSmi:
public HUnaryOperation {
On 2013/02/06 13:43:59, Jakob wrote:
For consistency with HCheckSmi, I'd call this HCheckSmiOrInt32.
Done.
https://codereview.chromium.org/12208013/diff/1/src/hydrogen-instructions.h#newcode2618
src/hydrogen-instructions.h:2618: explicit HDeoptimizeIfTaggedIsNotSmi(
On 2013/02/06 13:43:59, Jakob wrote:
no "explicit" necessary (unless you remove the Representation
argument, see
below).
Done.
https://codereview.chromium.org/12208013/diff/1/src/hydrogen-instructions.h#newcode2626
src/hydrogen-instructions.h:2626: } else {
On 2013/02/06 13:43:59, Jakob wrote:
I don't think we need this case. This instruction doesn't do anything
when the
representation is Integer32, and in the code stubs case it is always
Integer32.
So we can simply not insert it at all in that case.
Done.
https://codereview.chromium.org/12208013/diff/1/src/hydrogen-instructions.h#newcode3128
src/hydrogen-instructions.h:3128: class HGraphBuilder;
On 2013/02/06 13:43:59, Jakob wrote:
Please don't do this. Instead, move the insertion helper function to
the graph,
similar to HGraphBuilder::AddSimulate or
HOptimizedGraphBuilder::AddSoftDeoptimize(). (If we need the helper at
all --
see my other comments.)
Done.
https://codereview.chromium.org/12208013/diff/1/src/hydrogen-instructions.h#newcode3165
src/hydrogen-instructions.h:3165: : key_mode_(key_mode),
skip_check_(false) {
On 2013/02/06 13:43:59, Jakob wrote:
nit: more indentation
Done.
https://codereview.chromium.org/12208013/diff/1/src/ia32/lithium-ia32.h
File src/ia32/lithium-ia32.h (right):
https://codereview.chromium.org/12208013/diff/1/src/ia32/lithium-ia32.h#newcode2279
src/ia32/lithium-ia32.h:2279: class LDeoptimizeIfTaggedIsNotSmi: public
LTemplateInstruction<0, 1, 0> {
On 2013/02/06 13:43:59, Jakob wrote:
I think we don't need a new LInstruction.
Instead, I think the "representation.IsTagged() && !type.IsSmi()"
logic should
go into the Canonicalization phase, i.e. the
HDeoptimizeIfTaggedIsNotSmi should
just remove itself when it is not needed. When it is needed, we can
simply
translate it to an LCheckSmi. Or am I missing something?
Done.
https://codereview.chromium.org/12208013/
--
--
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.
