From 947740a6d8495c44bfab05094354e42f16605251 Mon Sep 17 00:00:00 2001 From: "mstarzinger@chromium.org" Date: Tue, 29 Jul 2014 13:08:51 +0000 Subject: [PATCH] Revert "Make --always-opt also optimize toplevel code." TBR=ishell@chromium.org Review URL: https://codereview.chromium.org/429583002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@22670 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/lithium-codegen-arm.cc | 6 +- src/arm64/lithium-codegen-arm64.cc | 10 +- src/factory.cc | 3 - src/hydrogen.cc | 2 +- src/ia32/lithium-codegen-ia32.cc | 6 +- src/mips/lithium-codegen-mips.cc | 6 +- src/x64/lithium-codegen-x64.cc | 6 +- test/cctest/test-api.cc | 3 +- test/cctest/test-debug.cc | 11 +-- test/cctest/test-decls.cc | 11 --- test/cctest/test-heap.cc | 99 +++++++++---------- test/cctest/test-parsing.cc | 9 +- .../debug-compile-event-newfunction.js | 3 +- test/mjsunit/mjsunit.status | 3 - 14 files changed, 63 insertions(+), 115 deletions(-) diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index 7339dfed6..92ef3aa31 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -181,11 +181,7 @@ bool LCodeGen::GeneratePrologue() { Comment(";;; Allocate local context"); bool need_write_barrier = true; // Argument to NewContext is the function, which is in r1. - if (FLAG_harmony_scoping && info()->scope()->is_global_scope()) { - __ push(r1); - __ Push(info()->scope()->GetScopeInfo()); - __ CallRuntime(Runtime::kNewGlobalContext, 2); - } else if (heap_slots <= FastNewContextStub::kMaximumSlots) { + if (heap_slots <= FastNewContextStub::kMaximumSlots) { FastNewContextStub stub(isolate(), heap_slots); __ CallStub(&stub); // Result of FastNewContextStub is always in new space. diff --git a/src/arm64/lithium-codegen-arm64.cc b/src/arm64/lithium-codegen-arm64.cc index 71dbecd95..aa6030e30 100644 --- a/src/arm64/lithium-codegen-arm64.cc +++ b/src/arm64/lithium-codegen-arm64.cc @@ -689,13 +689,7 @@ bool LCodeGen::GeneratePrologue() { Comment(";;; Allocate local context"); bool need_write_barrier = true; // Argument to NewContext is the function, which is in x1. - if (FLAG_harmony_scoping && info()->scope()->is_global_scope()) { - UseScratchRegisterScope temps(masm()); - Register scope_info = temps.AcquireX(); - __ Mov(scope_info, Operand(info()->scope()->GetScopeInfo())); - __ Push(x1, scope_info); - __ CallRuntime(Runtime::kNewGlobalContext, 2); - } else if (heap_slots <= FastNewContextStub::kMaximumSlots) { + if (heap_slots <= FastNewContextStub::kMaximumSlots) { FastNewContextStub stub(isolate(), heap_slots); __ CallStub(&stub); // Result of FastNewContextStub is always in new space. @@ -5032,6 +5026,8 @@ void LCodeGen::DoDeclareGlobals(LDeclareGlobals* instr) { Register scratch1 = x5; Register scratch2 = x6; ASSERT(instr->IsMarkedAsCall()); + + ASM_UNIMPLEMENTED_BREAK("DoDeclareGlobals"); // TODO(all): if Mov could handle object in new space then it could be used // here. __ LoadHeapObject(scratch1, instr->hydrogen()->pairs()); diff --git a/src/factory.cc b/src/factory.cc index 94da1681a..60dc0b7a8 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -5,7 +5,6 @@ #include "src/factory.h" #include "src/allocation-site-scopes.h" -#include "src/bootstrapper.h" #include "src/conversions.h" #include "src/isolate-inl.h" #include "src/macro-assembler.h" @@ -1374,8 +1373,6 @@ Handle Factory::NewFunctionFromSharedFunctionInfo( FLAG_always_opt && result->is_compiled() && !info->is_toplevel() && - // TODO(mstarzinger): Extend to optimization of builtin code. - !isolate()->bootstrapper()->IsActive() && info->allows_lazy_compilation() && !info->optimization_disabled() && !isolate()->DebuggerHasBreakPoints()) { diff --git a/src/hydrogen.cc b/src/hydrogen.cc index ddd3686b2..fe8e134d9 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -11275,7 +11275,7 @@ void HOptimizedGraphBuilder::VisitFunctionDeclaration( void HOptimizedGraphBuilder::VisitModuleDeclaration( ModuleDeclaration* declaration) { - return Bailout(kModuleDeclaration); + UNREACHABLE(); } diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index dd2f3cfda..c1881b6cb 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -256,11 +256,7 @@ bool LCodeGen::GeneratePrologue() { Comment(";;; Allocate local context"); bool need_write_barrier = true; // Argument to NewContext is the function, which is still in edi. - if (FLAG_harmony_scoping && info()->scope()->is_global_scope()) { - __ push(edi); - __ Push(info()->scope()->GetScopeInfo()); - __ CallRuntime(Runtime::kNewGlobalContext, 2); - } else if (heap_slots <= FastNewContextStub::kMaximumSlots) { + if (heap_slots <= FastNewContextStub::kMaximumSlots) { FastNewContextStub stub(isolate(), heap_slots); __ CallStub(&stub); // Result of FastNewContextStub is always in new space. diff --git a/src/mips/lithium-codegen-mips.cc b/src/mips/lithium-codegen-mips.cc index b4ea4fbd9..be130f641 100644 --- a/src/mips/lithium-codegen-mips.cc +++ b/src/mips/lithium-codegen-mips.cc @@ -200,11 +200,7 @@ bool LCodeGen::GeneratePrologue() { Comment(";;; Allocate local context"); bool need_write_barrier = true; // Argument to NewContext is the function, which is in a1. - if (FLAG_harmony_scoping && info()->scope()->is_global_scope()) { - __ push(a1); - __ Push(info()->scope()->GetScopeInfo()); - __ CallRuntime(Runtime::kNewGlobalContext, 2); - } else if (heap_slots <= FastNewContextStub::kMaximumSlots) { + if (heap_slots <= FastNewContextStub::kMaximumSlots) { FastNewContextStub stub(isolate(), heap_slots); __ CallStub(&stub); // Result of FastNewContextStub is always in new space. diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 02b55fe66..d552b3180 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -193,11 +193,7 @@ bool LCodeGen::GeneratePrologue() { Comment(";;; Allocate local context"); bool need_write_barrier = true; // Argument to NewContext is the function, which is still in rdi. - if (FLAG_harmony_scoping && info()->scope()->is_global_scope()) { - __ Push(rdi); - __ Push(info()->scope()->GetScopeInfo()); - __ CallRuntime(Runtime::kNewGlobalContext, 2); - } else if (heap_slots <= FastNewContextStub::kMaximumSlots) { + if (heap_slots <= FastNewContextStub::kMaximumSlots) { FastNewContextStub stub(isolate(), heap_slots); __ CallStub(&stub); // Result of FastNewContextStub is always in new space. diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index bef45beaf..95ec47963 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -5369,8 +5369,7 @@ void TryCatchMixedNestingCheck(v8::TryCatch* try_catch) { CHECK_EQ(0, strcmp(*v8::String::Utf8Value(message->Get()), "Uncaught Error: a")); CHECK_EQ(1, message->GetLineNumber()); - // TODO(mstarzinger): Our compilers disagree about the position. - CHECK_EQ(i::FLAG_always_opt ? 0 : 6, message->GetStartColumn()); + CHECK_EQ(6, message->GetStartColumn()); } diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc index ad22411b8..6623a3ed9 100644 --- a/test/cctest/test-debug.cc +++ b/test/cctest/test-debug.cc @@ -6624,10 +6624,6 @@ TEST(Backtrace) { v8::Debug::SetMessageHandler(BacktraceData::MessageHandler); - // TODO(mstarzinger): This doesn't work with --always-opt because we don't - // have correct source positions in optimized code. Enable once we have. - i::FLAG_always_opt = false; - const int kBufferSize = 1000; uint16_t buffer[kBufferSize]; const char* scripts_command = @@ -6966,12 +6962,13 @@ TEST(DeoptimizeDuringDebugBreak) { v8::Debug::SetDebugEventListener(DebugEventBreakDeoptimize); // Compile and run function bar which will optimize it for some flag settings. - v8::Local f = CompileFunction(&env, "function bar(){}", "bar"); - f->Call(v8::Undefined(env->GetIsolate()), 0, NULL); + v8::Script::Compile(v8::String::NewFromUtf8( + env->GetIsolate(), "function bar(){}; bar()"))->Run(); // Set debug break and call bar again. v8::Debug::DebugBreak(env->GetIsolate()); - f->Call(v8::Undefined(env->GetIsolate()), 0, NULL); + v8::Script::Compile(v8::String::NewFromUtf8(env->GetIsolate(), "bar()")) + ->Run(); CHECK(debug_event_break_deoptimize_done); diff --git a/test/cctest/test-decls.cc b/test/cctest/test-decls.cc index c900fac25..7a826c247 100644 --- a/test/cctest/test-decls.cc +++ b/test/cctest/test-decls.cc @@ -652,17 +652,6 @@ TEST(CrossScriptReferencesHarmony) { v8::Isolate* isolate = CcTest::isolate(); HandleScope scope(isolate); - // TODO(rossberg): Reparsing of top-level code does not work in the presence - // of harmony scoping and multiple scripts. This can already be reproduced - // without --always-opt by relying on OSR alone. - // - // ./d8 --harmony-scoping - // -e "'use strict'; let a = 1;" - // -e "'use strict'; let b = 2; for (var i = 0; i < 100000; ++i) b++;" - // - // For now we just disable --always-opt for this test. - i::FLAG_always_opt = false; - const char* decs[] = { "var x = 1; x", "x", "this.x", "function x() { return 1 }; x()", "x()", "this.x()", diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc index 087a7f574..0c79e6535 100644 --- a/test/cctest/test-heap.cc +++ b/test/cctest/test-heap.cc @@ -1416,21 +1416,16 @@ static int CountOptimizedUserFunctions(v8::Handle context) { TEST(TestInternalWeakLists) { - FLAG_allow_natives_syntax = true; v8::V8::Initialize(); - Isolate* isolate = CcTest::i_isolate(); - - // TODO(mstarzinger): Test should be resilient against optimization decisions. - if (i::FLAG_always_opt) return; - if (!isolate->use_crankshaft()) return; // Some flags turn Scavenge collections into Mark-sweep collections // and hence are incompatible with this test case. if (FLAG_gc_global || FLAG_stress_compaction) return; - static const int kNumTestContexts = 5; - static const int kNumTestCollections = 3; + static const int kNumTestContexts = 10; + Isolate* isolate = CcTest::i_isolate(); + Heap* heap = isolate->heap(); HandleScope scope(isolate); v8::Handle ctx[kNumTestContexts]; @@ -1443,7 +1438,9 @@ TEST(TestInternalWeakLists) { // Collect garbage that might have been created by one of the // installed extensions. isolate->compilation_cache()->Clear(); - CcTest::heap()->CollectAllGarbage(Heap::kNoGCFlags); + heap->CollectAllGarbage(Heap::kNoGCFlags); + + bool opt = (FLAG_always_opt && isolate->use_crankshaft()); CHECK_EQ(i + 1, CountNativeContexts()); @@ -1459,46 +1456,46 @@ TEST(TestInternalWeakLists) { "function f5() { };"; CompileRun(source); CHECK_EQ(0, CountOptimizedUserFunctions(ctx[i])); - CompileRun("f1(); %OptimizeFunctionOnNextCall(f1); f1()"); - CHECK_EQ(1, CountOptimizedUserFunctions(ctx[i])); - CompileRun("f2(); %OptimizeFunctionOnNextCall(f2); f2()"); - CHECK_EQ(2, CountOptimizedUserFunctions(ctx[i])); - CompileRun("f3(); %OptimizeFunctionOnNextCall(f3); f3()"); - CHECK_EQ(3, CountOptimizedUserFunctions(ctx[i])); - CompileRun("f4(); %OptimizeFunctionOnNextCall(f4); f4()"); - CHECK_EQ(4, CountOptimizedUserFunctions(ctx[i])); - CompileRun("f5(); %OptimizeFunctionOnNextCall(f5); f5()"); - CHECK_EQ(5, CountOptimizedUserFunctions(ctx[i])); + CompileRun("f1()"); + CHECK_EQ(opt ? 1 : 0, CountOptimizedUserFunctions(ctx[i])); + CompileRun("f2()"); + CHECK_EQ(opt ? 2 : 0, CountOptimizedUserFunctions(ctx[i])); + CompileRun("f3()"); + CHECK_EQ(opt ? 3 : 0, CountOptimizedUserFunctions(ctx[i])); + CompileRun("f4()"); + CHECK_EQ(opt ? 4 : 0, CountOptimizedUserFunctions(ctx[i])); + CompileRun("f5()"); + CHECK_EQ(opt ? 5 : 0, CountOptimizedUserFunctions(ctx[i])); // Remove function f1, and CompileRun("f1=null"); // Scavenge treats these references as strong. - for (int j = 0; j < kNumTestCollections; j++) { + for (int j = 0; j < 10; j++) { CcTest::heap()->CollectGarbage(NEW_SPACE); - CHECK_EQ(5, CountOptimizedUserFunctions(ctx[i])); + CHECK_EQ(opt ? 5 : 0, CountOptimizedUserFunctions(ctx[i])); } // Mark compact handles the weak references. isolate->compilation_cache()->Clear(); - CcTest::heap()->CollectAllGarbage(Heap::kNoGCFlags); - CHECK_EQ(4, CountOptimizedUserFunctions(ctx[i])); + heap->CollectAllGarbage(Heap::kNoGCFlags); + CHECK_EQ(opt ? 4 : 0, CountOptimizedUserFunctions(ctx[i])); // Get rid of f3 and f5 in the same way. CompileRun("f3=null"); - for (int j = 0; j < kNumTestCollections; j++) { + for (int j = 0; j < 10; j++) { CcTest::heap()->CollectGarbage(NEW_SPACE); - CHECK_EQ(4, CountOptimizedUserFunctions(ctx[i])); + CHECK_EQ(opt ? 4 : 0, CountOptimizedUserFunctions(ctx[i])); } CcTest::heap()->CollectAllGarbage(Heap::kNoGCFlags); - CHECK_EQ(3, CountOptimizedUserFunctions(ctx[i])); + CHECK_EQ(opt ? 3 : 0, CountOptimizedUserFunctions(ctx[i])); CompileRun("f5=null"); - for (int j = 0; j < kNumTestCollections; j++) { + for (int j = 0; j < 10; j++) { CcTest::heap()->CollectGarbage(NEW_SPACE); - CHECK_EQ(3, CountOptimizedUserFunctions(ctx[i])); + CHECK_EQ(opt ? 3 : 0, CountOptimizedUserFunctions(ctx[i])); } CcTest::heap()->CollectAllGarbage(Heap::kNoGCFlags); - CHECK_EQ(2, CountOptimizedUserFunctions(ctx[i])); + CHECK_EQ(opt ? 2 : 0, CountOptimizedUserFunctions(ctx[i])); ctx[i]->Exit(); } @@ -1515,7 +1512,7 @@ TEST(TestInternalWeakLists) { ctx[i].Clear(); // Scavenge treats these references as strong. - for (int j = 0; j < kNumTestCollections; j++) { + for (int j = 0; j < 10; j++) { CcTest::heap()->CollectGarbage(i::NEW_SPACE); CHECK_EQ(kNumTestContexts - i, CountNativeContexts()); } @@ -1569,15 +1566,10 @@ static int CountOptimizedUserFunctionsWithGC(v8::Handle context, TEST(TestInternalWeakListsTraverseWithGC) { - FLAG_allow_natives_syntax = true; v8::V8::Initialize(); Isolate* isolate = CcTest::i_isolate(); - // TODO(mstarzinger): Test should be resilient against optimization decisions. - if (i::FLAG_always_opt) return; - if (!isolate->use_crankshaft()) return; - - static const int kNumTestContexts = 5; + static const int kNumTestContexts = 10; HandleScope scope(isolate); v8::Handle ctx[kNumTestContexts]; @@ -1592,6 +1584,8 @@ TEST(TestInternalWeakListsTraverseWithGC) { CHECK_EQ(i + 1, CountNativeContextsWithGC(isolate, i / 2 + 1)); } + bool opt = (FLAG_always_opt && isolate->use_crankshaft()); + // Compile a number of functions the length of the weak list of optimized // functions both with and without GCs while iterating the list. ctx[0]->Enter(); @@ -1602,21 +1596,21 @@ TEST(TestInternalWeakListsTraverseWithGC) { "function f5() { };"; CompileRun(source); CHECK_EQ(0, CountOptimizedUserFunctions(ctx[0])); - CompileRun("f1(); %OptimizeFunctionOnNextCall(f1); f1()"); - CHECK_EQ(1, CountOptimizedUserFunctions(ctx[0])); - CHECK_EQ(1, CountOptimizedUserFunctionsWithGC(ctx[0], 1)); - CompileRun("f2(); %OptimizeFunctionOnNextCall(f2); f2()"); - CHECK_EQ(2, CountOptimizedUserFunctions(ctx[0])); - CHECK_EQ(2, CountOptimizedUserFunctionsWithGC(ctx[0], 1)); - CompileRun("f3(); %OptimizeFunctionOnNextCall(f3); f3()"); - CHECK_EQ(3, CountOptimizedUserFunctions(ctx[0])); - CHECK_EQ(3, CountOptimizedUserFunctionsWithGC(ctx[0], 1)); - CompileRun("f4(); %OptimizeFunctionOnNextCall(f4); f4()"); - CHECK_EQ(4, CountOptimizedUserFunctions(ctx[0])); - CHECK_EQ(4, CountOptimizedUserFunctionsWithGC(ctx[0], 2)); - CompileRun("f5(); %OptimizeFunctionOnNextCall(f5); f5()"); - CHECK_EQ(5, CountOptimizedUserFunctions(ctx[0])); - CHECK_EQ(5, CountOptimizedUserFunctionsWithGC(ctx[0], 4)); + CompileRun("f1()"); + CHECK_EQ(opt ? 1 : 0, CountOptimizedUserFunctions(ctx[0])); + CHECK_EQ(opt ? 1 : 0, CountOptimizedUserFunctionsWithGC(ctx[0], 1)); + CompileRun("f2()"); + CHECK_EQ(opt ? 2 : 0, CountOptimizedUserFunctions(ctx[0])); + CHECK_EQ(opt ? 2 : 0, CountOptimizedUserFunctionsWithGC(ctx[0], 1)); + CompileRun("f3()"); + CHECK_EQ(opt ? 3 : 0, CountOptimizedUserFunctions(ctx[0])); + CHECK_EQ(opt ? 3 : 0, CountOptimizedUserFunctionsWithGC(ctx[0], 1)); + CompileRun("f4()"); + CHECK_EQ(opt ? 4 : 0, CountOptimizedUserFunctions(ctx[0])); + CHECK_EQ(opt ? 4 : 0, CountOptimizedUserFunctionsWithGC(ctx[0], 2)); + CompileRun("f5()"); + CHECK_EQ(opt ? 5 : 0, CountOptimizedUserFunctions(ctx[0])); + CHECK_EQ(opt ? 5 : 0, CountOptimizedUserFunctionsWithGC(ctx[0], 4)); ctx[0]->Exit(); } @@ -4078,10 +4072,7 @@ TEST(NextCodeLinkIsWeak) { Isolate* isolate = CcTest::i_isolate(); v8::internal::Heap* heap = CcTest::heap(); - // TODO(titzer): Test should be resilient against optimization decisions. - if (i::FLAG_always_opt) return; if (!isolate->use_crankshaft()) return; - HandleScope outer_scope(heap->isolate()); Handle code; heap->CollectAllAvailableGarbage(); diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index f299b9f05..f55a625d6 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -2985,8 +2985,8 @@ namespace { int* global_use_counts = NULL; -void UseCounterCallback(v8::Isolate* isolate, - v8::Isolate::UseCounterFeature feature) { +void MockUseCounterCallback(v8::Isolate* isolate, + v8::Isolate::UseCounterFeature feature) { ++global_use_counts[feature]; } @@ -2999,13 +2999,12 @@ TEST(UseAsmUseCount) { LocalContext env; int use_counts[v8::Isolate::kUseCounterFeatureCount] = {}; global_use_counts = use_counts; - CcTest::isolate()->SetUseCounterCallback(UseCounterCallback); + CcTest::isolate()->SetUseCounterCallback(MockUseCounterCallback); CompileRun("\"use asm\";\n" "var foo = 1;\n" "\"use asm\";\n" // Only the first one counts. "function bar() { \"use asm\"; var baz = 1; }"); - // Optimizing will double-count because the source is parsed twice. - CHECK_EQ(i::FLAG_always_opt ? 4 : 2, use_counts[v8::Isolate::kUseAsm]); + CHECK_EQ(2, use_counts[v8::Isolate::kUseAsm]); } diff --git a/test/mjsunit/debug-compile-event-newfunction.js b/test/mjsunit/debug-compile-event-newfunction.js index 32e788906..fb43a87f7 100644 --- a/test/mjsunit/debug-compile-event-newfunction.js +++ b/test/mjsunit/debug-compile-event-newfunction.js @@ -61,8 +61,7 @@ function listener(event, exec_state, event_data, data) { Debug.setListener(listener); // Create a function from its body text. It will lead to an eval. -var f = new Function('arg1', 'return arg1 + 1;'); -// TODO(titzer): Assignment only needed because source positions are borked. +new Function('arg1', 'return arg1 + 1;'); assertNull(exception, "exception in listener"); diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index b755294f8..e3de741de 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -51,9 +51,6 @@ # Issue 3389: deopt_every_n_garbage_collections is unsafe 'regress/regress-2653': [SKIP], - # Issue 3475: Arrow function declaration cannot be optimized - 'harmony/arrow-functions': [SKIP], - ############################################################################## # Too slow in debug mode with --stress-opt mode. 'compiler/regress-stacktrace-methods': [PASS, ['mode == debug', SKIP]], -- 2.34.1