LGTM, but there are clearly more opportunities here.

http://codereview.chromium.org/2819012/diff/1/2
File src/x64/codegen-x64.cc (right):

http://codereview.chromium.org/2819012/diff/1/2#newcode10545
src/x64/codegen-x64.cc:10545: GenerateTypeTransition(masm);
Is it safe to assume that r9 is unchanged here?  GenerateTypeTransition
can't cause a GC or overwrite r9?

http://codereview.chromium.org/2819012/diff/1/2#newcode10551
src/x64/codegen-x64.cc:10551: if (static_operands_type_.IsNumber() &&
FLAG_debug_code) {
What's with the indentation here?

http://codereview.chromium.org/2819012/diff/1/2#newcode10567
src/x64/codegen-x64.cc:10567:
FloatingPointHelper::LoadFloatOperands(masm, xmm4, xmm5);
This looks bad, but perhaps Bill is working on this.  First we check
that it's a number then we load the number into the XMM registers.  This
duplicates the Smi checks.  I think the best solution is that the new
CheckNumberOperands should take two optional arguments (default to
no_reg) that say where the number should go.  Or vice versa, the Loader
should take an optional label pointer that tells it to check.

http://codereview.chromium.org/2819012/diff/1/2#newcode10589
src/x64/codegen-x64.cc:10589: // Allocate heap number in new space.
The ARM port is smarter here.  On both the Intel ports we are testing
for sminess before allocating the result.  On ARM we allocate space at
the point where we load the Smi into the fp register, so we don't have
to test again here.

http://codereview.chromium.org/2819012/diff/1/2#newcode10659
src/x64/codegen-x64.cc:10659: FloatingPointHelper::LoadAsIntegers(masm,
&call_runtime);
This should be passed the result of static_operands_type_.IsNumber() and
the heap number register.  It's doing unnecessary checking and it is
loading the map again.

http://codereview.chromium.org/2819012/diff/1/2#newcode10682
src/x64/codegen-x64.cc:10682: if (op_ == Token::SHR) {
How did the old version even work?

http://codereview.chromium.org/2819012/show

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to