[test][compiler] Refactor 'inline JStoWasm calls' tests to use %ObserveNode

Modify the cctests for the inlined JS-to-Wasm calls to use the
%ObserveNode intrinsic, to verify that the JSCall node is actually
inlined . This requires a small refactoring of the %ObserveNode
implementation.

Bug: v8:11092
Change-Id: I01727143fec64c6c11c58b1b664f51daae5bfdb6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2677811
Commit-Queue: Paolo Severini <paolosev@microsoft.com>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72838}

src/codegen/optimized-compilation-info.cc

@@ -39,6 +39,10 @@ OptimizedCompilationInfo::OptimizedCompilationInfo(
 
   SetTracingFlags(shared->PassesFilter(FLAG_trace_turbo_filter));
   ConfigureFlags();
+
+  if (isolate->node_observer()) {
+    SetNodeObserver(isolate->node_observer());
+  }
 }
 
 OptimizedCompilationInfo::OptimizedCompilationInfo(

src/compiler/node-observer.cc

@@ -20,6 +20,7 @@ void ObserveNodeManager::StartObserving(Node* node, NodeObserver* observer) {
   DCHECK_NOT_NULL(observer);
   DCHECK(observations_.find(node->id()) == observations_.end());
 
+  observer->set_has_observed_changes();
   NodeObserver::Observation observation = observer->OnNodeCreated(node);
   if (observation == NodeObserver::Observation::kContinue) {
     observations_[node->id()] =

src/compiler/node-observer.h

@@ -75,6 +75,12 @@ class NodeObserver : public ZoneObject {
                                     const ObservableNodeState& old_state) {
     return Observation::kContinue;
   }
+
+  void set_has_observed_changes() { has_observed_changes_ = true; }
+  bool has_observed_changes() const { return has_observed_changes_; }
+
+ private:
+  bool has_observed_changes_ = false;
 };
 inline NodeObserver::~NodeObserver() = default;
 

src/execution/isolate.h

@@ -118,6 +118,7 @@ class Interpreter;
 }  // namespace interpreter
 
 namespace compiler {
+class NodeObserver;
 class PerIsolateCompilerCache;
 }  // namespace compiler
 
@@ -463,7 +464,8 @@ using DebugObjectCache = std::vector<Handle<HeapObject>>;
   V(bool, only_terminate_in_safe_scope, false)                                \
   V(bool, detailed_source_positions_for_profiling, FLAG_detailed_line_info)   \
   V(int, embedder_wrapper_type_index, -1)                                     \
-  V(int, embedder_wrapper_object_index, -1)
+  V(int, embedder_wrapper_object_index, -1)                                   \
+  V(compiler::NodeObserver*, node_observer, nullptr)
 
 #define THREAD_LOCAL_TOP_ACCESSOR(type, name)                         \
   inline void set_##name(type v) { thread_local_top()->name##_ = v; } \

test/cctest/BUILD.gn

@@ -89,7 +89,6 @@ v8_source_set("cctest_sources") {
     "compiler/function-tester.cc",
     "compiler/function-tester.h",
     "compiler/graph-builder-tester.h",
-    "compiler/node-observer-tester.cc",
     "compiler/node-observer-tester.h",
     "compiler/serializer-tester.cc",
     "compiler/serializer-tester.h",

test/cctest/cctest.status

@@ -196,6 +196,10 @@
   'test-streaming-compilation/SingleThreadedTestDeserializationFails': [SKIP],
   'test-streaming-compilation/AsyncTestDeserializationFails': [SKIP],
   'test-streaming-compilation/AsyncTestDeserializationBypassesCompilation': [SKIP],
+
+  # %ObserveNode tests relies on TurboFan.
+  'test-sloppy-equality/*' : [SKIP],
+  'test-js-to-wasm/*': [SKIP],
 }],  # variant == nooptimization
 
 ##############################################################################

test/cctest/compiler/node-observer-tester.cc

