Eagerly deoptimize on never-executed code-paths.
authorfschneider@chromium.org <fschneider@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 1 Jun 2011 11:04:40 +0000 (11:04 +0000)
committerfschneider@chromium.org <fschneider@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Wed, 1 Jun 2011 11:04:40 +0000 (11:04 +0000)
If type-feedback indicates that an expression was never executed in
the non-optimized code, we insert a forced deoptimization right away
to enable re-optimization if we ever hit this path.

With this change we still continue to build the graph. As a next step, we
should remove the dead code after the deoptimize.

I had to remove one assert about the optimization status in a test since
we now immediately deoptimize after exiting the loop that triggers OSR.

Also remove a restriction that control-flow from an inlined function in a
test context always reaches both true- and false-target.
Review URL: http://codereview.chromium.org/7105015

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

src/arm/lithium-arm.cc
src/hydrogen-instructions.cc
src/hydrogen-instructions.h
src/hydrogen.cc
src/hydrogen.h
src/ia32/lithium-ia32.cc
src/type-info.cc
src/x64/lithium-x64.cc
test/mjsunit/regress/regress-1118.js

index a3b6b95..e752a56 100644 (file)
@@ -808,6 +808,11 @@ LInstruction* LChunkBuilder::DoBlockEntry(HBlockEntry* instr) {
 }
 
 
+LInstruction* LChunkBuilder::DoSoftDeoptimize(HSoftDeoptimize* instr) {
+  return AssignEnvironment(new LDeoptimize);
+}
+
+
 LInstruction* LChunkBuilder::DoDeoptimize(HDeoptimize* instr) {
   return AssignEnvironment(new LDeoptimize);
 }
index 545f481..cc2dcf5 100644 (file)
@@ -1083,6 +1083,16 @@ void HSimulate::PrintDataTo(StringStream* stream) {
 }
 
 
