[nci] Don't expose feedback to compiler phases in NCI mode

Native context independent code generation should, at the moment, not use any collected feedback. We implement this by returning InsufficientFeedback from the heap broker's ReadFeedbackForX methods if currently compiling nci code. Thus all feedback.IsInsufficient() calls inside the compiler will return true (disabling feedback-based optimizations). FeedbackSource::IsValid() (used in generic lowering) can still return true. Bug: v8:8888 Change-Id: I198b6457276073e7376c777b206c50726f1b3645 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2284494 Commit-Queue: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Cr-Commit-Position: refs/heads/master@{#68726}
2020-07-07 21:34:53 +02:00 · 2020-07-07 21:34:53 +02:00 · e33e84815d
commit e33e84815d
parent 6603127440
5 changed files with 76 additions and 67 deletions
--- a/src/compiler/js-call-reducer.cc
+++ b/src/compiler/js-call-reducer.cc
@ -4495,12 +4495,8 @@ Reduction JSCallReducer::ReduceJSConstruct(Node* node) {
          node, DeoptimizeReason::kInsufficientTypeFeedbackForConstruct);
    }

-    // TODO(jgruber,v8:8888): Remove the special case for native context
-    // independent codegen below once we control available feedback through
-    // this flag.
    base::Optional<HeapObjectRef> feedback_target = feedback.AsCall().target();
-    if (feedback_target.has_value() && feedback_target->IsAllocationSite() &&
-        !broker()->is_native_context_independent()) {
+    if (feedback_target.has_value() && feedback_target->IsAllocationSite()) {
      // The feedback is an AllocationSite, which means we have called the
      // Array function and collected transition (and pretenuring) feedback
      // for the resulting arrays.  This has to be kept in sync with the
--- a/src/compiler/js-heap-broker.cc
+++ b/src/compiler/js-heap-broker.cc
@ -4689,12 +4689,23 @@ void FilterRelevantReceiverMaps(Isolate* isolate, MapHandles* maps) {
 }
 }  // namespace

+bool JSHeapBroker::CanUseFeedback(const FeedbackNexus& nexus) const {
+  // TODO(jgruber,v8:8888): Currently, nci code does not use any
+  // feedback. This restriction will be relaxed in the future.
+  return !is_native_context_independent() && !nexus.IsUninitialized();
+}
+
+const ProcessedFeedback& JSHeapBroker::NewInsufficientFeedback(
+    FeedbackSlotKind kind) const {
+  return *new (zone()) InsufficientFeedback(kind);
+}
+
 ProcessedFeedback const& JSHeapBroker::ReadFeedbackForPropertyAccess(
    FeedbackSource const& source, AccessMode mode,
    base::Optional<NameRef> static_name) {
  FeedbackNexus nexus(source.vector, source.slot);
  FeedbackSlotKind kind = nexus.kind();
-  if (nexus.IsUninitialized()) return *new (zone()) InsufficientFeedback(kind);
+  if (!CanUseFeedback(nexus)) return NewInsufficientFeedback(kind);

  MapHandles maps;
  nexus.ExtractMaps(&maps);
@ -4703,7 +4714,7 @@ ProcessedFeedback const& JSHeapBroker::ReadFeedbackForPropertyAccess(
  // If no maps were found for a non-megamorphic access, then our maps died and
  // we should soft-deopt.
  if (maps.empty() && nexus.ic_state() != MEGAMORPHIC) {
-    return *new (zone()) InsufficientFeedback(kind);
+    return NewInsufficientFeedback(kind);
  }

  base::Optional<NameRef> name =
@ -4733,9 +4744,7 @@ ProcessedFeedback const& JSHeapBroker::ReadFeedbackForGlobalAccess(
         nexus.kind() == FeedbackSlotKind::kLoadGlobalNotInsideTypeof ||
         nexus.kind() == FeedbackSlotKind::kStoreGlobalSloppy ||
         nexus.kind() == FeedbackSlotKind::kStoreGlobalStrict);
-  if (nexus.IsUninitialized()) {
-    return *new (zone()) InsufficientFeedback(nexus.kind());
-  }
+  if (!CanUseFeedback(nexus)) return NewInsufficientFeedback(nexus.kind());
  if (nexus.ic_state() != MONOMORPHIC || nexus.GetFeedback()->IsCleared()) {
    return *new (zone()) GlobalAccessFeedback(nexus.kind());
  }
@ -4777,27 +4786,37 @@ ProcessedFeedback const& JSHeapBroker::ReadFeedbackForGlobalAccess(
  return *new (zone()) GlobalAccessFeedback(cell, nexus.kind());
 }

-BinaryOperationHint JSHeapBroker::ReadFeedbackForBinaryOperation(
+ProcessedFeedback const& JSHeapBroker::ReadFeedbackForBinaryOperation(
    FeedbackSource const& source) const {
-  return FeedbackNexus(source.vector, source.slot).GetBinaryOperationFeedback();
+  FeedbackNexus nexus(source.vector, source.slot);
+  if (!CanUseFeedback(nexus)) return NewInsufficientFeedback(nexus.kind());
+  BinaryOperationHint hint = nexus.GetBinaryOperationFeedback();
+  DCHECK_NE(hint, BinaryOperationHint::kNone);  // Not uninitialized.
+  return *new (zone()) BinaryOperationFeedback(hint, nexus.kind());
 }

-CompareOperationHint JSHeapBroker::ReadFeedbackForCompareOperation(
+ProcessedFeedback const& JSHeapBroker::ReadFeedbackForCompareOperation(
    FeedbackSource const& source) const {
-  return FeedbackNexus(source.vector, source.slot)
-      .GetCompareOperationFeedback();
+  FeedbackNexus nexus(source.vector, source.slot);
+  if (!CanUseFeedback(nexus)) return NewInsufficientFeedback(nexus.kind());
+  CompareOperationHint hint = nexus.GetCompareOperationFeedback();
+  DCHECK_NE(hint, CompareOperationHint::kNone);  // Not uninitialized.
+  return *new (zone()) CompareOperationFeedback(hint, nexus.kind());
 }

-ForInHint JSHeapBroker::ReadFeedbackForForIn(
+ProcessedFeedback const& JSHeapBroker::ReadFeedbackForForIn(
    FeedbackSource const& source) const {
-  return FeedbackNexus(source.vector, source.slot).GetForInFeedback();
+  FeedbackNexus nexus(source.vector, source.slot);
+  if (!CanUseFeedback(nexus)) return NewInsufficientFeedback(nexus.kind());
+  ForInHint hint = nexus.GetForInFeedback();
+  DCHECK_NE(hint, ForInHint::kNone);  // Not uninitialized.
+  return *new (zone()) ForInFeedback(hint, nexus.kind());
 }

 ProcessedFeedback const& JSHeapBroker::ReadFeedbackForInstanceOf(
    FeedbackSource const& source) {
  FeedbackNexus nexus(source.vector, source.slot);
-  if (nexus.IsUninitialized())
-    return *new (zone()) InsufficientFeedback(nexus.kind());
+  if (!CanUseFeedback(nexus)) return NewInsufficientFeedback(nexus.kind());

  base::Optional<JSObjectRef> optional_constructor;
  {
@ -4813,9 +4832,11 @@ ProcessedFeedback const& JSHeapBroker::ReadFeedbackForInstanceOf(
 ProcessedFeedback const& JSHeapBroker::ReadFeedbackForArrayOrObjectLiteral(
    FeedbackSource const& source) {
  FeedbackNexus nexus(source.vector, source.slot);
+  if (!CanUseFeedback(nexus)) return NewInsufficientFeedback(nexus.kind());
+
  HeapObject object;
-  if (nexus.IsUninitialized() || !nexus.GetFeedback()->GetHeapObject(&object)) {
-    return *new (zone()) InsufficientFeedback(nexus.kind());
+  if (!nexus.GetFeedback()->GetHeapObject(&object)) {
+    return NewInsufficientFeedback(nexus.kind());
  }

  AllocationSiteRef site(this, handle(object, isolate()));
@ -4829,9 +4850,11 @@ ProcessedFeedback const& JSHeapBroker::ReadFeedbackForArrayOrObjectLiteral(
 ProcessedFeedback const& JSHeapBroker::ReadFeedbackForRegExpLiteral(
    FeedbackSource const& source) {
  FeedbackNexus nexus(source.vector, source.slot);
+  if (!CanUseFeedback(nexus)) return NewInsufficientFeedback(nexus.kind());
+
  HeapObject object;
-  if (nexus.IsUninitialized() || !nexus.GetFeedback()->GetHeapObject(&object)) {
-    return *new (zone()) InsufficientFeedback(nexus.kind());
+  if (!nexus.GetFeedback()->GetHeapObject(&object)) {
+    return NewInsufficientFeedback(nexus.kind());
  }

  JSRegExpRef regexp(this, handle(object, isolate()));
@ -4842,9 +4865,11 @@ ProcessedFeedback const& JSHeapBroker::ReadFeedbackForRegExpLiteral(
 ProcessedFeedback const& JSHeapBroker::ReadFeedbackForTemplateObject(
    FeedbackSource const& source) {
  FeedbackNexus nexus(source.vector, source.slot);
+  if (!CanUseFeedback(nexus)) return NewInsufficientFeedback(nexus.kind());
+
  HeapObject object;
-  if (nexus.IsUninitialized() || !nexus.GetFeedback()->GetHeapObject(&object)) {
-    return *new (zone()) InsufficientFeedback(nexus.kind());
+  if (!nexus.GetFeedback()->GetHeapObject(&object)) {
+    return NewInsufficientFeedback(nexus.kind());
  }

  JSArrayRef array(this, handle(object, isolate()));
@ -4854,8 +4879,7 @@ ProcessedFeedback const& JSHeapBroker::ReadFeedbackForTemplateObject(
 ProcessedFeedback const& JSHeapBroker::ReadFeedbackForCall(
    FeedbackSource const& source) {
  FeedbackNexus nexus(source.vector, source.slot);
-  if (nexus.IsUninitialized())
-    return *new (zone()) InsufficientFeedback(nexus.kind());
+  if (!CanUseFeedback(nexus)) return NewInsufficientFeedback(nexus.kind());

  base::Optional<HeapObjectRef> target_ref;
  {
@ -4969,49 +4993,25 @@ ProcessedFeedback const& JSHeapBroker::ProcessFeedbackForTemplateObject(
 ProcessedFeedback const& JSHeapBroker::ProcessFeedbackForBinaryOperation(
    FeedbackSource const& source) {
  if (HasFeedback(source)) return GetFeedback(source);
-  BinaryOperationHint hint = ReadFeedbackForBinaryOperation(source);
-  ProcessedFeedback const* feedback;
-  if (hint == BinaryOperationHint::kNone) {
-    feedback =
-        new (zone()) InsufficientFeedback(source.vector->GetKind(source.slot));
-  } else {
-    feedback = new (zone())
-        BinaryOperationFeedback(hint, source.vector->GetKind(source.slot));
-  }
-  SetFeedback(source, feedback);
-  return *feedback;
+  ProcessedFeedback const& feedback = ReadFeedbackForBinaryOperation(source);
+  SetFeedback(source, &feedback);
+  return feedback;
 }

 ProcessedFeedback const& JSHeapBroker::ProcessFeedbackForCompareOperation(
    FeedbackSource const& source) {
  if (HasFeedback(source)) return GetFeedback(source);
-  CompareOperationHint hint = ReadFeedbackForCompareOperation(source);
-  ProcessedFeedback const* feedback;
-  if (hint == CompareOperationHint::kNone) {
-    feedback =
-        new (zone()) InsufficientFeedback(source.vector->GetKind(source.slot));
-  } else {
-    feedback = new (zone())
-        CompareOperationFeedback(hint, source.vector->GetKind(source.slot));
-  }
-  SetFeedback(source, feedback);
-  return *feedback;
+  ProcessedFeedback const& feedback = ReadFeedbackForCompareOperation(source);
+  SetFeedback(source, &feedback);
+  return feedback;
 }

 ProcessedFeedback const& JSHeapBroker::ProcessFeedbackForForIn(
    FeedbackSource const& source) {
  if (HasFeedback(source)) return GetFeedback(source);
-  ForInHint hint = ReadFeedbackForForIn(source);
-  ProcessedFeedback const* feedback;
-  if (hint == ForInHint::kNone) {
-    feedback =
-        new (zone()) InsufficientFeedback(source.vector->GetKind(source.slot));
-  } else {
-    feedback =
-        new (zone()) ForInFeedback(hint, source.vector->GetKind(source.slot));
-  }
-  SetFeedback(source, feedback);
-  return *feedback;
+  ProcessedFeedback const& feedback = ReadFeedbackForForIn(source);
+  SetFeedback(source, &feedback);
+  return feedback;
 }

 ProcessedFeedback const& JSHeapBroker::ProcessFeedbackForPropertyAccess(
--- a/src/compiler/js-heap-broker.h
+++ b/src/compiler/js-heap-broker.h
@ -213,15 +213,20 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
  friend class ObjectRef;
  friend class ObjectData;

+  bool CanUseFeedback(const FeedbackNexus& nexus) const;
+  const ProcessedFeedback& NewInsufficientFeedback(FeedbackSlotKind kind) const;
+
  // Bottleneck FeedbackNexus access here, for storage in the broker
  // or on-the-fly usage elsewhere in the compiler.
-  ForInHint ReadFeedbackForForIn(FeedbackSource const& source) const;
-  CompareOperationHint ReadFeedbackForCompareOperation(
+  ProcessedFeedback const& ReadFeedbackForArrayOrObjectLiteral(
+      FeedbackSource const& source);
+  ProcessedFeedback const& ReadFeedbackForBinaryOperation(
      FeedbackSource const& source) const;
-  BinaryOperationHint ReadFeedbackForBinaryOperation(
-      FeedbackSource const& source) const;
-
  ProcessedFeedback const& ReadFeedbackForCall(FeedbackSource const& source);
+  ProcessedFeedback const& ReadFeedbackForCompareOperation(
+      FeedbackSource const& source) const;
+  ProcessedFeedback const& ReadFeedbackForForIn(
+      FeedbackSource const& source) const;
  ProcessedFeedback const& ReadFeedbackForGlobalAccess(
      FeedbackSource const& source);
  ProcessedFeedback const& ReadFeedbackForInstanceOf(
@ -229,8 +234,6 @@ class V8_EXPORT_PRIVATE JSHeapBroker {
  ProcessedFeedback const& ReadFeedbackForPropertyAccess(
      FeedbackSource const& source, AccessMode mode,
      base::Optional<NameRef> static_name);
-  ProcessedFeedback const& ReadFeedbackForArrayOrObjectLiteral(
-      FeedbackSource const& source);
  ProcessedFeedback const& ReadFeedbackForRegExpLiteral(
      FeedbackSource const& source);
  ProcessedFeedback const& ReadFeedbackForTemplateObject(
--- a/test/cctest/cctest.status
+++ b/test/cctest/cctest.status
@ -638,11 +638,17 @@
 ################################################################################
 ['variant == nci', {
  # Optimizes and deopts differently than TurboFan.
+  'test-api/FastApiCalls': [SKIP],
  'test-cpu-profiler/Deopt*': [SKIP],
  'test-cpu-profiler/DetailedSourcePositionAPI_Inlining': [SKIP],
+  'test-heap/CellsInOptimizedCodeAreWeak': [SKIP],
  'test-heap/EnsureAllocationSiteDependentCodesProcessed': [SKIP],
+  'test-heap/NewSpaceObjectsInOptimizedCode': [SKIP],
+  'test-heap/ObjectsInEagerlyDeoptimizedCodeAreWeak': [SKIP],
+  'test-heap/ObjectsInOptimizedCodeAreWeak': [SKIP],
  'test-heap/OptimizedPretenuring*': [SKIP],
  'test-heap-profiler/SamplingHeapProfilerPretenuredInlineAllocations': [SKIP],
+  'test-log/LogAll': [SKIP],
 }],  # variant == nci

 ]
--- a/test/mjsunit/mjsunit.status
+++ b/test/mjsunit/mjsunit.status
@ -1275,6 +1275,10 @@
  'compiler/regress-9945-*': [SKIP],
  'regress/regress-1049982-1': [SKIP],
  'regress/regress-1049982-2': [SKIP],
+  # Deopts due to different behavior in BytecodeGraphBuilder::GetForInMode.  In
+  # default TF, the ForInHint is kAny, in NCI mode kNone (because we currently
+  # don't use feedback).
+  'regress/regress-3650-3': [SKIP],
  # assertUnoptimized: assumes full turbofan pipeline.
  'allocation-site-info': [SKIP],
  'array-bounds-check-removal': [SKIP],