Improve our type feedback by recogizining never-executed IC calls for binary operations.

In the case of inlined smi code in non-optimzied code we could not 
distinguish between the smi-only case and the case that the operation was
never executed.

With this change the first execution of a binary operation always jumps
to the stub which in turn patches the smi-check into the correct
conditional branch, so that we benefit from inlined smi code after the
first invocation.

A nop instruction after the call to the BinaryOpIC indicates that no
smi code was inlined. A "test eax" instruction says that there was smi
code inlined and encodes the delta to the patch site and the condition
code of the branch at the patch site to restore the original jump.

Review URL: http://codereview.chromium.org/5714001

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

src/arm/ic-arm.cc

@@ -2360,10 +2360,8 @@ Condition CompareIC::ComputeCondition(Token::Value op) {
 void CompareIC::UpdateCaches(Handle<Object> x, Handle<Object> y) {
   HandleScope scope;
   Handle<Code> rewritten;
-#ifdef DEBUG
   State previous_state = GetState();
-#endif
-  State state = TargetState(x, y);
+  State state = TargetState(previous_state, x, y);
   if (state == GENERIC) {
     CompareStub stub(GetCondition(), strict(), NO_COMPARE_FLAGS, r1, r0);
     rewritten = stub.GetCode();
@@ -2383,6 +2381,12 @@ void CompareIC::UpdateCaches(Handle<Object> x, Handle<Object> y) {
 #endif
 }
 
+
+void PatchInlinedSmiCode(Address address) {
+  UNIMPLEMENTED();
+}
+
+
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_ARM

src/full-codegen.h

@@ -38,6 +38,9 @@
 namespace v8 {
 namespace internal {
 
+// Forward declarations.
+class JumpPatchSite;
+
 // AST node visitor which can tell whether a given statement will be breakable
 // when the code is compiled by the full compiler in the debugger. This means
 // that there will be an IC (load/store/call) in the code generated for the
@@ -533,6 +536,10 @@ class FullCodeGenerator: public AstVisitor {
   // Helper for calling an IC stub.
   void EmitCallIC(Handle<Code> ic, RelocInfo::Mode mode);
 
+  // Helper for calling an IC stub with a patch site. Passing NULL for patch_site
+  // indicates no inlined smi code and emits a nop after the IC call.
+  void EmitCallIC(Handle<Code> ic, JumpPatchSite* patch_site);
+
   // Set fields in the stack frame. Offsets are the frame pointer relative
   // offsets defined in, e.g., StandardFrameConstants.
   void StoreToFrameField(int frame_offset, Register value);

src/ia32/assembler-ia32.h

@@ -571,6 +571,14 @@ class Assembler : public Malloced {
   static const byte kTestEaxByte = 0xA9;
   // One byte opcode for test al, 0xXX.
   static const byte kTestAlByte = 0xA8;
+  // One byte opcode for nop.
+  static const byte kNopByte = 0x90;
+
+  // One byte opcode for a short unconditional jump.
+  static const byte kJmpShortOpcode = 0xEB;
+  // One byte prefix for a short conditional jump.
+  static const byte kJccShortPrefix = 0x70;
+
 
   // ---------------------------------------------------------------------------
   // Code generation

src/ia32/code-stubs-ia32.cc

@@ -1366,8 +1366,8 @@ void TypeRecordingBinaryOpStub::GenerateSmiCode(MacroAssembler* masm,
   if (op_ == Token::DIV || op_ == Token::MOD) {
     left = eax;
     right = ebx;
-      __ mov(ebx, eax);
-      __ mov(eax, edx);
+    __ mov(ebx, eax);
+    __ mov(eax, edx);
   }
 
 

src/ia32/code-stubs-ia32.h

@@ -231,7 +231,8 @@ class TypeRecordingBinaryOpStub: public CodeStub {
     ASSERT(OpBits::is_valid(Token::NUM_TOKENS));
   }
 
-  TypeRecordingBinaryOpStub(int key,
+  TypeRecordingBinaryOpStub(
+      int key,
       TRBinaryOpIC::TypeInfo operands_type,
       TRBinaryOpIC::TypeInfo result_type = TRBinaryOpIC::UNINITIALIZED)
       : op_(OpBits::decode(key)),
@@ -239,8 +240,7 @@ class TypeRecordingBinaryOpStub: public CodeStub {
         use_sse3_(SSE3Bits::decode(key)),
         operands_type_(operands_type),
         result_type_(result_type),
-        name_(NULL) {
-  }
+        name_(NULL) { }
 
   // Generate code to call the stub with the supplied arguments. This will add
   // code at the call site to prepare arguments either in registers or on the

src/ia32/ic-ia32.cc

@@ -2052,10 +2052,9 @@ Condition CompareIC::ComputeCondition(Token::Value op) {
 void CompareIC::UpdateCaches(Handle<Object> x, Handle<Object> y) {
   HandleScope scope;
   Handle<Code> rewritten;
-#ifdef DEBUG
   State previous_state = GetState();
-#endif
-  State state = TargetState(x, y);
+
+  State state = TargetState(previous_state, x, y);
   if (state == GENERIC) {
     CompareStub stub(GetCondition(), strict(), NO_COMPARE_FLAGS);
     rewritten = stub.GetCode();
@@ -2073,6 +2072,40 @@ void CompareIC::UpdateCaches(Handle<Object> x, Handle<Object> y) {
            Token::Name(op_));
   }
 #endif
+
+  // Activate inlined smi code.
+  if (previous_state == UNINITIALIZED) {
+    PatchInlinedSmiCode(address());
+  }
+}
+
+
+void PatchInlinedSmiCode(Address address) {
+  // The address of the instruction following the call.
+  Address test_instruction_address =
+      address + Assembler::kCallTargetAddressOffset;
+
+  // If the instruction following the call is not a test al, nothing
+  // was inlined.
+  if (*test_instruction_address != Assembler::kTestAlByte) {
+    ASSERT(*test_instruction_address == Assembler::kNopByte);
+    return;
+  }
+
+  Address delta_address = test_instruction_address + 1;
+  // The delta to the start of the map check instruction and the
+  // condition code uses at the patched jump.
+  int8_t delta = *reinterpret_cast<int8_t*>(delta_address);
+  if (FLAG_trace_ic) {
+    PrintF("[  patching ic at %p, test=%p, delta=%d\n",
+           address, test_instruction_address, delta);
+  }
+
+  // Patch with a short conditional jump. There must be an unconditional
+  // short jump at this position.
+  Address jmp_address = test_instruction_address - delta;
+  ASSERT(*jmp_address == Assembler::kJmpShortOpcode);
+  *jmp_address = static_cast<byte>(Assembler::kJccShortPrefix | not_zero);
 }
 
 

src/ia32/lithium-codegen-ia32.cc

@@ -315,6 +315,13 @@ void LCodeGen::CallCode(Handle<Code> code,
     __ call(code, mode);
     RecordSafepoint(&no_pointers, Safepoint::kNoDeoptimizationIndex);
   }
+
+  // Signal that we don't inline smi code before these stubs in the
+  // optimizing code generator.
+  if (code->kind() == Code::TYPE_RECORDING_BINARY_OP_IC ||
+      code->kind() == Code::COMPARE_IC) {
+    __ nop();
+  }
 }
 
 

src/ic.cc

@@ -1951,7 +1951,7 @@ TRBinaryOpIC::State TRBinaryOpIC::ToState(TypeInfo type_info) {
 
 
 TRBinaryOpIC::TypeInfo TRBinaryOpIC::JoinTypes(TRBinaryOpIC::TypeInfo x,
-                                           TRBinaryOpIC::TypeInfo y) {
+                                               TRBinaryOpIC::TypeInfo y) {
   if (x == UNINITIALIZED) return y;
   if (y == UNINITIALIZED) return x;
   if (x == STRING && y == STRING) return STRING;
@@ -2041,6 +2041,11 @@ MaybeObject* TypeRecordingBinaryOp_Patch(Arguments args) {
              TRBinaryOpIC::GetName(result_type),
              Token::Name(op));
     }
+
+    // Activate inlined smi code.
+    if (previous_type == TRBinaryOpIC::UNINITIALIZED) {
+      PatchInlinedSmiCode(ic.address());
+    }
   }
 
   Handle<JSBuiltinsObject> builtins = Top::builtins();
@@ -2127,8 +2132,9 @@ const char* CompareIC::GetStateName(State state) {
 }
 
 
-CompareIC::State CompareIC::TargetState(Handle<Object> x, Handle<Object> y) {
-  State state = GetState();
+CompareIC::State CompareIC::TargetState(State state,
+                                        Handle<Object> x,
+                                        Handle<Object> y) {
   if (state != UNINITIALIZED) return GENERIC;
   if (x->IsSmi() && y->IsSmi()) return SMIS;
   if (x->IsNumber() && y->IsNumber()) return HEAP_NUMBERS;

src/ic.h

@@ -582,7 +582,7 @@ class CompareIC: public IC {
   static const char* GetStateName(State state);
 
  private:
-  State TargetState(Handle<Object> x, Handle<Object> y);
+  State TargetState(State state, Handle<Object> x, Handle<Object> y);
 
   bool strict() const { return op_ == Token::EQ_STRICT; }
   Condition GetCondition() const { return ComputeCondition(op_); }
@@ -591,6 +591,8 @@ class CompareIC: public IC {
   Token::Value op_;
 };
 
+// Helper for TRBinaryOpIC and CompareIC.
+void PatchInlinedSmiCode(Address address);
 
 } }  // namespace v8::internal
 

src/type-info.cc

@@ -142,6 +142,8 @@ TypeInfo TypeFeedbackOracle::CompareType(CompareOperation* expr, Side side) {
   CompareIC::State state = static_cast<CompareIC::State>(code->compare_state());
   switch (state) {
     case CompareIC::UNINITIALIZED:
+       // Uninitialized state means never executed.
+      return unknown;
     case CompareIC::SMIS:
       return TypeInfo::Smi();
     case CompareIC::HEAP_NUMBERS:
@@ -184,6 +186,8 @@ TypeInfo TypeFeedbackOracle::BinaryType(BinaryOperation* expr, Side side) {
 
     switch (type) {
       case TRBinaryOpIC::UNINITIALIZED:
+        // Uninitialized state means never executed.
+        return unknown;
       case TRBinaryOpIC::SMI:
         switch (result_type) {
           case TRBinaryOpIC::UNINITIALIZED:
@@ -224,6 +228,8 @@ TypeInfo TypeFeedbackOracle::SwitchType(CaseClause* clause) {
   CompareIC::State state = static_cast<CompareIC::State>(code->compare_state());
   switch (state) {
     case CompareIC::UNINITIALIZED:
+      // Uninitialized state means never executed.
+      return unknown;
     case CompareIC::SMIS:
       return TypeInfo::Smi();
     case CompareIC::HEAP_NUMBERS: