Revision: 8185
Author:   [email protected]
Date:     Mon Jun  6 07:57:25 2011
Log:      Re-land r8140: Deoptimize on never-executed code-paths.

Original cl: http://codereview.chromium.org/7105015

I'm removing the test GlobalLoadICGC test that was introduced for testing
inlined global cell loads (in the classic backend) and has an invalid assumption about the number of global objects referenced from a v8 context. We don't have
this feature with Crankshaft anymore.
Review URL: http://codereview.chromium.org/7112032
http://code.google.com/p/v8/source/detail?r=8185

Modified:
 /branches/bleeding_edge/src/arm/lithium-arm.cc
 /branches/bleeding_edge/src/hydrogen-instructions.cc
 /branches/bleeding_edge/src/hydrogen-instructions.h
 /branches/bleeding_edge/src/hydrogen.cc
 /branches/bleeding_edge/src/hydrogen.h
 /branches/bleeding_edge/src/ia32/lithium-ia32.cc
 /branches/bleeding_edge/src/type-info.cc
 /branches/bleeding_edge/src/x64/lithium-x64.cc
 /branches/bleeding_edge/test/cctest/test-api.cc
 /branches/bleeding_edge/test/mjsunit/regress/regress-1118.js

=======================================
--- /branches/bleeding_edge/src/arm/lithium-arm.cc      Mon Jun  6 04:30:17 2011
+++ /branches/bleeding_edge/src/arm/lithium-arm.cc      Mon Jun  6 07:57:25 2011
@@ -806,6 +806,11 @@
 LInstruction* LChunkBuilder::DoBlockEntry(HBlockEntry* instr) {
   return new LLabel(instr->block());
 }
+
+
+LInstruction* LChunkBuilder::DoSoftDeoptimize(HSoftDeoptimize* instr) {
+  return AssignEnvironment(new LDeoptimize);
+}


 LInstruction* LChunkBuilder::DoDeoptimize(HDeoptimize* instr) {
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.cc Wed Jun 1 06:34:15 2011 +++ /branches/bleeding_edge/src/hydrogen-instructions.cc Mon Jun 6 07:57:25 2011
@@ -1081,6 +1081,16 @@
     }
   }
 }
+
+
+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) {
=======================================
--- /branches/bleeding_edge/src/hydrogen-instructions.h Wed Jun 1 06:34:15 2011 +++ /branches/bleeding_edge/src/hydrogen-instructions.h Mon Jun 6 07:57:25 2011
@@ -146,6 +146,7 @@
   V(Shl)                                       \
   V(Shr)                                       \
   V(Simulate)                                  \
+  V(SoftDeoptimize)                            \
   V(StackCheck)                                \
   V(StoreContextSlot)                          \
   V(StoreGlobalCell)                           \
@@ -847,6 +848,19 @@
 };


+// 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 @@

   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);
=======================================
--- /branches/bleeding_edge/src/hydrogen.cc     Wed Jun  1 06:34:15 2011
+++ /branches/bleeding_edge/src/hydrogen.cc     Mon Jun  6 07:57:25 2011
@@ -68,8 +68,7 @@
       last_instruction_index_(-1),
       deleted_phis_(4),
       parent_loop_header_(NULL),
-      is_inline_return_target_(false) {
-}
+      is_inline_return_target_(false) { }


 void HBasicBlock::AttachLoopInformation() {
@@ -4310,21 +4309,22 @@
   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()) {
@@ -4791,6 +4791,10 @@
   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);
@@ -4801,6 +4805,10 @@
 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());
 }
@@ -5032,7 +5040,57 @@
                                                   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.
@@ -5050,36 +5108,6 @@
   instr->AssumeRepresentation(rep);
   return instr;
 }
-
-
-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').
@@ -5278,6 +5306,13 @@
     ast_context()->ReturnInstruction(instr, expr->id());
     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()));
@@ -5286,7 +5321,6 @@
   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
