[nci] Check for NCI code in the CompileLazy builtin

This CL fixes a spot (pointed out by Mythri) in which it was possible
to start running unoptimized bytecode even if cached NCI code was
present.

Previously, the CompileLazy builtin would skip the runtime and start
running bytecode immediately if the SharedFunctionInfo was already
compiled; any NCI code was ignored.

This CL changes the CompileLazy builtin to additionally check the
SFI::may_have_cached_code bit. If set, call into the (new) function
Runtime::kTryInstallNCICode to try and install NCI code.

Bug: v8:8888
Change-Id: Icbee9f0780f9b65e9339f1a958f5b28abe42c810
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2546680Reviewed-by: Mythri Alle <mythria@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71255}

src/builtins/builtins-lazy-gen.cc

@@ -123,24 +123,43 @@ void LazyBuiltinsAssembler::CompileLazy(TNode<JSFunction> function) {
   // If feedback cell isn't initialized, compile function
   GotoIf(IsUndefined(feedback_cell_value), &compile_function);
 
-  Label use_sfi_code(this);
+  Label maybe_use_sfi_code(this);
   // If there is no feedback, don't check for optimized code.
   GotoIf(HasInstanceType(feedback_cell_value, CLOSURE_FEEDBACK_CELL_ARRAY_TYPE),
-         &use_sfi_code);
+         &maybe_use_sfi_code);
 
   // If it isn't undefined or fixed array it must be a feedback vector.
   CSA_ASSERT(this, IsFeedbackVector(feedback_cell_value));
 
   // Is there an optimization marker or optimized code in the feedback vector?
   MaybeTailCallOptimizedCodeSlot(function, CAST(feedback_cell_value));
-  Goto(&use_sfi_code);
+  Goto(&maybe_use_sfi_code);
 
-  BIND(&use_sfi_code);
-  // If not, install the SFI's code entry and jump to that.
+  // At this point we have a candidate Code object. It's *not* a cached
+  // optimized Code object (we'd have tail-called it above). A usual case would
+  // be the InterpreterEntryTrampoline to start executing existing bytecode.
+  BIND(&maybe_use_sfi_code);
   CSA_ASSERT(this, TaggedNotEqual(sfi_code, HeapConstant(BUILTIN_CODE(
                                                 isolate(), CompileLazy))));
   StoreObjectField(function, JSFunction::kCodeOffset, sfi_code);
-  GenerateTailCallToJSCode(sfi_code, function);
+
+  // Finally, check for presence of an NCI cached Code object - if an entry
+  // possibly exists, call into runtime to query the cache.
+  TNode<Uint8T> flags2 =
+      LoadObjectField<Uint8T>(shared, SharedFunctionInfo::kFlags2Offset);
+  TNode<BoolT> may_have_cached_code =
+      IsSetWord32<SharedFunctionInfo::MayHaveCachedCodeBit>(flags2);
+  TNode<Code> code = Select<Code>(
+      may_have_cached_code,
+      [=]() {
+        return CAST(CallRuntime(Runtime::kTryInstallNCICode,
+                                Parameter<Context>(Descriptor::kContext),
+                                function));
+      },
+      [=]() { return sfi_code; });
+
+  // Jump to the selected code entry.
+  GenerateTailCallToJSCode(code, function);
 
   BIND(&compile_function);
   GenerateTailCallToReturnedCode(Runtime::kCompileLazy, function);

src/runtime/runtime-compiler.cc

