Fix triggering of method optimization: increment counter at all IC calls and return as before but a…

runtime/vm/stub_code_ia32.cc

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/globals.h"
 #if defined(TARGET_ARCH_IA32)
 
 #include "vm/assembler.h"
 #include "vm/assembler_macros.h"
 #include "vm/compiler.h"
@@ skipping 1540 matching lines @@
 // - Check if 'num_args' (including receiver) match any IC data group.
 // - Match found -> jump to target.
 // - Match not found -> jump to IC miss.
 // TODO(srdjan): Change IC data to keep class ids as integers not as Smi-s.
 void StubCode::GenerateNArgsCheckInlineCacheStub(Assembler* assembler,
                                                  intptr_t num_args) {
   const Immediate raw_null =
       Immediate(reinterpret_cast<intptr_t>(Object::null()));
 
   __ movl(EBX, FieldAddress(ECX, ICData::function_offset()));
+  Label is_hot;
+  if (FlowGraphCompiler::CanOptimize()) {
+    ASSERT(FLAG_optimization_counter_threshold > 1);
+    // The usage_counter is always less than FLAG_optimization_counter_threshold
+    // except when the function gets optimized.
+    __ cmpl(FieldAddress(EBX, Function::usage_counter_offset()),
+        Immediate(FLAG_optimization_counter_threshold - 1));
+    // Do not increment to equality with threshold, since a counter greater
+    // than threshold denotes a function that was already optimized.
+    // The equality should be reached only at exit of the method
+    // (return instruction).
+    __ j(EQUAL, &is_hot, Assembler::kNearJump);
+    // As long as VM has no OSR do not optimize in the middle of the function

[regis, 2012/07/17 16:08:06]

OSR? On-stack replacement?

[srdjan, 2012/07/17 16:25:06]

Yes.

+    // but only at exit so that we have collected all type feedback before
+    // optimizing.
+  }
   __ incl(FieldAddress(EBX, Function::usage_counter_offset()));
-  if (FlowGraphCompiler::CanOptimize()) {
-    __ cmpl(FieldAddress(EBX, Function::usage_counter_offset()),
-        Immediate(FLAG_optimization_counter_threshold));
-    Label not_yet_hot, already_optimized;
-    __ j(LESS, &not_yet_hot, Assembler::kNearJump);
-    __ j(GREATER, &already_optimized, Assembler::kNearJump);
-    // Create a stub frame as we are pushing some objects on the stack before
-    // calling into the runtime.
-    AssemblerMacros::EnterStubFrame(assembler);
-    __ pushl(ECX);  // Preserve inline cache data object.
-    __ pushl(EDX);  // Preserve arguments array.
-    __ pushl(EBX);  // Argument for runtime: function object.
-    __ CallRuntime(kOptimizeInvokedFunctionRuntimeEntry);

[regis, 2012/07/17 16:08:06]

Is this runtime entry still needed?

[srdjan, 2012/07/17 16:25:06]

Yes, from ReturnInstr::EmitNativeCode.

-    __ popl(EBX);  // Remove argument.
-    __ popl(EDX);  // Restore arguments array.
-    __ popl(ECX);  // Restore inline cache data object.
-    __ LeaveFrame();
-    __ Bind(&not_yet_hot);
-    __ Bind(&already_optimized);
-  }
+  __ Bind(&is_hot);
 
   ASSERT(num_args > 0);
   // Get receiver (first read number of arguments from argument descriptor array
   // and then access the receiver from the stack).
   __ movl(EAX, FieldAddress(EDX, Array::data_offset()));
   __ movl(EAX, Address(ESP, EAX, TIMES_2, 0));  // EAX (argument_count) is Smi.
 
   Label get_class_id_as_smi, ic_miss;
   // ECX: IC data array.
 
@@ skipping 356 matching lines @@
   __ movl(EAX, Address(ESP, 4 * kWordSize));  // Load error object.
   __ movl(EBP, Address(ESP, 3 * kWordSize));  // Load target frame_pointer.
   __ movl(EBX, Address(ESP, 1 * kWordSize));  // Load target PC into EBX.
   __ movl(ESP, Address(ESP, 2 * kWordSize));  // Load target stack_pointer.
   __ jmp(EBX);  // Jump to the exception handler code.
 }
 
 }  // namespace dart
 
 #endif  // defined TARGET_ARCH_IA32