-// Copyright 2021 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.
-
-#include "test/cctest/compiler/node-observer-tester.h"
-
-#include "src/api/api-inl.h"
-#include "src/codegen/optimized-compilation-info.h"
-#include "src/compiler/pipeline.h"
-
-namespace v8 {
-namespace internal {
-namespace compiler {
-
-void TestWithObserveNode::OptimizeFunctionWithObserver(
-    const char* function_name, NodeObserver* observer) {
-  DCHECK_NOT_NULL(function_name);
-  DCHECK_NOT_NULL(observer);
-  Local<Function> api_function = Local<Function>::Cast(
-      CcTest::global()
-          ->Get(CcTest::isolate()->GetCurrentContext(), v8_str(function_name))
-          .ToLocalChecked());
-  Handle<JSFunction> function =
-      Handle<JSFunction>::cast(v8::Utils::OpenHandle(*api_function));
-  CHECK(function->shared().HasBytecodeArray());
-  Handle<SharedFunctionInfo> sfi(function->shared(), isolate_);
-  IsCompiledScope is_compiled_scope(sfi->is_compiled_scope(isolate_));
-  JSFunction::EnsureFeedbackVector(function, &is_compiled_scope);
-
-  OptimizedCompilationInfo compilation_info(main_zone(), isolate_, sfi,
-                                            function, CodeKind::TURBOFAN);
-  compilation_info.SetNodeObserver(observer);
-  compilation_info.ReopenHandlesInNewHandleScope(isolate_);
-  Handle<Code> code =
-      Pipeline::GenerateCodeForTesting(&compilation_info, isolate_)
-          .ToHandleChecked();
-  function->set_code(*code);
-}
-
-}  // namespace compiler
-}  // namespace internal
-}  // namespace v8

test/cctest/compiler/node-observer-tester.h

@@ -15,22 +15,28 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
-// Test TurboFan compilation using the %ObserveNode intrinsic.
-class TestWithObserveNode : public HandleAndZoneScope {
+// Helpers to test TurboFan compilation using the %ObserveNode intrinsic.
+struct ObserveNodeScope {
  public:
-  explicit TestWithObserveNode(Isolate* isolate, const char* script)
-      : isolate_(isolate), script_(script) {
+  ObserveNodeScope(Isolate* isolate, NodeObserver* node_observer)
+      : isolate_(isolate) {
     DCHECK_NOT_NULL(isolate_);
-    DCHECK_NOT_NULL(script_);
-    CompileRun(script_);
+    DCHECK_NULL(isolate_->node_observer());
+    isolate_->set_node_observer(node_observer);
   }
 
-  void OptimizeFunctionWithObserver(const char* function_name,
-                                    NodeObserver* observer);
+  ~ObserveNodeScope() {
+    DCHECK_NOT_NULL(isolate_->node_observer());
+
+    // Checks that the code wrapped by %ObserveNode() was actually compiled in
+    // the test.
+    CHECK(isolate_->node_observer()->has_observed_changes());
+
+    isolate_->set_node_observer(nullptr);
+  }
 
  private:
   Isolate* isolate_;
-  const char* script_;
 };
 
 class CreationObserver : public NodeObserver {
@@ -53,7 +59,8 @@ class ModificationObserver : public NodeObserver {
  public:
   explicit ModificationObserver(
       std::function<void(const Node*)> on_created_handler,
-      std::function<void(const Node*, const ObservableNodeState& old_state)>
+      std::function<NodeObserver::Observation(
+          const Node*, const ObservableNodeState& old_state)>
           on_changed_handler)
       : on_created_handler_(on_created_handler),
         on_changed_handler_(on_changed_handler) {
@@ -68,13 +75,13 @@ class ModificationObserver : public NodeObserver {
 
   Observation OnNodeChanged(const char* reducer_name, const Node* node,
                             const ObservableNodeState& old_state) override {
-    on_changed_handler_(node, old_state);
-    return Observation::kContinue;
+    return on_changed_handler_(node, old_state);
   }
 
  private:
   std::function<void(const Node*)> on_created_handler_;
-  std::function<void(const Node*, const ObservableNodeState& old_state)>
+  std::function<NodeObserver::Observation(const Node*,
+                                          const ObservableNodeState& old_state)>
       on_changed_handler_;
 };
 

test/cctest/compiler/test-sloppy-equality.cc

@@ -41,10 +41,13 @@ class TestSloppyEqualityFactory {
         [created_op](const Node* node) {
           CHECK_EQ(created_op, node->opcode());
         },
-        [modified_op](const Node* node, const ObservableNodeState& old_state) {
+        [modified_op](const Node* node, const ObservableNodeState& old_state)
+            -> NodeObserver::Observation {
           if (old_state.opcode() != node->opcode()) {
             CHECK_EQ(modified_op, node->opcode());
+            return NodeObserver::Observation::kStop;
           }
+          return NodeObserver::Observation::kContinue;
         });
   }
 
@@ -122,11 +125,14 @@ TEST(TestSloppyEquality) {
         << "%PrepareFunctionForOptimization(test);\n";
     for (const auto& args : c.warmup) {
       src << "test(" << args.first << ", " << args.second << ");\n"
+          << "%OptimizeFunctionOnNextCall(test);"
           << "test(" << args.first << ", " << args.second << ");\n";
     }
 
-    TestWithObserveNode tester(isolate, src.str().c_str());
-    tester.OptimizeFunctionWithObserver("test", c.observer);
+    {
+      compiler::ObserveNodeScope scope(isolate, c.observer);
+      CompileRun(src.str().c_str());
+    }
   }
 }
 

test/cctest/test-js-to-wasm.cc

@@ -8,6 +8,7 @@
 #include "src/api/api.h"
 #include "src/wasm/wasm-module-builder.h"
 #include "test/cctest/cctest.h"
+#include "test/cctest/compiler/node-observer-tester.h"
 #include "test/cctest/test-api.h"
 #include "test/common/wasm/flag-utils.h"
 #include "test/common/wasm/test-signatures.h"
@@ -48,6 +49,13 @@ struct ExportedFunction {
   FunctionSig* signature;
   std::vector<ValueType> locals;
   std::vector<uint8_t> code;
+
+  bool DoesSignatureContainI64() const {
+    for (auto type : signature->all()) {
+      if (type == wasm::kWasmI64) return true;
+    }
+    return false;
+  }
 };
 
 #define WASM_CODE(...) __VA_ARGS__
@@ -246,6 +254,8 @@ DECLARE_EXPORTED_FUNCTION_WITH_LOCALS(
                                        WASM_I32V(kDeoptLoopCount))),
              WASM_CALL_FUNCTION(0, WASM_F64_SCONVERT_I32(WASM_LOCAL_GET(0))))}))
 
