About testing: Can you make sure that all of the code paths > are hit that
you've added?
The situation is the following: everything has good coverage except for
X87TOSToI and TruncateX87TOSToI, which is atm dead code, therefore not
testable.
It will be extensively used and covered in the coming hydrogen binopStubs.
If
you don't like that, i can rip them out and keep them for the binop cl.
https://codereview.chromium.org/22290005/diff/65011/src/ia32/code-stubs-ia32.cc
File src/ia32/code-stubs-ia32.cc (right):
https://codereview.chromium.org/22290005/diff/65011/src/ia32/code-stubs-ia32.cc#newcode2668
src/ia32/code-stubs-ia32.cc:2668: true, &call_runtime);
On 2013/08/20 16:14:28, danno wrote:
nit: odd indentation
Done.
https://codereview.chromium.org/22290005/diff/65011/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):
https://codereview.chromium.org/22290005/diff/65011/src/ia32/lithium-codegen-ia32.cc#newcode5302
src/ia32/lithium-codegen-ia32.cc:5302: (instr->temp() != NULL) ?
ToDoubleRegister(instr->temp()) : no_xmm_reg,
On 2013/08/20 16:14:28, danno wrote:
nit: factor this out into a variable that is passed in into TaggedToI.
It makes
debugging and readability better.
Done.
https://codereview.chromium.org/22290005/diff/65011/src/ia32/macro-assembler-ia32.cc
File src/ia32/macro-assembler-ia32.cc (right):
https://codereview.chromium.org/22290005/diff/65011/src/ia32/macro-assembler-ia32.cc#newcode407
src/ia32/macro-assembler-ia32.cc:407: j(not_equal,
!treat_minus_zero_as_zero
On 2013/08/20 16:14:28, danno wrote:
nit: Factor this label decision out into a variable that you pass to
this
statement and the next.
Done.
https://codereview.chromium.org/22290005/diff/65011/src/ia32/macro-assembler-ia32.h
File src/ia32/macro-assembler-ia32.h (right):
https://codereview.chromium.org/22290005/diff/65011/src/ia32/macro-assembler-ia32.h#newcode478
src/ia32/macro-assembler-ia32.h:478: int index =
HeapNumber::kValueOffset - kHeapObjectTag);
On 2013/08/20 16:14:28, danno wrote:
nit: this parameter should be called offset, right?
Done.
https://codereview.chromium.org/22290005/diff/65011/src/ia32/macro-assembler-ia32.h#newcode485
src/ia32/macro-assembler-ia32.h:485: XMMRegister scratch, bool
treat_minus_zero_as_zero,
On 2013/08/20 16:14:28, danno wrote:
Can you convert this bool into an enum?
enum MinusZeroMode {
TREAT_MINUS_ZERO_AS_ZERO,
FAIL_ON_MINUS_ZERO
}
Done.
https://codereview.chromium.org/22290005/
--
--
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.
