From c5229b311968fd638a6cd537c341b1055eb7be97 Mon Sep 17 00:00:00 2001 From: adamk Date: Thu, 11 Feb 2016 16:42:52 -0800 Subject: [PATCH] Revert of [interpreter] Correctly thread through catch prediction. (patchset #1 id:1 of https://codereview.chromium.org/1690973002/ ) Reason for revert: Depends on the reverted https://codereview.chromium.org/1691723002 Original issue's description: > [interpreter] Correctly thread through catch prediction. > > This change correctly sets the {CatchPrediction} field in exception > handler tables for bytecode and optimized code. It also adds tests > independent of promise handling for this prediction, to ensure all our > backends are in sync on their prediction. > > R=rmcilroy@chromium.org,yangguo@chromium.org > TEST=mjsunit/compiler/debug-catch-prediction > BUG=v8:4674 > LOG=n > > Committed: https://crrev.com/ba55f5594cb0b4a1a1e9b35d87fe54afe2d93f3b > Cr-Commit-Position: refs/heads/master@{#33906} TBR=rmcilroy@chromium.org,yangguo@chromium.org,mstarzinger@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=v8:4674 Review URL: https://codereview.chromium.org/1695613002 Cr-Commit-Position: refs/heads/master@{#33922} --- src/compiler/bytecode-graph-builder.cc | 10 +- src/compiler/bytecode-graph-builder.h | 4 - src/interpreter/bytecode-generator.cc | 20 +-- src/interpreter/bytecode-generator.h | 6 - src/interpreter/control-flow-builders.cc | 2 +- src/interpreter/control-flow-builders.h | 10 +- src/objects-inl.h | 6 - src/objects.h | 3 - .../compiler/debug-catch-prediction.js | 143 ------------------ 9 files changed, 10 insertions(+), 194 deletions(-) delete mode 100644 test/mjsunit/compiler/debug-catch-prediction.js diff --git a/src/compiler/bytecode-graph-builder.cc b/src/compiler/bytecode-graph-builder.cc index 4cfd68d4ca..e4cc3a90f8 100644 --- a/src/compiler/bytecode-graph-builder.cc +++ b/src/compiler/bytecode-graph-builder.cc @@ -1629,10 +1629,8 @@ void BytecodeGraphBuilder::EnterAndExitExceptionHandlers(int current_offset) { int next_end = table->GetRangeEnd(current_exception_handler_); int next_handler = table->GetRangeHandler(current_exception_handler_); int context_register = table->GetRangeData(current_exception_handler_); - CatchPrediction pred = - table->GetRangePrediction(current_exception_handler_); exception_handlers_.push( - {next_start, next_end, next_handler, context_register, pred}); + {next_start, next_end, next_handler, context_register}); current_exception_handler_++; } } @@ -1690,11 +1688,9 @@ Node* BytecodeGraphBuilder::MakeNode(const Operator* op, int value_input_count, if (!result->op()->HasProperty(Operator::kNoThrow) && inside_handler) { int handler_offset = exception_handlers_.top().handler_offset_; int context_index = exception_handlers_.top().context_register_; - CatchPrediction prediction = exception_handlers_.top().pred_; interpreter::Register context_register(context_index); - IfExceptionHint hint = prediction == CatchPrediction::CAUGHT - ? IfExceptionHint::kLocallyCaught - : IfExceptionHint::kLocallyUncaught; + // TODO(mstarzinger): Thread through correct prediction! + IfExceptionHint hint = IfExceptionHint::kLocallyCaught; Environment* success_env = environment()->CopyForConditional(); const Operator* op = common()->IfException(hint); Node* effect = environment()->GetEffectDependency(); diff --git a/src/compiler/bytecode-graph-builder.h b/src/compiler/bytecode-graph-builder.h index bf65b656dd..caa07f1425 100644 --- a/src/compiler/bytecode-graph-builder.h +++ b/src/compiler/bytecode-graph-builder.h @@ -157,9 +157,6 @@ class BytecodeGraphBuilder { // new nodes. static const int kInputBufferSizeIncrement = 64; - // The catch prediction from the handler table is reused. - typedef HandlerTable::CatchPrediction CatchPrediction; - // An abstract representation for an exception handler that is being // entered and exited while the graph builder is iterating over the // underlying bytecode. The exception handlers within the bytecode are @@ -169,7 +166,6 @@ class BytecodeGraphBuilder { int end_offset_; // End offset of the handled area in the bytecode. int handler_offset_; // Handler entry offset within the bytecode. int context_register_; // Index of register holding handler context. - CatchPrediction pred_; // Prediction of whether handler is catching. }; // Field accessors diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc index da04906d20..1d726664b4 100644 --- a/src/interpreter/bytecode-generator.cc +++ b/src/interpreter/bytecode-generator.cc @@ -301,12 +301,7 @@ class BytecodeGenerator::ControlScopeForTryCatch final public: ControlScopeForTryCatch(BytecodeGenerator* generator, TryCatchBuilder* try_catch_builder) - : ControlScope(generator) { - generator->try_catch_nesting_level_++; - } - virtual ~ControlScopeForTryCatch() { - generator()->try_catch_nesting_level_--; - } + : ControlScope(generator) {} protected: bool Execute(Command command, Statement* statement) override { @@ -333,12 +328,7 @@ class BytecodeGenerator::ControlScopeForTryFinally final DeferredCommands* commands) : ControlScope(generator), try_finally_builder_(try_finally_builder), - commands_(commands) { - generator->try_finally_nesting_level_++; - } - virtual ~ControlScopeForTryFinally() { - generator()->try_finally_nesting_level_--; - } + commands_(commands) {} protected: bool Execute(Command command, Statement* statement) override { @@ -553,9 +543,7 @@ BytecodeGenerator::BytecodeGenerator(Isolate* isolate, Zone* zone) execution_control_(nullptr), execution_context_(nullptr), execution_result_(nullptr), - register_allocator_(nullptr), - try_catch_nesting_level_(0), - try_finally_nesting_level_(0) { + register_allocator_(nullptr) { InitializeAstVisitor(isolate); } @@ -1165,7 +1153,7 @@ void BytecodeGenerator::VisitTryCatchStatement(TryCatchStatement* stmt) { void BytecodeGenerator::VisitTryFinallyStatement(TryFinallyStatement* stmt) { - TryFinallyBuilder try_control_builder(builder(), IsInsideTryCatch()); + TryFinallyBuilder try_control_builder(builder()); Register no_reg; // We keep a record of all paths that enter the finally-block to be able to diff --git a/src/interpreter/bytecode-generator.h b/src/interpreter/bytecode-generator.h index 9fd4da63da..db21c6b773 100644 --- a/src/interpreter/bytecode-generator.h +++ b/src/interpreter/bytecode-generator.h @@ -124,10 +124,6 @@ class BytecodeGenerator final : public AstVisitor { void RecordStoreToRegister(Register reg); Register LoadFromAliasedRegister(Register reg); - // Methods for tracking try-block nesting. - bool IsInsideTryCatch() const { return try_catch_nesting_level_ > 0; } - bool IsInsideTryFinally() const { return try_finally_nesting_level_ > 0; } - inline void set_builder(BytecodeArrayBuilder* builder) { builder_ = builder; } inline BytecodeArrayBuilder* builder() const { return builder_; } @@ -174,8 +170,6 @@ class BytecodeGenerator final : public AstVisitor { ContextScope* execution_context_; ExpressionResultScope* execution_result_; RegisterAllocationScope* register_allocator_; - int try_catch_nesting_level_; - int try_finally_nesting_level_; }; } // namespace interpreter diff --git a/src/interpreter/control-flow-builders.cc b/src/interpreter/control-flow-builders.cc index 6510aa443a..c30fc6d0f6 100644 --- a/src/interpreter/control-flow-builders.cc +++ b/src/interpreter/control-flow-builders.cc @@ -172,7 +172,7 @@ void TryFinallyBuilder::EndTry() { void TryFinallyBuilder::BeginHandler() { builder()->Bind(&handler_); - builder()->MarkHandler(handler_id_, will_catch_); + builder()->MarkHandler(handler_id_, false); } diff --git a/src/interpreter/control-flow-builders.h b/src/interpreter/control-flow-builders.h index e4d376b9b2..d273c2509d 100644 --- a/src/interpreter/control-flow-builders.h +++ b/src/interpreter/control-flow-builders.h @@ -165,11 +165,10 @@ class TryCatchBuilder final : public ControlFlowBuilder { // A class to help with co-ordinating control flow in try-finally statements. class TryFinallyBuilder final : public ControlFlowBuilder { public: - explicit TryFinallyBuilder(BytecodeArrayBuilder* builder, bool will_catch) + explicit TryFinallyBuilder(BytecodeArrayBuilder* builder) : ControlFlowBuilder(builder), handler_id_(builder->NewHandlerEntry()), - finalization_sites_(builder->zone()), - will_catch_(will_catch) {} + finalization_sites_(builder->zone()) {} void BeginTry(Register context); void LeaveTry(); @@ -184,11 +183,6 @@ class TryFinallyBuilder final : public ControlFlowBuilder { // Unbound labels that identify jumps to the finally block in the code. ZoneVector finalization_sites_; - - // Conservative prediction of whether exceptions thrown into the handler for - // this finally block will be caught. Note that such a prediction depends on - // whether this try-finally is nested inside a surrounding try-catch. - bool will_catch_; }; } // namespace interpreter diff --git a/src/objects-inl.h b/src/objects-inl.h index fd330074aa..71e2f02bbb 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -3411,12 +3411,6 @@ int HandlerTable::GetRangeData(int index) const { return Smi::cast(get(index * kRangeEntrySize + kRangeDataIndex))->value(); } -HandlerTable::CatchPrediction HandlerTable::GetRangePrediction( - int index) const { - return HandlerPredictionField::decode( - Smi::cast(get(index * kRangeEntrySize + kRangeHandlerIndex))->value()); -} - void HandlerTable::SetRangeStart(int index, int value) { set(index * kRangeEntrySize + kRangeStartIndex, Smi::FromInt(value)); } diff --git a/src/objects.h b/src/objects.h index 11c7620f6d..e17bc48186 100644 --- a/src/objects.h +++ b/src/objects.h @@ -4803,9 +4803,6 @@ class HandlerTable : public FixedArray { // Lookup handler in a table based on return addresses. int LookupReturn(int pc_offset, CatchPrediction* prediction); - // Returns the conservative catch predication. - inline CatchPrediction GetRangePrediction(int index) const; - // Returns the number of entries in the table. inline int NumberOfRangeEntries() const; diff --git a/test/mjsunit/compiler/debug-catch-prediction.js b/test/mjsunit/compiler/debug-catch-prediction.js deleted file mode 100644 index 34d3afd77e..0000000000 --- a/test/mjsunit/compiler/debug-catch-prediction.js +++ /dev/null @@ -1,143 +0,0 @@ -// Copyright 2016 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -// Flags: --expose-debug-as debug --allow-natives-syntax - -// Test debug event catch prediction for thrown exceptions. We distinguish -// between "caught" and "uncaught" based on the following assumptions: -// 1) try-catch : Will always catch the exception. -// 2) try-finally : Will always re-throw the exception. - -Debug = debug.Debug; - -var log = []; - -function listener(event, exec_state, event_data, data) { - try { - if (event == Debug.DebugEvent.Exception) { - log.push([event_data.exception(), event_data.uncaught()]); - } - } catch (e) { - %AbortJS(e + "\n" + e.stack); - } -} - -Debug.setBreakOnException(); -Debug.setListener(listener); - -(function TryCatch() { - log = []; // Clear log. - function f(a) { - try { - throw "boom" + a; - } catch(e) { - return e; - } - } - assertEquals("boom1", f(1)); - assertEquals("boom2", f(2)); - %OptimizeFunctionOnNextCall(f); - assertEquals("boom3", f(3)); - print("Collect log:", log); - assertEquals([["boom1",false], ["boom2",false], ["boom3",false]], log); -})(); - -(function TryFinally() { - log = []; // Clear log. - function f(a) { - try { - throw "baem" + a; - } finally { - return a + 10; - } - } - assertEquals(11, f(1)); - assertEquals(12, f(2)); - %OptimizeFunctionOnNextCall(f); - assertEquals(13, f(3)); - print("Collect log:", log); - assertEquals([["baem1",true], ["baem2",true], ["baem3",true]], log); -})(); - -(function TryCatchFinally() { - log = []; // Clear log. - function f(a) { - try { - throw "wosh" + a; - } catch(e) { - return e + a; - } finally { - // Nothing. - } - } - assertEquals("wosh11", f(1)); - assertEquals("wosh22", f(2)); - %OptimizeFunctionOnNextCall(f); - assertEquals("wosh33", f(3)); - print("Collect log:", log); - assertEquals([["wosh1",false], ["wosh2",false], ["wosh3",false]], log); -})(); - -(function TryCatchNestedFinally() { - log = []; // Clear log. - function f(a) { - try { - try { - throw "bang" + a; - } finally { - // Nothing. - } - } catch(e) { - return e + a; - } - } - assertEquals("bang11", f(1)); - assertEquals("bang22", f(2)); - %OptimizeFunctionOnNextCall(f); - assertEquals("bang33", f(3)); - print("Collect log:", log); - assertEquals([["bang1",false], ["bang2",false], ["bang3",false]], log); -})(); - -(function TryFinallyNestedCatch() { - log = []; // Clear log. - function f(a) { - try { - try { - throw "peng" + a; - } catch(e) { - return e - } - } finally { - return a + 10; - } - } - assertEquals(11, f(1)); - assertEquals(12, f(2)); - %OptimizeFunctionOnNextCall(f); - assertEquals(13, f(3)); - print("Collect log:", log); - assertEquals([["peng1",false], ["peng2",false], ["peng3",false]], log); -})(); - -(function TryFinallyNestedFinally() { - log = []; // Clear log. - function f(a) { - try { - try { - throw "oops" + a; - } finally { - // Nothing. - } - } finally { - return a + 10; - } - } - assertEquals(11, f(1)); - assertEquals(12, f(2)); - %OptimizeFunctionOnNextCall(f); - assertEquals(13, f(3)); - print("Collect log:", log); - assertEquals([["oops1",true], ["oops2",true], ["oops3",true]], log); -})();