=======================================
--- /branches/bleeding_edge/src/hydrogen.h      Wed Jun  1 06:34:15 2011
+++ /branches/bleeding_edge/src/hydrogen.h      Mon Jun  6 07:57:25 2011
@@ -878,10 +878,6 @@
   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,
=======================================
--- /branches/bleeding_edge/src/ia32/lithium-ia32.cc Mon Jun 6 04:30:17 2011 +++ /branches/bleeding_edge/src/ia32/lithium-ia32.cc Mon Jun 6 07:57:25 2011
@@ -802,6 +802,11 @@
 LInstruction* LChunkBuilder::DoBlockEntry(HBlockEntry* instr) {
   return new LLabel(instr->block());
 }
+
+
+LInstruction* LChunkBuilder::DoSoftDeoptimize(HSoftDeoptimize* instr) {
+  return AssignEnvironment(new LDeoptimize);
+}


 LInstruction* LChunkBuilder::DoDeoptimize(HDeoptimize* instr) {
=======================================
--- /branches/bleeding_edge/src/type-info.cc    Wed Jun  1 06:34:15 2011
+++ /branches/bleeding_edge/src/type-info.cc    Mon Jun  6 07:57:25 2011
@@ -224,8 +224,7 @@
   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 @@
     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:
=======================================
--- /branches/bleeding_edge/src/x64/lithium-x64.cc      Mon Jun  6 04:30:17 2011
+++ /branches/bleeding_edge/src/x64/lithium-x64.cc      Mon Jun  6 07:57:25 2011
@@ -801,6 +801,11 @@
 LInstruction* LChunkBuilder::DoBlockEntry(HBlockEntry* instr) {
   return new LLabel(instr->block());
 }
+
+
+LInstruction* LChunkBuilder::DoSoftDeoptimize(HSoftDeoptimize* instr) {
+  return AssignEnvironment(new LDeoptimize);
+}


 LInstruction* LChunkBuilder::DoDeoptimize(HDeoptimize* instr) {
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc     Mon May 30 06:49:22 2011
+++ /branches/bleeding_edge/test/cctest/test-api.cc     Mon Jun  6 07:57:25 2011
@@ -14036,48 +14036,6 @@
                "})()",
                "ReferenceError: cell is not defined");
 }
-
-
-TEST(GlobalLoadICGC) {
-  const char* function_code =
-      "function readCell() { while (true) { return cell; } }";
-
-  // Check inline load code for a don't delete cell is cleared during
-  // GC.
-  {
-    v8::HandleScope scope;
-    LocalContext context;
-    CompileRun("var cell = \"value\";");
-    ExpectBoolean("delete cell", false);
-    CompileRun(function_code);
-    ExpectString("readCell()", "value");
-    ExpectString("readCell()", "value");
-  }
-  {
-    v8::HandleScope scope;
-    LocalContext context2;
-    // Hold the code object in the second context.
-    CompileRun(function_code);
-    CheckSurvivingGlobalObjectsCount(1);
-  }
-
-  // Check inline load code for a deletable cell is cleared during GC.
-  {
-    v8::HandleScope scope;
-    LocalContext context;
-    CompileRun("cell = \"value\";");
-    CompileRun(function_code);
-    ExpectString("readCell()", "value");
-    ExpectString("readCell()", "value");
-  }
-  {
-    v8::HandleScope scope;
-    LocalContext context2;
-    // Hold the code object in the second context.
-    CompileRun(function_code);
-    CheckSurvivingGlobalObjectsCount(1);
-  }
-}


 TEST(RegExp) {
=======================================
--- /branches/bleeding_edge/test/mjsunit/regress/regress-1118.js Wed Jun 1 06:34:15 2011 +++ /branches/bleeding_edge/test/mjsunit/regress/regress-1118.js Mon Jun 6 07:57:25 2011
@@ -55,8 +55,6 @@
     while (%GetOptimizationStatus(h) == 2) {
       for (var j = 0; j < 100; j++) g();
     }
-    assertTrue(%GetOptimizationStatus(h) == 1 ||
-               %GetOptimizationStatus(h) == 3);
     g();
   }
 }

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to