+void HDeoptimize::PrintDataTo(StringStream* stream) {
+  if (OperandCount() == 0) return;
+  OperandAt(0)->PrintNameTo(stream);
+  for (int i = 1; i < OperandCount(); ++i) {
+    stream->Add(" ");
+    OperandAt(i)->PrintNameTo(stream);
+  }
+}
+
+
 void HEnterInlined::PrintDataTo(StringStream* stream) {
   SmartPointer<char> name = function()->debug_name()->ToCString();
   stream->Add("%s, id=%d", *name, function()->id());
index 15e3e1e..e8d8e81 100644 (file)
@@ -146,6 +146,7 @@ class LChunkBuilder;
   V(Shl)                                       \
   V(Shr)                                       \
   V(Simulate)                                  \
+  V(SoftDeoptimize)                            \
   V(StackCheck)                                \
   V(StoreContextSlot)                          \
   V(StoreGlobalCell)                           \
@@ -847,6 +848,19 @@ class HBlockEntry: public HTemplateInstruction<0> {
 };
 
 
+// We insert soft-deoptimize when we hit code with unknown typefeedback,
+// so that we get a chance of re-optimizing with useful typefeedback.
+// HSoftDeoptimize does not end a basic block as opposed to HDeoptimize.
+class HSoftDeoptimize: public HTemplateInstruction<0> {
+ public:
+  virtual Representation RequiredInputRepresentation(int index) const {
+    return Representation::None();
+  }
+
+  DECLARE_CONCRETE_INSTRUCTION(SoftDeoptimize)
+};
+
+
 class HDeoptimize: public HControlInstruction {
  public:
   explicit HDeoptimize(int environment_length)
@@ -859,6 +873,7 @@ class HDeoptimize: public HControlInstruction {
 
   virtual int OperandCount() { return values_.length(); }
   virtual HValue* OperandAt(int index) { return values_[index]; }
+  virtual void PrintDataTo(StringStream* stream);
 
   void AddEnvironmentValue(HValue* value) {
     values_.Add(NULL);
index 85fb187..7bf0db5 100644 (file)
@@ -68,8 +68,7 @@ HBasicBlock::HBasicBlock(HGraph* graph)
       last_instruction_index_(-1),
       deleted_phis_(4),
       parent_loop_header_(NULL),
-      is_inline_return_target_(false) {
-}
+      is_inline_return_target_(false) { }
 
 
 void HBasicBlock::AttachLoopInformation() {
@@ -4306,21 +4305,22 @@ bool HGraphBuilder::TryInline(Call* expr) {
   if (inlined_test_context() != NULL) {
     HBasicBlock* if_true = inlined_test_context()->if_true();
     HBasicBlock* if_false = inlined_test_context()->if_false();
-    if_true->SetJoinId(expr->id());
-    if_false->SetJoinId(expr->id());
-    ASSERT(ast_context() == inlined_test_context());
+
     // Pop the return test context from the expression context stack.
+    ASSERT(ast_context() == inlined_test_context());
     ClearInlinedTestContext();
 
     // Forward to the real test context.
-    HBasicBlock* true_target = TestContext::cast(ast_context())->if_true();
-    HBasicBlock* false_target = TestContext::cast(ast_context())->if_false();
-    if_true->Goto(true_target, false);
-    if_false->Goto(false_target, false);
-
-    // TODO(kmillikin): Come up with a better way to handle this. It is too
-    // subtle. NULL here indicates that the enclosing context has no control
-    // flow to handle.
+    if (if_true->HasPredecessor()) {
+      if_true->SetJoinId(expr->id());
+      HBasicBlock* true_target = TestContext::cast(ast_context())->if_true();
+      if_true->Goto(true_target, false);
+    }
+    if (if_false->HasPredecessor()) {
+      if_false->SetJoinId(expr->id());
+      HBasicBlock* false_target = TestContext::cast(ast_context())->if_false();
+      if_false->Goto(false_target, false);
+    }
     set_current_block(NULL);
 
   } else if (function_return()->HasPredecessor()) {
@@ -4787,6 +4787,10 @@ void HGraphBuilder::VisitSub(UnaryOperation* expr) {
   HValue* value = Pop();
   HInstruction* instr = new(zone()) HMul(value, graph_->GetConstantMinus1());
   TypeInfo info = oracle()->UnaryType(expr);
+  if (info.IsUninitialized()) {
+    AddInstruction(new(zone()) HSoftDeoptimize);
+    info = TypeInfo::Unknown();
+  }
   Representation rep = ToRepresentation(info);
   TraceRepresentation(expr->op(), info, instr, rep);
   instr->AssumeRepresentation(rep);
@@ -4797,6 +4801,10 @@ void HGraphBuilder::VisitSub(UnaryOperation* expr) {
 void HGraphBuilder::VisitBitNot(UnaryOperation* expr) {
   CHECK_ALIVE(VisitForValue(expr->expression()));
   HValue* value = Pop();
+  TypeInfo info = oracle()->UnaryType(expr);
+  if (info.IsUninitialized()) {
+    AddInstruction(new(zone()) HSoftDeoptimize);
+  }
   HInstruction* instr = new(zone()) HBitNot(value);
   ast_context()->ReturnInstruction(instr, expr->id());
 }
@@ -5028,7 +5036,57 @@ HInstruction* HGraphBuilder::BuildBinaryOperation(BinaryOperation* expr,
                                                   HValue* left,
                                                   HValue* right) {
   TypeInfo info = oracle()->BinaryType(expr);
-  HInstruction* instr = BuildBinaryOperation(expr->op(), left, right, info);
+  if (info.IsUninitialized()) {
+    AddInstruction(new(zone()) HSoftDeoptimize);
+    info = TypeInfo::Unknown();
+  }
+  HInstruction* instr = NULL;
+  switch (expr->op()) {
+    case Token::ADD:
+      if (info.IsString()) {
+        AddInstruction(new(zone()) HCheckNonSmi(left));
+        AddInstruction(HCheckInstanceType::NewIsString(left));
+        AddInstruction(new(zone()) HCheckNonSmi(right));
+        AddInstruction(HCheckInstanceType::NewIsString(right));
+        instr = new(zone()) HStringAdd(left, right);
+      } else {
+        instr = new(zone()) HAdd(left, right);
+      }
+      break;
+    case Token::SUB:
+      instr = new(zone()) HSub(left, right);
+      break;
+    case Token::MUL:
+      instr = new(zone()) HMul(left, right);
+      break;
+    case Token::MOD:
+      instr = new(zone()) HMod(left, right);
+      break;
+    case Token::DIV:
+      instr = new(zone()) HDiv(left, right);
+      break;
+    case Token::BIT_XOR:
+      instr = new(zone()) HBitXor(left, right);
+      break;
+    case Token::BIT_AND:
+      instr = new(zone()) HBitAnd(left, right);
+      break;
+    case Token::BIT_OR:
+      instr = new(zone()) HBitOr(left, right);
+      break;
+    case Token::SAR:
+      instr = new(zone()) HSar(left, right);
+      break;
+    case Token::SHR:
+      instr = new(zone()) HShr(left, right);
+      break;
+    case Token::SHL:
+      instr = new(zone()) HShl(left, right);
+      break;
+    default:
+      UNREACHABLE();
+  }
+
   // If we hit an uninitialized binary op stub we will get type info
   // for a smi operation. If one of the operands is a constant string
   // do not generate code assuming it is a smi operation.
@@ -5048,36 +5106,6 @@ HInstruction* HGraphBuilder::BuildBinaryOperation(BinaryOperation* expr,
 }
 
 
-HInstruction* HGraphBuilder::BuildBinaryOperation(
-    Token::Value op, HValue* left, HValue* right, TypeInfo info) {
-  switch (op) {
-    case Token::ADD:
-      if (info.IsString()) {
-        AddInstruction(new(zone()) HCheckNonSmi(left));
-        AddInstruction(HCheckInstanceType::NewIsString(left));
-        AddInstruction(new(zone()) HCheckNonSmi(right));
-        AddInstruction(HCheckInstanceType::NewIsString(right));
-        return new(zone()) HStringAdd(left, right);
-      } else {
-        return new(zone()) HAdd(left, right);
-      }
-    case Token::SUB: return new(zone()) HSub(left, right);
-    case Token::MUL: return new(zone()) HMul(left, right);
-    case Token::MOD: return new(zone()) HMod(left, right);
-    case Token::DIV: return new(zone()) HDiv(left, right);
-    case Token::BIT_XOR: return new(zone()) HBitXor(left, right);
-    case Token::BIT_AND: return new(zone()) HBitAnd(left, right);
-    case Token::BIT_OR: return new(zone()) HBitOr(left, right);
-    case Token::SAR: return new(zone()) HSar(left, right);
-    case Token::SHR: return new(zone()) HShr(left, right);
-    case Token::SHL: return new(zone()) HShl(left, right);
-    default:
-      UNREACHABLE();
-      return NULL;
-  }
-}
-
-
 // Check for the form (%_ClassOf(foo) === 'BarClass').
 static bool IsClassOfTest(CompareOperation* expr) {
   if (expr->op() != Token::EQ_STRICT) return false;
@@ -5275,6 +5303,13 @@ void HGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
     return;
   }
 
+  TypeInfo type_info = oracle()->CompareType(expr);
+  // Check if this expression was ever executed according to type feedback.
+  if (type_info.IsUninitialized()) {
+    AddInstruction(new(zone()) HSoftDeoptimize);
+    type_info = TypeInfo::Unknown();
+  }
+
   CHECK_ALIVE(VisitForValue(expr->left()));
   CHECK_ALIVE(VisitForValue(expr->right()));
 
@@ -5282,7 +5317,6 @@ void HGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
   HValue* left = Pop();
   Token::Value op = expr->op();
 
-  TypeInfo type_info = oracle()->CompareType(expr);
   HInstruction* instr = NULL;
   if (op == Token::INSTANCEOF) {
     // Check to see if the rhs of the instanceof is a global function not
index 6034801..3be8fd8 100644 (file)
@@ -878,10 +878,6 @@ class HGraphBuilder: public AstVisitor {
   HInstruction* BuildBinaryOperation(BinaryOperation* expr,
                                      HValue* left,
                                      HValue* right);
-  HInstruction* BuildBinaryOperation(Token::Value op,
-                                     HValue* left,
-                                     HValue* right,
-                                     TypeInfo info);
   HInstruction* BuildIncrement(bool returns_original_input,
                                CountOperation* expr);
   HLoadNamedField* BuildLoadNamedField(HValue* object,
index 1a9316c..a47f506 100644 (file)
@@ -804,6 +804,11 @@ LInstruction* LChunkBuilder::DoBlockEntry(HBlockEntry* instr) {
 }
 
 
+LInstruction* LChunkBuilder::DoSoftDeoptimize(HSoftDeoptimize* instr) {
+  return AssignEnvironment(new LDeoptimize);
+}
+
+
 LInstruction* LChunkBuilder::DoDeoptimize(HDeoptimize* instr) {
   return AssignEnvironment(new LDeoptimize);
 }
index 5f794bd..37ab706 100644 (file)
@@ -224,8 +224,7 @@ TypeInfo TypeFeedbackOracle::CompareType(CompareOperation* expr) {
   switch (state) {
     case CompareIC::UNINITIALIZED:
       // Uninitialized means never executed.
-      // TODO(fschneider): Introduce a separate value for never-executed ICs.
-      return unknown;
+      return TypeInfo::Uninitialized();
     case CompareIC::SMIS:
       return TypeInfo::Smi();
     case CompareIC::HEAP_NUMBERS:
@@ -286,8 +285,7 @@ TypeInfo TypeFeedbackOracle::BinaryType(BinaryOperation* expr) {
     switch (type) {
       case BinaryOpIC::UNINITIALIZED:
         // Uninitialized means never executed.
-        // TODO(fschneider): Introduce a separate value for never-executed ICs
-        return unknown;
+        return TypeInfo::Uninitialized();
       case BinaryOpIC::SMI:
         switch (result_type) {
           case BinaryOpIC::UNINITIALIZED:
index 517870f..72f1321 100644 (file)
@@ -803,6 +803,11 @@ LInstruction* LChunkBuilder::DoBlockEntry(HBlockEntry* instr) {
 }
 
 
+LInstruction* LChunkBuilder::DoSoftDeoptimize(HSoftDeoptimize* instr) {
+  return AssignEnvironment(new LDeoptimize);
+}
+
+
 LInstruction* LChunkBuilder::DoDeoptimize(HDeoptimize* instr) {
   return AssignEnvironment(new LDeoptimize);
 }
index 86b8981..7e0461d 100644 (file)
@@ -55,8 +55,6 @@ function h() {
     while (%GetOptimizationStatus(h) == 2) {
       for (var j = 0; j < 100; j++) g();
     }
-    assertTrue(%GetOptimizationStatus(h) == 1 ||
-               %GetOptimizationStatus(h) == 3);
     g();
   }
 }