[nci] Change testing mode to --turbo-nci-as-midtier

To properly test tier-up in the V8 test suite, change the test variant
previously called --turbo-nci-as-highest-tier to
--turbo-nci-as-midtier.  As a midtier (between ignition and turbofan),
all major parts of the NCI pipeline (codegen, caching inside the same
native context, tier-up) are exercised by test suite.

Bug: v8:8888
Change-Id: Ic8ee2f3e3d72768c3869f5e0b25800dd0a5f25b7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2361462
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69501}

infra/testing/builders.pyl

@@ -330,7 +330,7 @@
       {'name': 'v8testing', 'variant': 'infra_staging', 'shards': 2},
       # Native context independent code.
       {'name': 'v8testing', 'variant': 'nci'},
-      {'name': 'v8testing', 'variant': 'nci_as_highest_tier'},
+      {'name': 'v8testing', 'variant': 'nci_as_midtier'},
       # Stress sampling.
       {'name': 'mjsunit', 'variant': 'stress_sampling'},
       {'name': 'webkit', 'variant': 'stress_sampling'},
@@ -993,7 +993,7 @@
       {'name': 'v8testing', 'variant': 'infra_staging', 'shards': 2},
       # Native context independent code.
       {'name': 'v8testing', 'variant': 'nci'},
-      {'name': 'v8testing', 'variant': 'nci_as_highest_tier'},
+      {'name': 'v8testing', 'variant': 'nci_as_midtier'},
       # Stress sampling.
       {'name': 'mjsunit', 'variant': 'stress_sampling'},
       {'name': 'webkit', 'variant': 'stress_sampling'},
@@ -1025,7 +1025,7 @@
       {'name': 'v8testing', 'variant': 'infra_staging'},
       # Native context independent code.
       {'name': 'v8testing', 'variant': 'nci'},
-      {'name': 'v8testing', 'variant': 'nci_as_highest_tier'},
+      {'name': 'v8testing', 'variant': 'nci_as_midtier'},
       # Stress sampling.
       {'name': 'mjsunit', 'variant': 'stress_sampling'},
       {'name': 'webkit', 'variant': 'stress_sampling'},

src/codegen/compiler.cc

