From a7857d44f43c2dd183de883a3b586766dc499dd1 Mon Sep 17 00:00:00 2001 From: Mythri A Date: Mon, 9 Nov 2020 12:26:13 +0000 Subject: [PATCH] [turboprop] Fix an incorrect DCHECK When setting optimized code on feedback vector we had a DCHECK that ensured the optimization tier is kNone or it is kMidTier and we are installing TurboFan code. While this holds usually, this fails in few corner cases like: 1. Trigger a TF concurrent compilation 2. Create a new closure with --always-opt, which triggers a TF concurrent compilation and installs optimized code. We set OptimizationTier to kTopTier 3. Optimized code gets deoptimized / GC clears the optimized code, but we haven't healed the optimized code slot / optimization tier yet. 4. Concurrent compilation finishes and tries to install optimized code but the optimization tier is still set to kTopTier. This cl fixes the DCHECK by actually checking we are not overwriting valid optimized code except for tiering up. Drive by fixes: Also print optimization tier with feedback vector and print when marking a function for optimization with --always-opt. Bug: v8:11101, v8:9684 Change-Id: Icad673ea01bb225f8b05e727a56f890af7e86514 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2520900 Commit-Queue: Mythri Alle Reviewed-by: Ross McIlroy Reviewed-by: Jakob Gruber Cr-Commit-Position: refs/heads/master@{#71047} --- src/codegen/compiler.cc | 11 +++++++++++ src/diagnostics/objects-printer.cc | 1 + src/objects/feedback-vector.cc | 7 +++++-- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/codegen/compiler.cc b/src/codegen/compiler.cc index efccdbcae9..bb51b3be1e 100644 --- a/src/codegen/compiler.cc +++ b/src/codegen/compiler.cc @@ -171,6 +171,16 @@ class CompilerTracer : public AllStatic { PrintF(scope.file(), " because --always-opt"); PrintTraceSuffix(scope); } + + static void TraceMarkForAlwaysOpt(Isolate* isolate, + Handle function) { + if (!FLAG_trace_opt) return; + CodeTracer::Scope scope(isolate->GetCodeTracer()); + PrintF(scope.file(), "[marking "); + function->ShortPrint(scope.file()); + PrintF(scope.file(), " for optimized recompilation because --always-opt"); + PrintF(scope.file(), "]\n"); + } }; } // namespace @@ -3100,6 +3110,7 @@ void Compiler::PostInstantiation(Handle function) { if (FLAG_always_opt && shared->allows_lazy_compilation() && !shared->optimization_disabled() && !function->HasAvailableOptimizedCode()) { + CompilerTracer::TraceMarkForAlwaysOpt(isolate, function); JSFunction::EnsureFeedbackVector(function, &is_compiled_scope); function->MarkForOptimization(ConcurrencyMode::kNotConcurrent); } diff --git a/src/diagnostics/objects-printer.cc b/src/diagnostics/objects-printer.cc index 9cabbd5bb1..124190e9f6 100644 --- a/src/diagnostics/objects-printer.cc +++ b/src/diagnostics/objects-printer.cc @@ -898,6 +898,7 @@ void FeedbackVector::FeedbackVectorPrint(std::ostream& os) { // NOLINT os << "\n - no optimized code"; } os << "\n - optimization marker: " << optimization_marker(); + os << "\n - optimization tier: " << optimization_tier(); os << "\n - invocation count: " << invocation_count(); os << "\n - profiler ticks: " << profiler_ticks(); diff --git a/src/objects/feedback-vector.cc b/src/objects/feedback-vector.cc index 6816abc39d..7c30e1a045 100644 --- a/src/objects/feedback-vector.cc +++ b/src/objects/feedback-vector.cc @@ -383,8 +383,11 @@ void FeedbackVector::SaturatingIncrementProfilerTicks() { void FeedbackVector::SetOptimizedCode(Handle vector, Handle code) { DCHECK(CodeKindIsOptimizedJSFunction(code->kind())); - DCHECK(vector->optimization_tier() == OptimizationTier::kNone || - (vector->optimization_tier() == OptimizationTier::kMidTier && + // We should only set optimized code only when there is no valid optimized + // code or we are tiering up. + DCHECK(!vector->has_optimized_code() || + vector->optimized_code().marked_for_deoptimization() || + (vector->optimized_code().kind() == CodeKind::TURBOPROP && code->kind() == CodeKind::TURBOFAN)); // TODO(mythria): We could see a CompileOptimized marker here either from // tests that use %OptimizeFunctionOnNextCall or because we re-mark the