Reviewers: Toon Verwaest,

Message:
Hi Toon, here is the change we discussed for deopt reasons.
Thanks,
--Michael

Description:
Deoptimization is easier to diagnose when there is a text reason.

BUG=

Please review this at https://codereview.chromium.org/22339018/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/arm/lithium-codegen-arm.cc
  M src/code-stubs-hydrogen.cc
  M src/hydrogen-instructions.h
  M src/hydrogen.h
  M src/hydrogen.cc
  M src/ia32/lithium-codegen-ia32.cc
  M src/mips/lithium-codegen-mips.cc
  M src/x64/lithium-codegen-x64.cc


Index: src/arm/lithium-codegen-arm.cc
diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index d22bcb59100ddf6abd751288c422dbca189ff285..9d2c9bfe55ee5f713e16d3d4687fa67b5f5b4c11 100644
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -5680,6 +5680,8 @@ void LCodeGen::DoDeoptimize(LDeoptimize* instr) {
   if (info()->IsStub() && type == Deoptimizer::EAGER) {
     type = Deoptimizer::LAZY;
   }
+
+  Comment(";;; deoptimize: %s", instr->hydrogen()->reason());
   DeoptimizeIf(al, instr->environment(), type);
 }

Index: src/code-stubs-hydrogen.cc
diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc
index 852f7b569727b08939d1c952146699d6ff4999f7..9bbab9ab7c69ee67beb514754d117d193ed3c94b 100644
--- a/src/code-stubs-hydrogen.cc
+++ b/src/code-stubs-hydrogen.cc
@@ -92,7 +92,7 @@ class CodeStubGraphBuilderBase : public HGraphBuilder {
     }

     ~ArrayContextChecker() {
-      checker_.ElseDeopt();
+ checker_.ElseDeopt("Array constructor called from different context");
       checker_.End();
     }
    private:
@@ -233,7 +233,7 @@ class CodeStubGraphBuilder: public CodeStubGraphBuilderBase {
     IfBuilder builder(this);
builder.IfNot<HCompareObjectEqAndBranch, HValue*>(undefined, undefined);
     builder.Then();
-    builder.ElseDeopt();
+    builder.ElseDeopt("Forced deopt to runtime");
     return undefined;
   }

@@ -387,7 +387,7 @@ HValue* CodeStubGraphBuilder<FastCloneShallowArrayStub>::BuildCodeStub() {
                                                length));
   }

-  checker.ElseDeopt();
+  checker.ElseDeopt("Uninitialized boilerplate literals");
   checker.End();

   return environment()->Pop();
@@ -434,7 +434,7 @@ HValue* CodeStubGraphBuilder<FastCloneShallowObjectStub>::BuildCodeStub() {
   }

   environment()->Push(object);
-  checker.ElseDeopt();
+  checker.ElseDeopt("Uninitialized boilerplate in fast clone");
   checker.End();

   return environment()->Pop();
