From 0aa422923cda6b78e074c9dd71ab2cdc61437324 Mon Sep 17 00:00:00 2001 From: "fschneider@chromium.org" Date: Wed, 1 Jun 2011 11:04:40 +0000 Subject: [PATCH] Eagerly deoptimize on never-executed code-paths. 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 | 5 ++ src/hydrogen-instructions.cc | 10 +++ src/hydrogen-instructions.h | 15 ++++ src/hydrogen.cc | 124 +++++++++++++++++---------- src/hydrogen.h | 4 - src/ia32/lithium-ia32.cc | 5 ++ src/type-info.cc | 6 +- src/x64/lithium-x64.cc | 5 ++ test/mjsunit/regress/regress-1118.js | 2 - 9 files changed, 121 insertions(+), 55 deletions(-) diff --git a/src/arm/lithium-arm.cc b/src/arm/lithium-arm.cc index a3b6b95659..e752a56dad 100644 --- a/src/arm/lithium-arm.cc +++ b/src/arm/lithium-arm.cc @@ -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); } diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc index 545f481089..cc2dcf5c78 100644 --- a/src/hydrogen-instructions.cc +++ b/src/hydrogen-instructions.cc @@ -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 name = function()->debug_name()->ToCString(); stream->Add("%s, id=%d", *name, function()->id()); diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 15e3e1e515..e8d8e81a8b 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -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); diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 85fb18765b..7bf0db5ab4 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -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 diff --git a/src/hydrogen.h b/src/hydrogen.h index 603480130b..3be8fd8fd7 100644 --- a/src/hydrogen.h +++ b/src/hydrogen.h @@ -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, diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc index 1a9316c559..a47f506834 100644 --- a/src/ia32/lithium-ia32.cc +++ b/src/ia32/lithium-ia32.cc @@ -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); } diff --git a/src/type-info.cc b/src/type-info.cc index 5f794bdef4..37ab706c6b 100644 --- a/src/type-info.cc +++ b/src/type-info.cc @@ -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: diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc index 517870ffd4..72f13216f6 100644 --- a/src/x64/lithium-x64.cc +++ b/src/x64/lithium-x64.cc @@ -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); } diff --git a/test/mjsunit/regress/regress-1118.js b/test/mjsunit/regress/regress-1118.js index 86b8981d2c..7e0461db4d 100644 --- a/test/mjsunit/regress/regress-1118.js +++ b/test/mjsunit/regress/regress-1118.js @@ -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(); } }