+enum TestMode { kJSToWasmInliningDisabled, kJSToWasmInliningEnabled };
+
 class FastJSWasmCallTester {
  public:
   FastJSWasmCallTester()
@@ -269,6 +279,14 @@ class FastJSWasmCallTester {
     func->Emit(kExprEnd);
     builder_->AddExport(CStrVector(exported_func.name.c_str()),
                         kExternalFunction, func->func_index());
+
+    // JS-to-Wasm inlining is disabled when targeting 32 bits if the Wasm
+    // function signature contains an I64.
+#if defined(V8_TARGET_ARCH_32_BIT)
+    if (exported_func.DoesSignatureContainI64()) {
+      test_mode_ = kJSToWasmInliningDisabled;
+    }
+#endif
   }
 
   // Executes a test function that returns a value of type T.
@@ -351,9 +369,9 @@ class FastJSWasmCallTester {
         "let module = new WebAssembly.Module(buf);"
         "let instance = new WebAssembly.Instance(module, importObj);"
         "function test(value) {"
-        "  return instance.exports." +
+        "  return %ObserveNode(instance.exports." +
         exported_function_name +
-        "(value);"
+        "(value));"
         "}"
         "%PrepareFunctionForOptimization(test);"
         "test(" +
@@ -363,7 +381,8 @@ class FastJSWasmCallTester {
         "test(" +
         arg + ");";
 
-    v8::Local<v8::Value> result_value = CompileRun(js_code.c_str());
+    v8::Local<v8::Value> result_value =
+        CompileRunWithJSWasmCallNodeObserver(js_code.c_str());
     CHECK(CheckType<T>(result_value));
     T result = ConvertJSValue<T>::Get(result_value, env.local()).ToChecked();
     CHECK_EQ(result, expected_result);
@@ -399,9 +418,9 @@ class FastJSWasmCallTester {
         exported_function_name +
         ";"
         "function test() {"
-        "  return " +
+        "  return %ObserveNode(" +
         exported_function_name +
-        "(arg);"
+        "(arg));"
         "}"
         "%PrepareFunctionForOptimization(test);"
         "test();";
@@ -410,7 +429,8 @@ class FastJSWasmCallTester {
     CHECK(try_catch.HasCaught());
 
     try_catch.Reset();
-    CompileRun("%OptimizeFunctionOnNextCall(test); test();");
+    CompileRunWithJSWasmCallNodeObserver(
+        "%OptimizeFunctionOnNextCall(test); test();");
     CHECK(try_catch.HasCaught());
   }
 
@@ -557,7 +577,39 @@ class FastJSWasmCallTester {
                                          exported_function_name, args.size())
             : GetJSTestCode(WasmModuleAsJSArray(), exported_function_name,
                             args.size());
-    return CompileRun(js_code.c_str());
+    return CompileRunWithJSWasmCallNodeObserver(js_code);
+  }
+
+  v8::Local<v8::Value> CompileRunWithJSWasmCallNodeObserver(
+      const std::string& js_code) {
+    compiler::ModificationObserver js_wasm_call_observer(
+        [](const compiler::Node* node) {
+          CHECK_EQ(compiler::IrOpcode::kJSCall, node->opcode());
+        },
+        [this](const compiler::Node* node,
+               const compiler::ObservableNodeState& old_state)
+            -> compiler::NodeObserver::Observation {
+          if (old_state.opcode() != node->opcode()) {
+            CHECK_EQ(compiler::IrOpcode::kJSCall, old_state.opcode());
+
+            // JS-to-Wasm inlining is disabled when targeting 32 bits if the
+            // Wasm function signature contains an I64.
+            if (test_mode_ == kJSToWasmInliningEnabled) {
+              CHECK_EQ(compiler::IrOpcode::kJSWasmCall, node->opcode());
+            } else {
+              CHECK_EQ(compiler::IrOpcode::kCall, node->opcode());
+            }
+
+            return compiler::NodeObserver::Observation::kStop;
+          }
+          return compiler::NodeObserver::Observation::kContinue;
+        });
+
+    {
+      compiler::ObserveNodeScope scope(CcTest::i_isolate(),
+                                       &js_wasm_call_observer);
+      return CompileRun(js_code.c_str());
+    }
   }
 
   // Format the JS test code that loads and instantiates a Wasm module and
@@ -585,9 +637,9 @@ class FastJSWasmCallTester {
            wasm_exported_function_name +
            ";"
            "function test() {"
-           "  let result = " +
+           "  let result = %ObserveNode(" +
            wasm_exported_function_name + "(" + js_args +
-           ");"
+           "));"
            "  return result;"
            "}"
            "%PrepareFunctionForOptimization(test);"
@@ -652,10 +704,11 @@ class FastJSWasmCallTester {
         "  var result = 0;"
         "  for (let i = 0; i < " +
         std::to_string(kDeoptLoopCount) + " + 5; i++) {";
-    code += bigint_arg ? "    result = " + wasm_exported_function_name + "(" +
-                             js_args + "+ BigInt(b)) + BigInt(n);"
-                       : "    result = " + wasm_exported_function_name + "(" +
-                             js_args + "+ b) + n;";
+    code += bigint_arg
+                ? "    result = %ObserveNode(" + wasm_exported_function_name +
+                      "(" + js_args + " + BigInt(b))) + BigInt(n);"
+                : "    result = %ObserveNode(" + wasm_exported_function_name +
+                      "(" + js_args + " + b)) + n;";
     code +=
         "  }"
         "  return result;"
@@ -684,6 +737,7 @@ class FastJSWasmCallTester {
   AccountingAllocator allocator_;
   Zone zone_;
   WasmModuleBuilder* builder_;
+  TestMode test_mode_ = kJSToWasmInliningEnabled;
 };
 
 TEST(TestFastJSWasmCall_Nop) {