@@ -844,7 +844,7 @@ HValue* CodeStubGraphBuilder<StoreGlobalStub>::BuildCodeInitializedStub() {
     IfBuilder builder(this);
     builder.If<HCompareObjectEqAndBranch>(cell_contents, value);
     builder.Then();
-    builder.ElseDeopt();
+    builder.ElseDeopt("Unexpected cell contents in constant global store");
     builder.End();
   } else {
// Load the payload of the global parameter cell. A hole indicates that the @@ -854,7 +854,7 @@ HValue* CodeStubGraphBuilder<StoreGlobalStub>::BuildCodeInitializedStub() {
     HValue* hole_value = Add<HConstant>(hole);
     builder.If<HCompareObjectEqAndBranch>(cell_contents, hole_value);
     builder.Then();
-    builder.Deopt();
+    builder.Deopt("Unexpected cell contents in global store");
     builder.Else();
     Add<HStoreNamedField>(cell, access, value);
     builder.End();
@@ -878,7 +878,8 @@ HValue* CodeStubGraphBuilder<ElementsTransitionAndStoreStub>::BuildCodeStub() {

   if (FLAG_trace_elements_transitions) {
     // Tracing elements transitions is the job of the runtime.
-    Add<HDeoptimize>(Deoptimizer::EAGER);
+    Add<HDeoptimize>("Deopt due to --trace-elements-transitions",
+                     Deoptimizer::EAGER);
   } else {
     info()->MarkAsSavesCallerDoubles();

Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index f977a750d946a6b24d657b08229fc4a238c368b2..68d5b3f6da19dfed99f4d8dba64d3b8822f135bf 100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -1248,19 +1248,23 @@ class HDummyUse: public HTemplateInstruction<1> {

 class HDeoptimize: public HTemplateInstruction<0> {
  public:
-  DECLARE_INSTRUCTION_FACTORY_P1(HDeoptimize, Deoptimizer::BailoutType);
+  DECLARE_INSTRUCTION_FACTORY_P2(HDeoptimize, const char*,
+                                 Deoptimizer::BailoutType);

   virtual Representation RequiredInputRepresentation(int index) {
     return Representation::None();
   }

+  const char* reason() const { return reason_; }
   Deoptimizer::BailoutType type() { return type_; }

   DECLARE_CONCRETE_INSTRUCTION(Deoptimize)

  private:
-  explicit HDeoptimize(Deoptimizer::BailoutType type) : type_(type) {}
+  explicit HDeoptimize(const char* reason, Deoptimizer::BailoutType type)
+      : reason_(reason), type_(type) {}

+  const char* reason_;
   Deoptimizer::BailoutType type_;
 };

Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 2523d9b1b24502a0fe09e01c64103581812e6c8d..e7ac2fe1b18e7a5fdd87b8586a05d378484b99b2 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -824,14 +824,14 @@ void HGraphBuilder::IfBuilder::Else() {
 }


-void HGraphBuilder::IfBuilder::Deopt() {
+void HGraphBuilder::IfBuilder::Deopt(const char* reason) {
   ASSERT(did_then_);
   if (did_else_) {
     deopt_else_ = true;
   } else {
     deopt_then_ = true;
   }
-  builder_->Add<HDeoptimize>(Deoptimizer::EAGER);
+  builder_->Add<HDeoptimize>(reason, Deoptimizer::EAGER);
 }


@@ -1034,9 +1034,9 @@ HValue* HGraphBuilder::BuildCheckHeapObject(HValue* obj) {


 void HGraphBuilder::FinishExitWithHardDeoptimization(
-    HBasicBlock* continuation) {
+    HBasicBlock* continuation, const char* reason) {
   PadEnvironmentForContinuation(current_block(), continuation);
-  Add<HDeoptimize>(Deoptimizer::EAGER);
+  Add<HDeoptimize>(reason, Deoptimizer::EAGER);
   if (no_side_effects_scope_count_ > 0) {
     current_block()->GotoNoSimulate(continuation);
   } else {
@@ -1108,7 +1108,7 @@ HValue* HGraphBuilder::BuildCheckForCapacityGrow(HValue* object,
   IfBuilder key_checker(this);
   key_checker.If<HCompareNumericAndBranch>(key, max_capacity, Token::LT);
   key_checker.Then();
-  key_checker.ElseDeopt();
+  key_checker.ElseDeopt("key out of capacity range");
   key_checker.End();

   HValue* new_capacity = BuildNewElementsCapacity(key);
@@ -1264,7 +1264,7 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
       negative_checker.Then();
       HInstruction* result = AddExternalArrayElementAccess(
external_elements, key, val, bounds_check, elements_kind, is_store);
-      negative_checker.ElseDeopt();
+      negative_checker.ElseDeopt("Negative key encountered");
       length_checker.End();
       return result;
     } else {
@@ -1751,7 +1751,7 @@ void HGraphBuilder::BuildCompareNil(
       // emitted below is the actual monomorphic map.
       BuildCheckMap(value, type->Classes().Current());
     } else {
-      if_nil.Deopt();
+      if_nil.Deopt("Too many undetectable types");
     }
   }

@@ -3347,7 +3347,7 @@ void HOptimizedGraphBuilder::VisitSwitchStatement(SwitchStatement* stmt) {

     if (stmt->switch_type() == SwitchStatement::SMI_SWITCH) {
       if (!clause->compare_type()->Is(Type::Smi())) {
-        Add<HDeoptimize>(Deoptimizer::SOFT);
+        Add<HDeoptimize>("Non-smi switch type", Deoptimizer::SOFT);
       }

       HCompareNumericAndBranch* compare_ =
@@ -4736,7 +4736,7 @@ void HOptimizedGraphBuilder::HandlePolymorphicStoreNamedField(
   // 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) {
-    FinishExitWithHardDeoptimization(join);
+    FinishExitWithHardDeoptimization(join, "All known maps handled");
   } else {
HInstruction* instr = BuildStoreNamedGeneric(object, name, store_value);
     instr->set_position(position);
@@ -4784,7 +4784,10 @@ void HOptimizedGraphBuilder::HandlePropertyAssignment(Assignment* expr) {
     HValue* value = environment()->ExpressionStackAt(0);
     HValue* object = environment()->ExpressionStackAt(1);

-    if (expr->IsUninitialized()) Add<HDeoptimize>(Deoptimizer::SOFT);
+    if (expr->IsUninitialized()) {
+ Add<HDeoptimize>("Insufficient type feedback for property assignment",
+                       Deoptimizer::SOFT);
+    }
     return BuildStoreNamed(expr, expr->id(), expr->position(),
expr->AssignmentId(), prop, object, value, value);
   } else {
@@ -4830,7 +4833,8 @@ void HOptimizedGraphBuilder::HandleGlobalVariableAssignment(
       }
       builder.Then();
       builder.Else();
-      Add<HDeoptimize>(Deoptimizer::EAGER);
+      Add<HDeoptimize>("Constant global variable assignment",
+                       Deoptimizer::EAGER);
       builder.End();
     }
     HInstruction* instr =
@@ -5271,7 +5275,8 @@ HInstruction* HOptimizedGraphBuilder::BuildLoadNamedGeneric(
     Handle<String> name,
     Property* expr) {
   if (expr->IsUninitialized()) {
-    Add<HDeoptimize>(Deoptimizer::SOFT);
+    Add<HDeoptimize>("Insufficient feedback for generic named load",
+                     Deoptimizer::SOFT);
   }
   HValue* context = environment()->context();
   return new(zone()) HLoadNamedGeneric(context, object, name);
@@ -5588,7 +5593,8 @@ HValue* HOptimizedGraphBuilder::HandlePolymorphicElementAccess(

   // Deopt if none of the cases matched.
   NoObservableSideEffectsScope scope(this);
-  FinishExitWithHardDeoptimization(join);
+  FinishExitWithHardDeoptimization(join,
+      "Unknown type in polymorphic element access");
   set_current_block(join);
   return is_store ? NULL : Pop();
 }
@@ -5624,12 +5630,14 @@ HValue* HOptimizedGraphBuilder::HandleKeyedElementAccess(
   } else {
     if (is_store) {
if (expr->IsAssignment() && expr->AsAssignment()->IsUninitialized()) {
-        Add<HDeoptimize>(Deoptimizer::SOFT);
+        Add<HDeoptimize>("Insufficient feedback for keyed store",
+                         Deoptimizer::SOFT);
       }
       instr = BuildStoreKeyedGeneric(obj, key, val);
     } else {
       if (expr->AsProperty()->IsUninitialized()) {
-        Add<HDeoptimize>(Deoptimizer::SOFT);
+        Add<HDeoptimize>("Insufficient feedback for keyed load",
+                         Deoptimizer::SOFT);
       }
       instr = BuildLoadKeyedGeneric(obj, key);
     }
@@ -6076,7 +6084,8 @@ void HOptimizedGraphBuilder::HandlePolymorphicCallNamed( // that the environment stack matches the depth on deopt that it otherwise
     // would have had after a successful call.
     Drop(argument_count - (ast_context()->IsEffect() ? 0 : 1));
-    FinishExitWithHardDeoptimization(join);
+    FinishExitWithHardDeoptimization(join,
+                                     "Unknown map in polymorphic call");
   } else {
     HValue* context = environment()->context();
HCallNamed* call = new(zone()) HCallNamed(context, name, argument_count); @@ -7636,12 +7645,14 @@ HInstruction* HOptimizedGraphBuilder::BuildBinaryOperation(
   }

   if (left_type->Is(Type::None())) {
-    Add<HDeoptimize>(Deoptimizer::SOFT);
+    Add<HDeoptimize>("Insufficient type feedback for left side",
+                     Deoptimizer::SOFT);
// TODO(rossberg): we should be able to get rid of non-continuous defaults.
     left_type = handle(Type::Any(), isolate());
   }
   if (right_type->Is(Type::None())) {
-    Add<HDeoptimize>(Deoptimizer::SOFT);
+    Add<HDeoptimize>("Insufficient type feedback for right side",
+                     Deoptimizer::SOFT);
     right_type = handle(Type::Any(), isolate());
   }
   HInstruction* instr = NULL;
@@ -7991,7 +8002,8 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
   // Cases handled below depend on collected type feedback. They should
   // soft deoptimize when there is no type feedback.
   if (combined_type->Is(Type::None())) {
-    Add<HDeoptimize>(Deoptimizer::SOFT);
+    Add<HDeoptimize>("insufficient type feedback for combined type",
+                     Deoptimizer::SOFT);
combined_type = left_type = right_type = handle(Type::Any(), isolate());
   }

Index: src/hydrogen.h
diff --git a/src/hydrogen.h b/src/hydrogen.h
index 6312a52376f50713ac584460634dbf4a63161d28..9f3ffa0f1e8b9a38236129398e048e7a7846bcfa 100644
--- a/src/hydrogen.h
+++ b/src/hydrogen.h
@@ -1270,7 +1270,8 @@ class HGraphBuilder {

   void PushAndAdd(HInstruction* instr);

-  void FinishExitWithHardDeoptimization(HBasicBlock* continuation);
+  void FinishExitWithHardDeoptimization(HBasicBlock* continuation,
+                                        const char* reason);

   void AddIncrementCounter(StatsCounter* counter,
                            HValue* context);
@@ -1374,10 +1375,10 @@ class HGraphBuilder {
     void Else();
     void End();

-    void Deopt();
-    void ElseDeopt() {
+    void Deopt(const char* reason);
+    void ElseDeopt(const char* reason) {
       Else();
-      Deopt();
+      Deopt(reason);
     }

     void Return(HValue* value);
@@ -1582,13 +1583,13 @@ class HGraphBuilder {

 template<>
 inline HInstruction* HGraphBuilder::AddUncasted<HDeoptimize>(
-    Deoptimizer::BailoutType type) {
+    const char* reason, Deoptimizer::BailoutType type) {
   if (type == Deoptimizer::SOFT) {
     isolate()->counters()->soft_deopts_requested()->Increment();
     if (FLAG_always_opt) return NULL;
   }
   if (current_block()->IsDeoptimizing()) return NULL;
-  HDeoptimize* instr = New<HDeoptimize>(type);
+  HDeoptimize* instr = New<HDeoptimize>(reason, type);
   AddInstruction(instr);
   if (type == Deoptimizer::SOFT) {
     isolate()->counters()->soft_deopts_inserted()->Increment();
@@ -1601,8 +1602,8 @@ inline HInstruction* HGraphBuilder::AddUncasted<HDeoptimize>(

 template<>
 inline HDeoptimize* HGraphBuilder::Add<HDeoptimize>(
-    Deoptimizer::BailoutType type) {
-  return static_cast<HDeoptimize*>(AddUncasted<HDeoptimize>(type));
+    const char* reason, Deoptimizer::BailoutType type) {
+  return static_cast<HDeoptimize*>(AddUncasted<HDeoptimize>(reason, type));
 }


Index: src/ia32/lithium-codegen-ia32.cc
diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index 0d22bc914168eb9fd10d69db7d34023626b8d5a7..ddad4551a614db6f76c375f5b6c294c3ee2b20a8 100644
--- a/src/ia32/lithium-codegen-ia32.cc
+++ b/src/ia32/lithium-codegen-ia32.cc
@@ -6376,6 +6376,7 @@ void LCodeGen::DoDeoptimize(LDeoptimize* instr) {
   if (info()->IsStub() && type == Deoptimizer::EAGER) {
     type = Deoptimizer::LAZY;
   }
+  Comment(";;; deoptimize: %s", instr->hydrogen()->reason());
   DeoptimizeIf(no_condition, instr->environment(), type);
 }

Index: src/mips/lithium-codegen-mips.cc
diff --git a/src/mips/lithium-codegen-mips.cc b/src/mips/lithium-codegen-mips.cc index 34e601ccaa386a3eded1aad2b94fefccaf29bf56..6c8f0172f9f7ee8a3b40cef44652b13a9efabf26 100644
--- a/src/mips/lithium-codegen-mips.cc
+++ b/src/mips/lithium-codegen-mips.cc
@@ -5675,6 +5675,8 @@ void LCodeGen::DoDeoptimize(LDeoptimize* instr) {
   if (info()->IsStub() && type == Deoptimizer::EAGER) {
     type = Deoptimizer::LAZY;
   }
+
+  Comment(";;; deoptimize: %s", instr->hydrogen()->reason());
DeoptimizeIf(al, instr->environment(), type, zero_reg, Operand(zero_reg));
 }

Index: src/x64/lithium-codegen-x64.cc
diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index cc635100b18dcab10c9e4e9c02aee8ab50206464..5b6bd34d1a36b2475929052b6f9b723a180b6934 100644
--- a/src/x64/lithium-codegen-x64.cc
+++ b/src/x64/lithium-codegen-x64.cc
@@ -5447,6 +5447,8 @@ void LCodeGen::DoDeoptimize(LDeoptimize* instr) {
   if (info()->IsStub() && type == Deoptimizer::EAGER) {
     type = Deoptimizer::LAZY;
   }
+
+  Comment(";;; deoptimize: %s", instr->hydrogen()->reason());
   DeoptimizeIf(no_condition, instr->environment(), type);
 }



--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to