Simplify the transitions in the Binary Op ICs. Now a single call

to the runtime will both patch in the more specialized binary op
stub and calculate the answer.  This eliminates the need to call
both the rest of the binary op and the patching runtime call.  The
runtime routines are altered to be more agressive in returning
Smis so we don't get spurious heap numbers as inputs to binary ops
while we are patching the binary op ICs.
Review URL: http://codereview.chromium.org/2843049

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@5026 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/ia32/codegen-ia32.cc

@@ -9866,6 +9866,7 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) {
           // the four basic operations. The stub stays in the DEFAULT state
           // forever for all other operations (also if smi code is skipped).
           GenerateTypeTransition(masm);
+          break;
         }
 
         Label not_floats;
@@ -10213,51 +10214,28 @@ void GenericBinaryOpStub::GenerateRegisterArgsPush(MacroAssembler* masm) {
 
 
 void GenericBinaryOpStub::GenerateTypeTransition(MacroAssembler* masm) {
-  Label get_result;
-
-  // Keep a copy of operands on the stack and make sure they are also in
-  // edx, eax.
+  // Ensure the operands are on the stack.
   if (HasArgsInRegisters()) {
     GenerateRegisterArgsPush(masm);
-  } else {
-    GenerateLoadArguments(masm);
   }
 
-  // Internal frame is necessary to handle exceptions properly.
-  __ EnterInternalFrame();
-
-  // Push arguments on stack if the stub expects them there.
-  if (!HasArgsInRegisters()) {
-    __ push(edx);
-    __ push(eax);
-  }
-  // Call the stub proper to get the result in eax.
-  __ call(&get_result);
-  __ LeaveInternalFrame();
+  __ pop(ecx);  // Save return address.
 
-  __ pop(ecx);  // Return address.
   // Left and right arguments are now on top.
-  // Push the operation result. The tail call to BinaryOp_Patch will
-  // return it to the original caller.
-  __ push(eax);
   // Push this stub's key. Although the operation and the type info are
   // encoded into the key, the encoding is opaque, so push them too.
   __ push(Immediate(Smi::FromInt(MinorKey())));
   __ push(Immediate(Smi::FromInt(op_)));
   __ push(Immediate(Smi::FromInt(runtime_operands_type_)));
 
-  __ push(ecx);  // Return address.
+  __ push(ecx);  // Push return address.
 
-  // Patch the caller to an appropriate specialized stub
-  // and return the operation result.
+  // Patch the caller to an appropriate specialized stub and return the
+  // operation result to the caller of the stub.
   __ TailCallExternalReference(
       ExternalReference(IC_Utility(IC::kBinaryOp_Patch)),
-      6,
+      5,
       1);
-
-  // The entry point for the result calculation is assumed to be immediately
-  // after this sequence.
-  __ bind(&get_result);
 }
 
 

src/ic.cc

@@ -1639,16 +1639,15 @@ Handle<Code> GetBinaryOpStub(int key, BinaryOpIC::TypeInfo type_info);
 
 
 Object* BinaryOp_Patch(Arguments args) {
-  ASSERT(args.length() == 6);
+  ASSERT(args.length() == 5);
 
   Handle<Object> left = args.at<Object>(0);
   Handle<Object> right = args.at<Object>(1);
-  Handle<Object> result = args.at<Object>(2);
-  int key = Smi::cast(args[3])->value();
+  int key = Smi::cast(args[2])->value();
+  Token::Value op = static_cast<Token::Value>(Smi::cast(args[3])->value());
 #ifdef DEBUG
-  Token::Value op = static_cast<Token::Value>(Smi::cast(args[4])->value());
   BinaryOpIC::TypeInfo prev_type_info =
-      static_cast<BinaryOpIC::TypeInfo>(Smi::cast(args[5])->value());
+      static_cast<BinaryOpIC::TypeInfo>(Smi::cast(args[4])->value());
 #endif  // DEBUG
   { HandleScope scope;
     BinaryOpIC::TypeInfo type_info = BinaryOpIC::GetTypeInfo(*left, *right);
@@ -1667,6 +1666,61 @@ Object* BinaryOp_Patch(Arguments args) {
     }
   }
 
+  HandleScope scope;
+  Handle<JSBuiltinsObject> builtins = Top::builtins();
+
+  Object* builtin = NULL;  // Initialization calms down the compiler.
+
+  switch (op) {
+    case Token::ADD:
+      builtin = builtins->javascript_builtin(Builtins::ADD);
+      break;
+    case Token::SUB:
+      builtin = builtins->javascript_builtin(Builtins::SUB);
+      break;
+    case Token::MUL:
+      builtin = builtins->javascript_builtin(Builtins::MUL);
+      break;
+    case Token::DIV:
+      builtin = builtins->javascript_builtin(Builtins::DIV);
+      break;
+    case Token::MOD:
+      builtin = builtins->javascript_builtin(Builtins::MOD);
+      break;
+    case Token::BIT_AND:
+      builtin = builtins->javascript_builtin(Builtins::BIT_AND);
+      break;
+    case Token::BIT_OR:
+      builtin = builtins->javascript_builtin(Builtins::BIT_OR);
+      break;
+    case Token::BIT_XOR:
+      builtin = builtins->javascript_builtin(Builtins::BIT_XOR);
+      break;
+    case Token::SHR:
+      builtin = builtins->javascript_builtin(Builtins::SHR);
+      break;
+    case Token::SAR:
+      builtin = builtins->javascript_builtin(Builtins::SAR);
+      break;
+    case Token::SHL:
+      builtin = builtins->javascript_builtin(Builtins::SHL);
+      break;
+    default:
+      UNREACHABLE();
+  }
+
+  Handle<JSFunction> builtin_function(JSFunction::cast(builtin));
+
+  bool caught_exception;
+  Object** builtin_args[] = { right.location() };
+  Handle<Object> result = Execution::Call(builtin_function,
+                                          left,
+                                          ARRAY_SIZE(builtin_args),
+                                          builtin_args,
+                                          &caught_exception);
+  if (caught_exception) {
+    return Failure::Exception();
+  }
   return *result;
 }
 

src/runtime.cc

@@ -5575,7 +5575,7 @@ static Object* Runtime_NumberAdd(Arguments args) {
 
   CONVERT_DOUBLE_CHECKED(x, args[0]);
   CONVERT_DOUBLE_CHECKED(y, args[1]);
-  return Heap::AllocateHeapNumber(x + y);
+  return Heap::NumberFromDouble(x + y);
 }
 
 
@@ -5585,7 +5585,7 @@ static Object* Runtime_NumberSub(Arguments args) {
 
   CONVERT_DOUBLE_CHECKED(x, args[0]);
   CONVERT_DOUBLE_CHECKED(y, args[1]);
-  return Heap::AllocateHeapNumber(x - y);
+  return Heap::NumberFromDouble(x - y);
 }
 
 
@@ -5595,7 +5595,7 @@ static Object* Runtime_NumberMul(Arguments args) {
 
   CONVERT_DOUBLE_CHECKED(x, args[0]);
   CONVERT_DOUBLE_CHECKED(y, args[1]);
-  return Heap::AllocateHeapNumber(x * y);
+  return Heap::NumberFromDouble(x * y);
 }
 
 
@@ -5604,7 +5604,7 @@ static Object* Runtime_NumberUnaryMinus(Arguments args) {
   ASSERT(args.length() == 1);
 
   CONVERT_DOUBLE_CHECKED(x, args[0]);
-  return Heap::AllocateHeapNumber(-x);
+  return Heap::NumberFromDouble(-x);
 }
 
 
@@ -6206,7 +6206,7 @@ static Object* Runtime_Math_pow(Arguments args) {
   // custom powi() function than the generic pow().
   if (args[1]->IsSmi()) {
     int y = Smi::cast(args[1])->value();
-    return Heap::AllocateHeapNumber(powi(x, y));
+    return Heap::NumberFromDouble(powi(x, y));
   }
 
   CONVERT_DOUBLE_CHECKED(y, args[1]);

src/x64/codegen-x64.cc

@@ -10611,6 +10611,7 @@ void GenericBinaryOpStub::Generate(MacroAssembler* masm) {
           // the four basic operations. The stub stays in the DEFAULT state
           // forever for all other operations (also if smi code is skipped).
           GenerateTypeTransition(masm);
+          break;
         }
 
         Label not_floats;
@@ -10928,31 +10929,13 @@ void GenericBinaryOpStub::GenerateRegisterArgsPush(MacroAssembler* masm) {
 void GenericBinaryOpStub::GenerateTypeTransition(MacroAssembler* masm) {
   Label get_result;
 
-  // Keep a copy of operands on the stack and make sure they are also in
-  // rdx, rax.
+  // Ensure the operands are on the stack.
   if (HasArgsInRegisters()) {
     GenerateRegisterArgsPush(masm);
-  } else {
-    GenerateLoadArguments(masm);
-  }
-
-  // Internal frame is necessary to handle exceptions properly.
-  __ EnterInternalFrame();
-
-  // Push arguments on stack if the stub expects them there.
-  if (!HasArgsInRegisters()) {
-    __ push(rdx);
-    __ push(rax);
   }
-  // Call the stub proper to get the result in rax.
-  __ call(&get_result);
-  __ LeaveInternalFrame();
 
   // Left and right arguments are already on stack.
-  __ pop(rcx);
-  // Push the operation result. The tail call to BinaryOp_Patch will
-  // return it to the original caller..
-  __ push(rax);
+  __ pop(rcx);  // Save the return address.
 
   // Push this stub's key.
   __ Push(Smi::FromInt(MinorKey()));
@@ -10963,17 +10946,13 @@ void GenericBinaryOpStub::GenerateTypeTransition(MacroAssembler* masm) {
 
   __ Push(Smi::FromInt(runtime_operands_type_));
 
-  __ push(rcx);
+  __ push(rcx);  // The return address.
 
   // Perform patching to an appropriate fast case and return the result.
   __ TailCallExternalReference(
       ExternalReference(IC_Utility(IC::kBinaryOp_Patch)),
-      6,
+      5,
       1);
-
-  // The entry point for the result calculation is assumed to be immediately
-  // after this sequence.
-  __ bind(&get_result);
 }
 
 

test/mjsunit/math-min-max.js

@@ -42,7 +42,16 @@ assertEquals(1.1, Math.min(2.2, 3.3, 1.1));
 
 // Prepare a non-Smi zero value.
 function returnsNonSmi(){ return 0.25; }
-var ZERO = returnsNonSmi() - returnsNonSmi();
+var ZERO = (function() {
+  var z;
+  // We have to have a loop here because the first time we get a Smi from the
+  // runtime system.  After a while the binary op IC settles down and we get
+  // a non-Smi from the generated code.
+  for (var i = 0; i < 10; i++) {
+    z = returnsNonSmi() - returnsNonSmi();
+  }
+  return z;
+})();
 assertEquals(0, ZERO);
 assertEquals(Infinity, 1/ZERO);
 assertEquals(-Infinity, 1/-ZERO);

test/mjsunit/value-of.js

+// Copyright 2010 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
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+function MyException() { }
+
+var o = new Object();
+o.valueOf = function() { throw new MyException(); }
+
+assertThrows(function() { o + 1 }, MyException);