X64: Convert HeapNumbers that contain valid smi values to smis in binop-stub.

src/x64/code-stubs-x64.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 1087 matching lines @@
                overwrite_name,
                TRBinaryOpIC::GetName(operands_type_));
   return name_;
 }
 
 
 void TypeRecordingBinaryOpStub::GenerateSmiCode(MacroAssembler* masm,
     Label* slow,
     SmiCodeGenerateHeapNumberResults allow_heapnumber_results) {
 
+  // Arguments to TypeRecordingBinaryOpStub are in rdx and rax.
+  Register left = rdx;
+  Register right = rax;
+
   // We only generate heapnumber answers for overflowing calculations
-  // for the four basic arithmetic operations.
+  // for the four basic arithmetic operations and logical right shift by 0.
   bool generate_inline_heapnumber_results =
       (allow_heapnumber_results == ALLOW_HEAPNUMBER_RESULTS) &&
       (op_ == Token::ADD || op_ == Token::SUB ||
        op_ == Token::MUL || op_ == Token::DIV || op_ == Token::SHR);
 
-  // Arguments to TypeRecordingBinaryOpStub are in rdx and rax.
-  Register left = rdx;
-  Register right = rax;
-
-
   // Smi check of both operands.  If op is BIT_OR, the check is delayed
   // until after the OR operation.
   Label not_smis;
   Label use_fp_on_smis;
-  Label restore_MOD_registers;  // Only used if op_ == Token::MOD.
+  Label fail;
 
-  if (op_ != Token::BIT_OR) {
-    Comment smi_check_comment(masm, "-- Smi check arguments");
-    __ JumpIfNotBothSmi(left, right, &not_smis);
-  }
+  Comment smi_check_comment(masm, "-- Smi check arguments");
+  __ JumpIfNotBothSmi(left, right, &not_smis);
 
+  Label smi_values;
+  __ bind(&smi_values);
   // Perform the operation.
   Comment perform_smi(masm, "-- Perform smi operation");
   switch (op_) {
     case Token::ADD:
       ASSERT(right.is(rax));
       __ SmiAdd(right, right, left, &use_fp_on_smis);  // ADD is commutative.
       break;
 
     case Token::SUB:
       __ SmiSub(left, left, right, &use_fp_on_smis);
@@ skipping 18 matching lines @@
       // SmiMod will not accept left in rdx or right in rax.
       left = rcx;
       right = rbx;
       __ movq(rbx, rax);
       __ movq(rcx, rdx);
       __ SmiMod(rax, left, right, &use_fp_on_smis);
       break;
 
     case Token::BIT_OR: {
       ASSERT(right.is(rax));
-      __ movq(rcx, right);  // Save the right operand.
       __ SmiOr(right, right, left);  // BIT_OR is commutative.

[William Hesse, 2011/04/08 09:27:58]

Change to SmiOrAndJumpIfNotBothSmi(right, right, left, rcx), and leave the
functionality as is. The second time you reach this (after LoadUnknownsAsSmis),
you know that it will succeed, so no infinite loop.

-      __ JumpIfNotSmi(right, &not_smis);  // Test delayed until after BIT_OR.
       break;
       }
     case Token::BIT_XOR:
       ASSERT(right.is(rax));
       __ SmiXor(right, right, left);  // BIT_XOR is commutative.
       break;
 
     case Token::BIT_AND:
       ASSERT(right.is(rax));
       __ SmiAnd(right, right, left);  // BIT_AND is commutative.
@@ skipping 44 matching lines @@
         case Token::ADD: __ addsd(xmm0, xmm1); break;
         case Token::SUB: __ subsd(xmm0, xmm1); break;
         case Token::MUL: __ mulsd(xmm0, xmm1); break;
         case Token::DIV: __ divsd(xmm0, xmm1); break;
         default: UNREACHABLE();
         }
       }
       __ movsd(FieldOperand(rcx, HeapNumber::kValueOffset), xmm0);
       __ movq(rax, rcx);
       __ ret(0);
+    } else {
+      __ jmp(&fail);
     }
   }
 
   // 7. Non-smi operands reach the end of the code generated by
   //    GenerateSmiCode, and fall through to subsequent code,
   //    with the operands in rdx and rax.
+  //    But first we check if non-smi values are HeapNumbers holding
+  //    values that could be smi.
   Comment done_comment(masm, "-- Enter non-smi code");
   __ bind(&not_smis);
-  if (op_ == Token::BIT_OR) {
-    __ movq(right, rcx);
+  {  // See if there were smi values stored in HeapNumbers.

[William Hesse, 2011/04/08 09:27:58]

I would use LoadAsIntegers (really LoadUnknownsAsIntegers), followed by
SmiTag(), for simplicity's sake.

If you really want to avoid the extra smi-tagging, then I would write the
LoadUnknownsAsSmis function, and call that.  But this code is not that fast
anyway - at most you could make a ToDo for this optimization.

[Lasse Reichstein, 2011/04/08 11:18:10]

LoadNumberAsIntegers will do truncating conversion as ToInteger32 (it's used
before bit-ops). It doesn't check whether the value is a valid smi before
truncating.
We don't want that here - if the value isn't something that could be a smi, we
should fail and go to the type transition.
There is no bailout in the code that allows us to do this.

+    __ LoadRoot(rcx, Heap::kHeapNumberMapRootIndex);
+    NearLabel left_smi, check_right;
+    __ JumpIfSmi(left, &left_smi);
+    __ cmpq(FieldOperand(left, HeapObject::kMapOffset), rcx);
+    __ j(not_equal, &fail);
+    // Convert HeapNumber to smi if possible.
+    __ movsd(xmm0, FieldOperand(left, HeapNumber::kValueOffset));
+    __ movq(rbx, xmm0);
+    __ cvttsd2siq(rdi, xmm0);
+    // Check if conversion was successful by converting back and
+    // comparing to the original double's bits.
+    __ cvtlsi2sd(xmm1, rdi);
+    __ movq(kScratchRegister, xmm1);
+    __ cmpq(rbx, kScratchRegister);
+    __ j(not_equal, &fail);
+    __ Integer32ToSmi(left, rdi);
+
+    __ bind(&check_right);
+    __ JumpIfSmi(right, &smi_values);
+    __ bind(&left_smi);
+    if (FLAG_debug_code) {
+      // One of left or right should be non-smi if we get here.
+      __ AbortIfSmi(right);
+    }
+    __ cmpq(FieldOperand(right, HeapObject::kMapOffset), rcx);
+    __ j(not_equal, &fail);
+    // Convert right to smi, if possible.
+    __ movsd(xmm0, FieldOperand(right, HeapNumber::kValueOffset));
+    __ movq(rbx, xmm0);
+    __ cvttsd2siq(rdi, xmm0);
+    __ cvtlsi2sd(xmm1, rdi);
+    __ movq(kScratchRegister, xmm1);
+    __ cmpq(rbx, kScratchRegister);
+    __ j(not_equal, &fail);
+    __ Integer32ToSmi(right, rdi);
+    __ jmp(&smi_values);
   }
+  __ bind(&fail);
 }
 
 
 void TypeRecordingBinaryOpStub::GenerateFloatingPointCode(
     MacroAssembler* masm,
     Label* allocation_failure,
     Label* non_numeric_failure) {
   switch (op_) {
     case Token::ADD:
     case Token::SUB:
@@ skipping 3885 matching lines @@
   // Do a tail call to the rewritten stub.
   __ jmp(rdi);
 }
 
 
 #undef __
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_X64