Fix OSR for nested loops.

src/ia32/deoptimizer-ia32.cc

 // Copyright 2012 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 193 matching lines @@
 
   if (FLAG_trace_deopt) {
     PrintF("[forced deoptimization: ");
     function->PrintName();
     PrintF(" / %x]\n", reinterpret_cast<uint32_t>(function));
   }
 }
 
 
 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;
 
+// 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::PatchStackCheckCodeAt(Code* unoptimized_code,
-                                        Address pc_after,
-                                        Code* check_code,
-                                        Code* replacement_code) {
+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());
 
   unoptimized_code->GetHeap()->incremental_marking()->RecordCodeTargetPatch(
       unoptimized_code, call_target_address, replacement_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) {
   ByteArray* translations = data->TranslationByteArray();
   int length = data->DeoptCount();
   for (int i = 0; i < length; i++) {
     if (data->AstId(i) == ast_id) {
       TranslationIterator it(translations,  data->TranslationIndex(i)->value());
       int value = it.Next();
       ASSERT(Translation::BEGIN == static_cast<Translation::Opcode>(value));
       // Read the number of frames.
       value = it.Next();
@@ skipping 613 matching lines @@
   }
   __ bind(&done);
 }
 
 #undef __
 
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_IA32