@@ -21,54 +21,6 @@
 namespace v8 {
 namespace internal {
 
-RUNTIME_FUNCTION(Runtime_CompileLazy) {
-  HandleScope scope(isolate);
-  DCHECK_EQ(1, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
-
-  Handle<SharedFunctionInfo> sfi(function->shared(), isolate);
-
-#ifdef DEBUG
-  if (FLAG_trace_lazy && !sfi->is_compiled()) {
-    PrintF("[unoptimized: ");
-    function->PrintName();
-    PrintF("]\n");
-  }
-#endif
-
-  StackLimitCheck check(isolate);
-  if (check.JsHasOverflowed(kStackSpaceRequiredForCompilation * KB)) {
-    return isolate->StackOverflow();
-  }
-  IsCompiledScope is_compiled_scope;
-  if (!Compiler::Compile(function, Compiler::KEEP_EXCEPTION,
-                         &is_compiled_scope)) {
-    return ReadOnlyRoots(isolate).exception();
-  }
-  if (sfi->may_have_cached_code()) {
-    MaybeHandle<Code> maybe_code;
-    MaybeHandle<SerializedFeedback> maybe_feedback;
-    if (sfi->TryGetCachedCodeAndSerializedFeedback(isolate, &maybe_code,
-                                                   &maybe_feedback)) {
-      Handle<Code> code = maybe_code.ToHandleChecked();
-      if (FLAG_trace_turbo_nci) CompilationCacheCode::TraceHit(sfi, code);
-      function->set_code(*code);
-
-      if (!function->has_feedback_vector()) {
-        JSFunction::EnsureFeedbackVector(function, &is_compiled_scope);
-        // TODO(jgruber,v8:8888): Consider combining shared feedback with
-        // existing feedback here.
-        maybe_feedback.ToHandleChecked()->DeserializeInto(
-            function->feedback_vector());
-      }
-
-      return *code;
-    }
-  }
-  DCHECK(function->is_compiled());
-  return function->code();
-}
-
 namespace {
 
 // Returns false iff an exception was thrown.
@@ -117,8 +69,77 @@ Object CompileOptimized(Isolate* isolate, Handle<JSFunction> function,
   return function->code();
 }
 
+void TryInstallNCICode(Isolate* isolate, Handle<JSFunction> function,
+                       Handle<SharedFunctionInfo> sfi,
+                       IsCompiledScope* is_compiled_scope) {
+  // This function should only be called if there's a possibility that cached
+  // code exists.
+  DCHECK(sfi->may_have_cached_code());
+  DCHECK_EQ(function->shared(), *sfi);
+
+  MaybeHandle<Code> maybe_code;
+  MaybeHandle<SerializedFeedback> maybe_feedback;
+  if (sfi->TryGetCachedCodeAndSerializedFeedback(isolate, &maybe_code,
+                                                 &maybe_feedback)) {
+    Handle<Code> code = maybe_code.ToHandleChecked();
+    if (FLAG_trace_turbo_nci) CompilationCacheCode::TraceHit(sfi, code);
+    function->set_code(*code);
+
+    if (!function->has_feedback_vector()) {
+      JSFunction::EnsureFeedbackVector(function, is_compiled_scope);
+      // TODO(jgruber,v8:8888): Consider combining shared feedback with
+      // existing feedback here.
+      maybe_feedback.ToHandleChecked()->DeserializeInto(
+          function->feedback_vector());
+    }
+  }
+}
+
 }  // namespace
 
+RUNTIME_FUNCTION(Runtime_CompileLazy) {
+  HandleScope scope(isolate);
+  DCHECK_EQ(1, args.length());
+  CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
+
+  Handle<SharedFunctionInfo> sfi(function->shared(), isolate);
+
+#ifdef DEBUG
+  if (FLAG_trace_lazy && !sfi->is_compiled()) {
+    PrintF("[unoptimized: ");
+    function->PrintName();
+    PrintF("]\n");
+  }
+#endif
+
+  StackLimitCheck check(isolate);
+  if (check.JsHasOverflowed(kStackSpaceRequiredForCompilation * KB)) {
+    return isolate->StackOverflow();
+  }
+  IsCompiledScope is_compiled_scope;
+  if (!Compiler::Compile(function, Compiler::KEEP_EXCEPTION,
+                         &is_compiled_scope)) {
+    return ReadOnlyRoots(isolate).exception();
+  }
+  if (sfi->may_have_cached_code()) {
+    TryInstallNCICode(isolate, function, sfi, &is_compiled_scope);
+  }
+  DCHECK(function->is_compiled());
+  return function->code();
+}
+
+RUNTIME_FUNCTION(Runtime_TryInstallNCICode) {
+  HandleScope scope(isolate);
+  DCHECK_EQ(1, args.length());
+  CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
+  DCHECK(function->is_compiled());
+  Handle<SharedFunctionInfo> sfi(function->shared(), isolate);
+  IsCompiledScope is_compiled_scope(*sfi, isolate);
+  TryInstallNCICode(isolate, function, sfi, &is_compiled_scope);
+  DCHECK(function->is_compiled());
+  return function->code();
+}
+
 RUNTIME_FUNCTION(Runtime_CompileOptimized_Concurrent) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());

src/runtime/runtime.h

@@ -112,7 +112,8 @@ namespace internal {
   F(FunctionFirstExecution, 1, 1)         \
   F(InstantiateAsmJs, 4, 1)               \
   F(NotifyDeoptimized, 0, 1)              \
-  F(ResolvePossiblyDirectEval, 6, 1)
+  F(ResolvePossiblyDirectEval, 6, 1)      \
+  F(TryInstallNCICode, 1, 1)
 
 #define FOR_EACH_INTRINSIC_DATE(F, I) F(DateCurrentTime, 0, 1)