 Chromium Code Reviews
 Chromium Code Reviews Issue 22935005:
  Refine CountOperation of FullCodeGen  (Closed) 
  Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
    
  
    Issue 22935005:
  Refine CountOperation of FullCodeGen  (Closed) 
  Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge| Index: src/x64/full-codegen-x64.cc | 
| diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc | 
| index 20c56226e8cca27078e518d32fb357ecd1e5b49b..734604e99efa99f6e9c70363eecb5ee53c25f325 100644 | 
| --- a/src/x64/full-codegen-x64.cc | 
| +++ b/src/x64/full-codegen-x64.cc | 
| @@ -4387,14 +4387,51 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) { | 
| PrepareForBailoutForId(prop->LoadId(), TOS_REG); | 
| } | 
| - // Call ToNumber only if operand is not a smi. | 
| - Label no_conversion; | 
| + // Inline smi case if we are in a loop. | 
| + Label done, stub_call; | 
| + JumpPatchSite patch_site(masm_); | 
| if (ShouldInlineSmiCase(expr->op())) { | 
| - __ JumpIfSmi(rax, &no_conversion, Label::kNear); | 
| + Label slow; | 
| + patch_site.EmitJumpIfNotSmi(rax, &slow, Label::kNear); | 
| + | 
| + // Save result for postfix expressions. | 
| + if (expr->is_postfix()) { | 
| + if (!context()->IsEffect()) { | 
| + // Save the result on the stack. If we have a named or keyed property | 
| + // we store the result under the receiver that is currently on top | 
| + // of the stack. | 
| + switch (assign_type) { | 
| + case VARIABLE: | 
| + __ push(rax); | 
| + break; | 
| + case NAMED_PROPERTY: | 
| + __ movq(Operand(rsp, kPointerSize), rax); | 
| + break; | 
| + case KEYED_PROPERTY: | 
| + __ movq(Operand(rsp, 2 * kPointerSize), rax); | 
| + break; | 
| + } | 
| + } | 
| + } | 
| + | 
| + if (expr->op() == Token::INC) { | 
| + __ SmiAddConstant(rax, rax, Smi::FromInt(1)); | 
| + __ j(no_overflow, &done, Label::kNear); | 
| + // Call stub. Undo operation first. | 
| + if (expr->op() == Token::INC) { | 
| + __ SmiSubConstant(rax, rax, Smi::FromInt(1)); | 
| + } | 
| + } else { | 
| + SmiOperationExecutionMode mode( | 
| + (1 << NEED_RESERVE_SOURCES) | (1 << BAILOUT_ON_NO_OVERFLOW)); | 
| 
danno
2013/10/14 18:30:45
I think this is clearer as:
SmiOperationExecution
 
haitao.feng
2013/10/16 09:17:01
Done.
 | 
| + __ SmiSubConstant(rax, rax, Smi::FromInt(1), mode, &done, Label::kNear); | 
| + } | 
| + __ jmp(&stub_call, Label::kNear); | 
| + __ bind(&slow); | 
| } | 
| + | 
| ToNumberStub convert_stub; | 
| __ CallStub(&convert_stub); | 
| - __ bind(&no_conversion); | 
| // Save result for postfix expressions. | 
| if (expr->is_postfix()) { | 
| @@ -4416,34 +4453,11 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) { | 
| } | 
| } | 
| - // Inline smi case if we are in a loop. | 
| - Label done, stub_call; | 
| - JumpPatchSite patch_site(masm_); | 
| - | 
| - if (ShouldInlineSmiCase(expr->op())) { | 
| - if (expr->op() == Token::INC) { | 
| - __ SmiAddConstant(rax, rax, Smi::FromInt(1)); | 
| - } else { | 
| - __ SmiSubConstant(rax, rax, Smi::FromInt(1)); | 
| - } | 
| - __ j(overflow, &stub_call, Label::kNear); | 
| - // We could eliminate this smi check if we split the code at | 
| - // the first smi check before calling ToNumber. | 
| - patch_site.EmitJumpIfSmi(rax, &done, Label::kNear); | 
| - | 
| - __ bind(&stub_call); | 
| - // Call stub. Undo operation first. | 
| - if (expr->op() == Token::INC) { | 
| - __ SmiSubConstant(rax, rax, Smi::FromInt(1)); | 
| - } else { | 
| - __ SmiAddConstant(rax, rax, Smi::FromInt(1)); | 
| - } | 
| - } | 
| - | 
| // Record position before stub call. | 
| SetSourcePosition(expr->position()); | 
| // Call stub for +1/-1. | 
| + __ bind(&stub_call); | 
| __ movq(rdx, rax); | 
| __ Move(rax, Smi::FromInt(1)); | 
| BinaryOpStub stub(expr->binary_op(), NO_OVERWRITE); |