Make HDeoptimize to explicitly use environment values.

Otherwise dead phi elimination can actually remove some of the implicitly used phis.

BUG=1257
TEST=test/mjsunit/regress/regress-1257.js

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

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

diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index 775177a5ac62c23c420410d2184b56ad47f0c212..fce52e8ed8ef2a12b71d4833bc587cb55fa320c0 100644 (file)
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -758,15 +758,33 @@ class HBlockEntry: public HTemplateInstruction<0> {
 };
 
 
-class HDeoptimize: public HTemplateControlInstruction<0> {
+class HDeoptimize: public HControlInstruction {
  public:
-  HDeoptimize() : HTemplateControlInstruction<0>(NULL, NULL) { }
+  HDeoptimize(int environment_length)
+      : HControlInstruction(NULL, NULL),
+        values_(environment_length) { }
 
   virtual Representation RequiredInputRepresentation(int index) const {
     return Representation::None();
   }
 
+  virtual int OperandCount() { return values_.length(); }
+  virtual HValue* OperandAt(int index) { return values_[index]; }
+
+  void AddEnvironmentValue(HValue* value) {
+    values_.Add(NULL);
+    SetOperandAt(values_.length() - 1, value);
+  }
+
   DECLARE_CONCRETE_INSTRUCTION(Deoptimize, "deoptimize")
+
+ protected:
+  virtual void InternalSetOperandAt(int index, HValue* value) {
+    values_[index] = value;
+  }
+
+ private:
+  ZoneList<HValue*> values_;
 };
 
 

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 77714f66da842ced5572da82e5485abcc350b0f6..513d4a8501b9210ab23ae2edd4b377cc1fcb8188 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -113,6 +113,21 @@ void HBasicBlock::AddInstruction(HInstruction* instr) {
 }
 
 
+HDeoptimize* HBasicBlock::CreateDeoptimize() {
+  ASSERT(HasEnvironment());
+  HEnvironment* environment = last_environment();
+
+  HDeoptimize* instr = new HDeoptimize(environment->length());
+
+  for (int i = 0; i < environment->length(); i++) {
+    HValue* val = environment->values()->at(i);
+    instr->AddEnvironmentValue(val);
+  }
+
+  return instr;
+}
+
+
 HSimulate* HBasicBlock::CreateSimulate(int id) {
   ASSERT(HasEnvironment());
   HEnvironment* environment = last_environment();
@@ -2483,6 +2498,7 @@ void HGraphBuilder::VisitSwitchStatement(SwitchStatement* stmt) {
   }
 
   VISIT_FOR_VALUE(stmt->tag());
+  AddSimulate(stmt->EntryId());
   HValue* tag_value = Pop();
   HBasicBlock* first_test_block = current_block();
 
@@ -2498,15 +2514,7 @@ void HGraphBuilder::VisitSwitchStatement(SwitchStatement* stmt) {
     // Unconditionally deoptimize on the first non-smi compare.
     clause->RecordTypeFeedback(oracle());
     if (!clause->IsSmiCompare()) {
-      if (current_block() == first_test_block) {
-        // If the first test is the one that deopts and if the tag value is
-        // a phi, we need to have some use of that phi to prevent phi
-        // elimination from removing it.  This HSimulate is such a use.
-        Push(tag_value);
-        AddSimulate(stmt->EntryId());
-        Drop(1);
-      }
-      current_block()->Finish(new HDeoptimize());
+      current_block()->FinishExitWithDeoptimization();
       set_current_block(NULL);
       break;
     }
@@ -3130,7 +3138,7 @@ void HGraphBuilder::HandlePolymorphicStoreNamedField(Assignment* expr,
   // know about and do not want to handle ones we've never seen.  Otherwise
   // use a generic IC.
   if (count == types->length() && FLAG_deoptimize_uncommon_cases) {
-    current_block()->FinishExit(new HDeoptimize);
+    current_block()->FinishExitWithDeoptimization();
   } else {
     HInstruction* instr = BuildStoreNamedGeneric(object, name, value);
     instr->set_position(expr->position());
@@ -3480,7 +3488,7 @@ void HGraphBuilder::HandlePolymorphicLoadNamedField(Property* expr,
   // know about and do not want to handle ones we've never seen.  Otherwise
   // use a generic IC.
   if (count == types->length() && FLAG_deoptimize_uncommon_cases) {
-    current_block()->FinishExit(new HDeoptimize);
+    current_block()->FinishExitWithDeoptimization();
   } else {
     HInstruction* instr = BuildLoadNamedGeneric(object, expr);
     instr->set_position(expr->position());
@@ -3845,7 +3853,7 @@ void HGraphBuilder::HandlePolymorphicCallNamed(Call* expr,
   // know about and do not want to handle ones we've never seen.  Otherwise
   // use a generic IC.
   if (count == types->length() && FLAG_deoptimize_uncommon_cases) {
-    current_block()->FinishExit(new HDeoptimize);
+    current_block()->FinishExitWithDeoptimization();
   } else {
     HContext* context = new HContext;
     AddInstruction(context);

diff --git a/src/hydrogen.h b/src/hydrogen.h
index 53e908c1888dc63275f6b8f34929fe56ecf502a2..1ec8b0a1662d395e71b21bb579741a32ead4d248 100644 (file)
--- a/src/hydrogen.h
+++ b/src/hydrogen.h
@@ -124,6 +124,10 @@ class HBasicBlock: public ZoneObject {
   void AddSimulate(int id) { AddInstruction(CreateSimulate(id)); }
   void AssignCommonDominator(HBasicBlock* other);
 
+  void FinishExitWithDeoptimization() {
+    FinishExit(CreateDeoptimize());
+  }
+
   // Add the inlined function exit sequence, adding an HLeaveInlined
   // instruction and updating the bailout environment.
   void AddLeaveInlined(HValue* return_value, HBasicBlock* target);
@@ -146,6 +150,7 @@ class HBasicBlock: public ZoneObject {
   void AddDominatedBlock(HBasicBlock* block);
 
   HSimulate* CreateSimulate(int id);
+  HDeoptimize* CreateDeoptimize();
 
   int block_id_;
   HGraph* graph_;

diff --git a/test/mjsunit/regress/regress-1257.js b/test/mjsunit/regress/regress-1257.js
new file mode 100644 (file)
index 0000000..c20fb86
--- /dev/null
+++ b/test/mjsunit/regress/regress-1257.js
@@ -0,0 +1,58 @@
+// Copyright 2011 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 g(y) { assertEquals(y, 12); }
+
+var X = 0;
+
+function foo () {
+  var cnt = 0;
+  var l = -1;
+  var x = 0;
+  while (1) switch (l) {
+      case -1:
+        var y = x + 12;
+        l = 0;
+        break;
+      case 0:
+        // Loop for to hit OSR.
+        if (cnt++ < 10000000) {
+          l = 0;
+          break;
+        } else {
+          l = 1;
+          break;
+        }
+      case 1:
+        // This case will contain deoptimization
+        // because it has no type feedback.
+        g(y);
+        return;
+    };
+}
+
+foo();