@@ -59,12 +59,11 @@ namespace internal {
 namespace {
 
 bool IsForNativeContextIndependentCachingOnly(CodeKind kind) {
+  // NCI code is only cached (and not installed on the JSFunction upon
+  // successful compilation), unless the testing-only
+  // FLAG_turbo_nci_as_midtier is enabled.
   return CodeKindIsNativeContextIndependentJSFunction(kind) &&
-         !FLAG_turbo_nci_as_highest_tier;
-}
-
-bool IsForNativeContextIndependentCachingOnly(OptimizedCompilationInfo* info) {
-  return IsForNativeContextIndependentCachingOnly(info->code_kind());
+         !FLAG_turbo_nci_as_midtier;
 }
 
 class CompilerTracer : public AllStatic {
@@ -835,12 +834,16 @@ V8_WARN_UNUSED_RESULT MaybeHandle<Code> GetCodeFromOptimizedCodeCache(
     // Caching of optimized code enabled and optimized code found.
     DCHECK(!code.marked_for_deoptimization());
     DCHECK(function->shared().is_compiled());
+    DCHECK(CodeKindIsStoredInOptimizedCodeCache(code.kind()));
+    DCHECK_IMPLIES(!osr_offset.IsNone(),
+                   code.kind() == CodeKind::OPTIMIZED_FUNCTION);
     return Handle<Code>(code, isolate);
   }
   return MaybeHandle<Code>();
 }
 
 void ClearOptimizedCodeCache(OptimizedCompilationInfo* compilation_info) {
+  DCHECK(CodeKindIsStoredInOptimizedCodeCache(compilation_info->code_kind()));
   Handle<JSFunction> function = compilation_info->closure();
   if (compilation_info->osr_offset().IsNone()) {
     Handle<FeedbackVector> vector =
@@ -851,21 +854,17 @@ void ClearOptimizedCodeCache(OptimizedCompilationInfo* compilation_info) {
 
 void InsertCodeIntoOptimizedCodeCache(
     OptimizedCompilationInfo* compilation_info) {
-  // Cached NCI code currently does not use the optimization marker field.
-  if (IsForNativeContextIndependentCachingOnly(compilation_info)) return;
+  const CodeKind kind = compilation_info->code_kind();
+  if (!CodeKindIsStoredInOptimizedCodeCache(kind)) return;
 
-  if (!CodeKindIsOptimizedJSFunction(compilation_info->code_kind())) return;
-
-  // Function context specialization folds-in the function context,
-  // so no sharing can occur.
   if (compilation_info->function_context_specializing()) {
-    // Native context specialized code is not shared, so make sure the optimized
-    // code cache is clear.
+    // Function context specialization folds-in the function context, so no
+    // sharing can occur. Make sure the optimized code cache is cleared.
     ClearOptimizedCodeCache(compilation_info);
     return;
   }
 
-  // Cache optimized context-specific code.
+  // Cache optimized code.
   Handle<Code> code = compilation_info->code();
   Handle<JSFunction> function = compilation_info->closure();
   Handle<SharedFunctionInfo> shared(function->shared(), function->GetIsolate());
@@ -876,6 +875,7 @@ void InsertCodeIntoOptimizedCodeCache(
         handle(function->feedback_vector(), function->GetIsolate());
     FeedbackVector::SetOptimizedCode(vector, code);
   } else {
+    DCHECK_EQ(kind, CodeKind::OPTIMIZED_FUNCTION);
     OSROptimizedCodeCache::AddOptimizedCode(native_context, shared, code,
                                             compilation_info->osr_offset());
   }
@@ -984,9 +984,7 @@ bool GetOptimizedCodeLater(std::unique_ptr<OptimizedCompilationJob> job,
     PrintF(" for concurrent optimization.\n");
   }
 
-  // Set the optimization marker and return a code object which checks it.
-  if (!IsForNativeContextIndependentCachingOnly(code_kind)) {
-    // Cached NCI code currently does not use the optimization marker field.
+  if (CodeKindIsStoredInOptimizedCodeCache(code_kind)) {
     function->SetOptimizationMarker(OptimizationMarker::kInOptimizationQueue);
   }
   DCHECK(function->ActiveTierIsIgnition());
@@ -1005,7 +1003,11 @@ MaybeHandle<Code> GetOptimizedCode(Handle<JSFunction> function,
 
   // Make sure we clear the optimization marker on the function so that we
   // don't try to re-optimize.
-  if (function->HasOptimizationMarker()) {
+  // If compiling for NCI caching only (which does not use the optimization
+  // marker), don't touch the marker to avoid interfering with Turbofan
+  // compilation.
+  if (CodeKindIsStoredInOptimizedCodeCache(code_kind) &&
+      function->HasOptimizationMarker()) {
     function->ClearOptimizationMarker();
   }
 
@@ -1030,7 +1032,7 @@ MaybeHandle<Code> GetOptimizedCode(Handle<JSFunction> function,
     PendingOptimizationTable::FunctionWasOptimized(isolate, function);
   }
 
-  if (!IsForNativeContextIndependentCachingOnly(code_kind)) {
+  if (CodeKindIsStoredInOptimizedCodeCache(code_kind)) {
     Handle<Code> cached_code;
     if (GetCodeFromOptimizedCodeCache(function, osr_offset)
             .ToHandle(&cached_code)) {
@@ -1858,12 +1860,14 @@ bool Compiler::CompileOptimized(Handle<JSFunction> function,
   DCHECK(!isolate->has_pending_exception());
   DCHECK(function->shared().is_compiled());
   DCHECK(function->is_compiled());
-  DCHECK_IMPLIES(function->HasOptimizationMarker(),
-                 function->IsInOptimizationQueue());
-  DCHECK_IMPLIES(function->HasOptimizationMarker(),
-                 function->ChecksOptimizationMarker());
-  DCHECK_IMPLIES(function->IsInOptimizationQueue(),
-                 mode == ConcurrencyMode::kConcurrent);
+  if (CodeKindIsStoredInOptimizedCodeCache(code_kind)) {
+    DCHECK_IMPLIES(function->HasOptimizationMarker(),
+                   function->IsInOptimizationQueue());
+    DCHECK_IMPLIES(function->HasOptimizationMarker(),
+                   function->ChecksOptimizationMarker());
+    DCHECK_IMPLIES(function->IsInOptimizationQueue(),
+                   mode == ConcurrencyMode::kConcurrent);
+  }
   return true;
 }
 
@@ -2933,8 +2937,9 @@ bool Compiler::FinalizeOptimizedCompilationJob(OptimizedCompilationJob* job,
 
   Handle<SharedFunctionInfo> shared = compilation_info->shared_info();
 
+  CodeKind code_kind = compilation_info->code_kind();
   const bool should_install_code_on_function =
-      !IsForNativeContextIndependentCachingOnly(compilation_info);
+      !IsForNativeContextIndependentCachingOnly(code_kind);
   if (should_install_code_on_function) {
     // Reset profiler ticks, function is no longer considered hot.
     compilation_info->closure()->feedback_vector().set_profiler_ticks(0);
@@ -2969,7 +2974,8 @@ bool Compiler::FinalizeOptimizedCompilationJob(OptimizedCompilationJob* job,
   CompilerTracer::TraceAbortedJob(isolate, compilation_info);
   compilation_info->closure()->set_code(shared->GetCode());
   // Clear the InOptimizationQueue marker, if it exists.
-  if (compilation_info->closure()->IsInOptimizationQueue()) {
+  if (CodeKindIsStoredInOptimizedCodeCache(code_kind) &&
+      compilation_info->closure()->IsInOptimizationQueue()) {
     compilation_info->closure()->ClearOptimizationMarker();
   }
   return CompilationJob::FAILED;

src/codegen/compiler.h

@@ -45,7 +45,7 @@ using UnoptimizedCompilationJobList =
     std::forward_list<std::unique_ptr<UnoptimizedCompilationJob>>;
 
 inline bool ShouldSpawnExtraNativeContextIndependentCompilationJob() {
-  return FLAG_turbo_nci && !FLAG_turbo_nci_as_highest_tier;
+  return FLAG_turbo_nci && !FLAG_turbo_nci_as_midtier;
 }
 
 // The V8 compiler API.

src/execution/runtime-profiler.cc

@@ -196,8 +196,8 @@ void RuntimeProfiler::MaybeOptimizeNCIFrame(JSFunction function) {
 
   if (function.shared().optimization_disabled()) return;
 
-  // Note: NCI code does not OSR except when FLAG_turbo_nci_as_highest_tier
-  // is enabled, in which case we do not tier up from NCI code.
+  // Note: We currently do not trigger OSR compilation from NCI code.
+  // TODO(jgruber,v8:8888): But we should.
 
   OptimizationReason reason =
       ShouldOptimize(function, function.shared().GetBytecodeArray());

src/flags/flag-definitions.h

@@ -683,9 +683,8 @@ DEFINE_BOOL(turbo_nci, false,
 // TODO(v8:8888): Temporary until NCI caching is implemented or
 // feedback collection is made unconditional.
 DEFINE_IMPLICATION(turbo_nci, turbo_collect_feedback_in_generic_lowering)
-DEFINE_BOOL(turbo_nci_as_highest_tier, false,
-            "replace default TF with NCI code as the highest tier for testing "
-            "purposes.")
+DEFINE_BOOL(turbo_nci_as_midtier, false,
+            "insert NCI as a midtier compiler for testing purposes.")
 DEFINE_BOOL(print_nci_code, false, "print native context independent code.")
 DEFINE_BOOL(trace_turbo_nci, false, "trace native context independent code.")
 DEFINE_BOOL(turbo_collect_feedback_in_generic_lowering, true,

src/objects/code-kind.h

@@ -77,11 +77,19 @@ inline constexpr bool CodeKindChecksOptimizationMarker(CodeKind kind) {
          kind == CodeKind::NATIVE_CONTEXT_INDEPENDENT;
 }
 
-inline CodeKind CodeKindForTopTier() {
-  return FLAG_turbo_nci_as_highest_tier ? CodeKind::NATIVE_CONTEXT_INDEPENDENT
-                                        : CodeKind::OPTIMIZED_FUNCTION;
+// The optimization marker field on the feedback vector has a dual purpose of
+// controlling the tier-up workflow, and caching the produced code object for
+// access from multiple closures. The marker is not used for all code kinds
+// though, in particular it is not used when generating NCI code for caching
+// only.
+inline constexpr bool CodeKindIsStoredInOptimizedCodeCache(CodeKind kind) {
+  return kind == CodeKind::OPTIMIZED_FUNCTION ||
+         (FLAG_turbo_nci_as_midtier &&
+          kind == CodeKind::NATIVE_CONTEXT_INDEPENDENT);
 }
 
+inline CodeKind CodeKindForTopTier() { return CodeKind::OPTIMIZED_FUNCTION; }
+
 // The dedicated CodeKindFlag enum represents all code kinds in a format
 // suitable for bit sets.
 enum class CodeKindFlag {

src/objects/js-function.cc

@@ -141,6 +141,12 @@ bool JSFunction::ActiveTierIsNCI() const {
   return highest_tier == CodeKind::NATIVE_CONTEXT_INDEPENDENT;
 }
 
+CodeKind JSFunction::NextTier() const {
+  return (FLAG_turbo_nci_as_midtier && ActiveTierIsIgnition())
+             ? CodeKind::NATIVE_CONTEXT_INDEPENDENT
+             : CodeKind::OPTIMIZED_FUNCTION;
+}
+
 bool JSFunction::HasOptimizationMarker() {
   return has_feedback_vector() && feedback_vector().has_optimization_marker();
 }

src/objects/js-function.h

@@ -114,6 +114,8 @@ class JSFunction : public JSFunctionOrBoundFunction {
   bool ActiveTierIsTurbofan() const;
   bool ActiveTierIsNCI() const;
 
+  CodeKind NextTier() const;
+
   // Tells whether or not this function checks its optimization marker in its
   // feedback vector.
   bool ChecksOptimizationMarker();

src/runtime/runtime-compiler.cc

@@ -65,7 +65,8 @@ Object CompileOptimized(Isolate* isolate, Handle<JSFunction> function,
   if (check.JsHasOverflowed(kStackSpaceRequiredForCompilation * KB)) {
     return isolate->StackOverflow();
   }
-  if (!Compiler::CompileOptimized(function, mode, CodeKindForTopTier())) {
+
+  if (!Compiler::CompileOptimized(function, mode, function->NextTier())) {
     return ReadOnlyRoots(isolate).exception();
   }
   if (ShouldSpawnExtraNativeContextIndependentCompilationJob()) {

test/cctest/cctest.status

@@ -656,7 +656,7 @@
 }],
 
 ################################################################################
-['variant == nci or variant == nci_as_highest_tier', {
+['variant == nci or variant == nci_as_midtier', {
   # Optimizes and deopts differently than TurboFan.
   'test-api/FastApiCalls': [SKIP],
   'test-cpu-profiler/Deopt*': [SKIP],
@@ -673,6 +673,6 @@
 
   # NCI code currently does not use the feedback vector's optimized code cache.
   'test-compiler/OptimizedCodeSharing1': [SKIP],
-}],  # variant == nci or variant == nci_as_highest_tier
+}],  # variant == nci or variant == nci_as_midtier
 
 ]

test/cctest/test-compiler.cc

@@ -972,7 +972,7 @@ TEST(DecideToPretenureDuringCompilation) {
   // compilation.
   if (!i::FLAG_opt || i::FLAG_always_opt || i::FLAG_minor_mc ||
       i::FLAG_stress_incremental_marking || i::FLAG_optimize_for_size ||
-      i::FLAG_turbo_nci || i::FLAG_turbo_nci_as_highest_tier) {
+      i::FLAG_turbo_nci || i::FLAG_turbo_nci_as_midtier) {
     return;
   }
 

test/debugger/debugger.status

@@ -136,11 +136,11 @@
 }], # lite_mode or variant == jitless
 
 ##############################################################################
-['variant == turboprop or variant == nci or variant == nci_as_highest_tier', {
+['variant == turboprop or variant == nci or variant == nci_as_midtier', {
   # Deopts differently than TurboFan.
   'debug/debug-optimize': [SKIP],
   'debug/debug-compile-optimized': [SKIP],
-}],  # variant == turboprop or variant == nci or variant == nci_as_highest_tier
+}],  # variant == turboprop or variant == nci or variant == nci_as_midtier
 
 ##############################################################################
 # Liftoff needs to be enabled before running these tests.

test/mjsunit/mjsunit.status

@@ -1303,7 +1303,7 @@
 }],  # variant == slow_path
 
 ################################################################################
-['variant == nci or variant == nci_as_highest_tier', {
+['variant == nci or variant == nci_as_midtier', {
   # Deopts differently than TurboFan.
   # Deoptimization support is still incomplete in general, since deopts can
   # only happen when explicitly requested by tests. NCI code objects are then
@@ -1436,6 +1436,6 @@
   'compiler/serializer-feedback-propagation-1': [SKIP],
   'compiler/serializer-feedback-propagation-2': [SKIP],
   'compiler/serializer-transition-propagation': [SKIP],
-}],  # variant == nci or variant == nci_as_highest_tier
+}],  # variant == nci or variant == nci_as_midtier
 
 ]

tools/testrunner/local/variants.py

@@ -16,7 +16,7 @@ ALL_VARIANT_FLAGS = {
   "jitless": [["--jitless"]],
   "minor_mc": [["--minor-mc"]],
   "nci": [["--turbo-nci"]],
-  "nci_as_highest_tier": [["--turbo-nci-as-highest-tier"]],
+  "nci_as_midtier": [["--turbo-nci-as-midtier"]],
   "no_lfa": [["--no-lazy-feedback-allocation"]],
   # No optimization means disable all optimizations. OptimizeFunctionOnNextCall
   # would not force optimization too. It turns into a Nop. Please see