[Turboprop] Only enable dynamic map checks for TurboProp.

The dynamic map check builtin loads the feedback vector from the
function's frame, therefore it doesn't work if we inline the
function. We don't do inlining on TurboProp so this is fine, but
it was possible to enable dynamic map checks on TurboFan which does.

This change prevents that, and also makes the dynamic map checks flag
specific to TurboProp and no longer an implication, which also allos
it to be switched on the command line independenly of --turboprop.

BUG=chromium:1141502,v8:9684

Change-Id: I365de461a6373335a45a7a154af7d4cf1c13dc2c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2494928
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Sathya Gunasekaran  <gsathya@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70737}

src/compiler/js-heap-broker.cc

@@ -4682,9 +4682,11 @@ void FilterRelevantReceiverMaps(Isolate* isolate, MapHandles* maps) {
 
 MaybeObjectHandle TryGetMinimorphicHandler(
     std::vector<MapAndHandler> const& maps_and_handlers, FeedbackSlotKind kind,
-    Handle<NativeContext> native_context) {
-  if (!FLAG_dynamic_map_checks || !IsLoadICKind(kind))
+    Handle<NativeContext> native_context, bool is_turboprop) {
+  if (!is_turboprop || !FLAG_turboprop_dynamic_map_checks ||
+      !IsLoadICKind(kind)) {
     return MaybeObjectHandle();
+  }
 
   // Don't use dynamic map checks when loading properties from Array.prototype.
   // Using dynamic map checks prevents constant folding and hence does not
@@ -4755,7 +4757,8 @@ ProcessedFeedback const& JSHeapBroker::ReadFeedbackForPropertyAccess(
   base::Optional<NameRef> name =
       static_name.has_value() ? static_name : GetNameFeedback(nexus);
   MaybeObjectHandle handler = TryGetMinimorphicHandler(
-      maps_and_handlers, kind, target_native_context().object());
+      maps_and_handlers, kind, target_native_context().object(),
+      is_turboprop());
   if (!handler.is_null()) {
     MaybeHandle<Map> maybe_map;
     if (nexus.ic_state() == MONOMORPHIC) {

src/compiler/js-native-context-specialization.cc

@@ -1066,6 +1066,16 @@ Reduction JSNativeContextSpecialization::ReduceMinimorphicPropertyAccess(
               : SerializationPolicy::kSerializeIfNeeded);
   if (access_info.IsInvalid()) return NoChange();
 
+  // The dynamic map check operator loads the feedback vector from the
+  // function's frame, so we can only use this for non-inlined functions.
+  // TODO(rmcilroy): Add support for using a trampoline like LoadICTrampoline
+  // and otherwise pass feedback vector explicitly if we need support for
+  // inlined functions.
+  // TODO(rmcilroy): Ideally we would check whether we are have an inlined frame
+  // state here, but there isn't a good way to distinguish inlined from OSR
+  // framestates.
+  DCHECK(broker()->is_turboprop());
+
   PropertyAccessBuilder access_builder(jsgraph(), broker(), nullptr);
   CheckMapsFlags flags = CheckMapsFlag::kNone;
   if (feedback.has_migration_target_maps()) {

src/flags/flag-definitions.h

@@ -556,11 +556,13 @@ DEFINE_BOOL(trace_generalization, false, "trace map generalization")
 DEFINE_BOOL(turboprop, false, "enable experimental turboprop mid-tier compiler")
 DEFINE_BOOL(turboprop_mid_tier_reg_alloc, true,
             "enable mid-tier register allocator for turboprop")
+DEFINE_BOOL(turboprop_dynamic_map_checks, true,
+            "use dynamic map checks when generating code for property accesses "
+            "if all handlers in an IC are the same for turboprop")
 DEFINE_NEG_IMPLICATION(turboprop, turbo_inlining)
 DEFINE_IMPLICATION(turboprop, concurrent_inlining)
 DEFINE_VALUE_IMPLICATION(turboprop, interrupt_budget, 15 * KB)
 DEFINE_VALUE_IMPLICATION(turboprop, reuse_opt_code_count, 2)
-DEFINE_IMPLICATION(turboprop, dynamic_map_checks)
 DEFINE_UINT_READONLY(max_minimorphic_map_checks, 4,
                      "max number of map checks to perform in minimorphic state")
 
@@ -729,9 +731,6 @@ DEFINE_BOOL(
 DEFINE_BOOL(turbo_fast_api_calls, false, "enable fast API calls from TurboFan")
 DEFINE_INT(reuse_opt_code_count, 0,
            "don't discard optimized code for the specified number of deopts.")
-DEFINE_BOOL(dynamic_map_checks, false,
-            "use dynamic map checks when generating code for property accesses "
-            "if all handlers in an IC are the same")
 
 // Native context independent (NCI) code.
 DEFINE_BOOL(turbo_nci, false,

src/runtime/runtime-test.cc

@@ -254,7 +254,7 @@ RUNTIME_FUNCTION(Runtime_IsConcurrentRecompilationSupported) {
 RUNTIME_FUNCTION(Runtime_DynamicMapChecksEnabled) {
   SealHandleScope shs(isolate);
   DCHECK_EQ(0, args.length());
-  return isolate->heap()->ToBoolean(FLAG_dynamic_map_checks);
+  return isolate->heap()->ToBoolean(FLAG_turboprop_dynamic_map_checks);
 }
 
 RUNTIME_FUNCTION(Runtime_OptimizeFunctionOnNextCall) {

test/cctest/heap/test-heap.cc

@@ -1259,7 +1259,8 @@ UNINITIALIZED_TEST(Regress10843) {
 // Tests that spill slots from optimized code don't have weak pointers.
 TEST(Regress10774) {
   i::FLAG_allow_natives_syntax = true;
-  i::FLAG_dynamic_map_checks = true;
+  i::FLAG_turboprop = true;
+  i::FLAG_turboprop_dynamic_map_checks = true;
 #ifdef VERIFY_HEAP
   i::FLAG_verify_heap = true;
 #endif

test/mjsunit/compiler/test-dynamic-map-check-deprecated-maps-polymorphic.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --turboprop --dynamic-map-checks --opt
+// Flags: --allow-natives-syntax --turboprop --turboprop-dynamic-map-checks --opt
 // Flags: --no-always-opt
 
 function f(o) {

test/mjsunit/compiler/test-dynamic-map-check-deprecated-maps.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --turboprop --dynamic-map-checks --opt
+// Flags: --allow-natives-syntax --turboprop --turboprop-dynamic-map-checks --opt
 // Flags: --no-always-opt
 
 function f(o) {

test/mjsunit/compiler/test-dynamic-map-check-deprecated-maps2.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --turboprop --dynamic-map-checks --opt
+// Flags: --allow-natives-syntax --turboprop --turboprop-dynamic-map-checks --opt
 // Flags: --no-always-opt
 
 function b(a) { return a; }

test/mjsunit/compiler/test-dynamic-map-checks-poly-mono.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --turboprop --dynamic-map-checks --opt
+// Flags: --allow-natives-syntax --turboprop --turboprop-dynamic-map-checks --opt
 // Flags: --no-always-opt
 
 function load(obj){

test/mjsunit/compiler/test-dynamic-map-checks-wrong-handler.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --turboprop --dynamic-map-checks --opt
+// Flags: --allow-natives-syntax --turboprop --turboprop-dynamic-map-checks --opt
 // Flags: --no-always-opt
 
 function load(obj){

test/mjsunit/compiler/test-dynamic-map-checks-wrong-handler1.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --turboprop --dynamic-map-checks --opt
+// Flags: --allow-natives-syntax --turboprop --turboprop-dynamic-map-checks --opt
 // Flags: --no-always-opt
 
 function load(obj){

test/mjsunit/compiler/test-dynamic-map-checks.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --turboprop --dynamic-map-checks --opt
+// Flags: --allow-natives-syntax --turboprop --turboprop-dynamic-map-checks --opt
 // Flags: --no-always-opt
 
 function load(obj){

test/mjsunit/regress/regress-1112155.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
-// Flags: --dynamic-map-checks --allow-natives-syntax --opt --no-always-opt
+// Flags: --turboprop-dynamic-map-checks --allow-natives-syntax --opt --no-always-opt
 
 function f(v) {
   return v.b;

test/mjsunit/regress/regress-1125871.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --allow-natives-syntax --dynamic-map-checks --opt --no-always-opt
+// Flags: --allow-natives-syntax --turboprop-dynamic-map-checks --opt --no-always-opt
 
 function bar(obj) {
   // Add two dummy loads to make sure obj.b is in the same slot index as obj.a

test/mjsunit/regress/regress-1141502.js

+// Copyright 2020 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.
+
+// Flags: --allow-natives-syntax --no-lazy-feedback-allocation
+// Flags: --turboprop-dynamic-map-checks
+
+function bar(a) {
+  return a.x;
+}
+
+function foo(a) {
+  return 1 * bar(a);
+}
+
+var obj = {x: 2};
+
+%PrepareFunctionForOptimization(foo);
+foo(obj, obj);
+%OptimizeFunctionOnNextCall(foo);
+assertThrows(() => foo());