Fix OSR for nested loops.

src/ia32/deoptimizer-ia32.cc

Index: src/ia32/deoptimizer-ia32.cc
diff --git a/src/ia32/deoptimizer-ia32.cc b/src/ia32/deoptimizer-ia32.cc
index 8cdcf9965f0e399e6124b019def79fca269cfb01..eebb0f7aa53057be571175e31920f9c35f2428ac 100644
--- a/src/ia32/deoptimizer-ia32.cc
+++ b/src/ia32/deoptimizer-ia32.cc
@@ -211,41 +211,37 @@ void Deoptimizer::DeoptimizeFunctionWithPreparedFunctionList(
 
 
 static const byte kJnsInstruction = 0x79;
-static const byte kJnsOffset = 0x13;
+static const byte kJnsOffset = 0x11;
 static const byte kCallInstruction = 0xe8;
 static const byte kNopByteOne = 0x66;
 static const byte kNopByteTwo = 0x90;
 
-
-void Deoptimizer::PatchStackCheckCodeAt(Code* unoptimized_code,
-                                        Address pc_after,
-                                        Code* check_code,
-                                        Code* replacement_code) {
+// The back edge bookkeeping code matches the pattern:
+//
+//     sub <profiling_counter>, <delta>
+//     jns ok
+//     call <interrupt stub>
+//
+// The patched back edge looks like this:
+//
+//     sub <profiling_counter>, <delta>  ;; Not changed
+//     nop
+//     nop
+//     call <on-stack replacment>
+
+void Deoptimizer::PatchInterruptCodeAt(Code* unoptimized_code,
+                                       Address pc_after,
+                                       Code* interrupt_code,
+                                       Code* replacement_code) {
+  ASSERT(!InterruptCodeIsPatched(unoptimized_code,
+                                 pc_after,
+                                 interrupt_code,
+                                 replacement_code));
+  // Turn the jump into nops.
   Address call_target_address = pc_after - kIntSize;
-  ASSERT_EQ(check_code->entry(),
-            Assembler::target_address_at(call_target_address));
-  // The back edge bookkeeping code matches the pattern:
-  //
-  //     sub <profiling_counter>, <delta>
-  //     jns ok
-  //     call <stack guard>
-  //     test eax, <loop nesting depth>
-  // ok: ...

[Jakob Kummerow, 2013/04/09 17:23:25]

This label got lost when you moved the comment. Please add it back, it is
meaningful.

[Yang, 2013/04/10 08:08:30]

Done.

-  //
-  // We will patch away the branch so the code is:
-  //
-  //     sub <profiling_counter>, <delta>  ;; Not changed
-  //     nop
-  //     nop
-  //     call <on-stack replacment>
-  //     test eax, <loop nesting depth>
-  // ok:
-
-  ASSERT_EQ(kJnsInstruction,  *(call_target_address - 3));

[Jakob Kummerow, 2013/04/09 17:23:25]

I have to say that I preferred this version of the checks, because here it's
trivially easy to convince yourself that exactly the right thing is being
checked.
Happy to discuss this if you prefer the new version :-)

[Yang, 2013/04/10 08:08:30]

As discussed offline, this was refactored to be reused in VerifyInterruptCode.

-  ASSERT_EQ(kJnsOffset,       *(call_target_address - 2));
-  ASSERT_EQ(kCallInstruction, *(call_target_address - 1));
   *(call_target_address - 3) = kNopByteOne;
   *(call_target_address - 2) = kNopByteTwo;
+  // Replace the call address.
   Assembler::set_target_address_at(call_target_address,
                                    replacement_code->entry());
 
@@ -254,27 +250,48 @@ void Deoptimizer::PatchStackCheckCodeAt(Code* unoptimized_code,
 }
 
 
-void Deoptimizer::RevertStackCheckCodeAt(Code* unoptimized_code,
-                                         Address pc_after,
-                                         Code* check_code,
-                                         Code* replacement_code) {
+void Deoptimizer::RevertInterruptCodeAt(Code* unoptimized_code,
+                                        Address pc_after,
+                                        Code* interrupt_code,
+                                        Code* replacement_code) {
+  ASSERT(InterruptCodeIsPatched(unoptimized_code,
+                                pc_after,
+                                interrupt_code,
+                                replacement_code));
+  // Restore the original jump.
   Address call_target_address = pc_after - kIntSize;
-  ASSERT_EQ(replacement_code->entry(),
-            Assembler::target_address_at(call_target_address));
-
-  // Replace the nops from patching (Deoptimizer::PatchStackCheckCode) to
-  // restore the conditional branch.
-  ASSERT_EQ(kNopByteOne,      *(call_target_address - 3));
-  ASSERT_EQ(kNopByteTwo,      *(call_target_address - 2));
-  ASSERT_EQ(kCallInstruction, *(call_target_address - 1));
   *(call_target_address - 3) = kJnsInstruction;
   *(call_target_address - 2) = kJnsOffset;
+  // Restore the original call address.
   Assembler::set_target_address_at(call_target_address,
-                                   check_code->entry());
+                                   interrupt_code->entry());
 
-  check_code->GetHeap()->incremental_marking()->RecordCodeTargetPatch(
-      unoptimized_code, call_target_address, check_code);
+  interrupt_code->GetHeap()->incremental_marking()->RecordCodeTargetPatch(
+      unoptimized_code, call_target_address, interrupt_code);
+}
+
+
+#ifdef DEBUG
+bool Deoptimizer::InterruptCodeIsPatched(Code* unoptimized_code,
+                                         Address pc_after,
+                                         Code* interrupt_code,
+                                         Code* replacement_code) {
+  Address call_target_address = pc_after - kIntSize;
+  ASSERT_EQ(kCallInstruction, *(call_target_address - 1));
+  if (*(call_target_address - 3) == kNopByteOne) {
+    ASSERT_EQ(replacement_code->entry(),
+             Assembler::target_address_at(call_target_address));
+    ASSERT_EQ(kNopByteTwo,      *(call_target_address - 2));
+    return true;
+  } else {
+    ASSERT_EQ(interrupt_code->entry(),
+              Assembler::target_address_at(call_target_address));
+    ASSERT_EQ(kJnsInstruction,  *(call_target_address - 3));
+    ASSERT_EQ(kJnsOffset,       *(call_target_address - 2));
+    return false;
+  }
 }
+#endif  // DEBUG
 
 
 static int LookupBailoutId(DeoptimizationInputData* data, BailoutId ast_id) {