commit c85aa83087e7146281a95369cadf943ef78bf321
author Georg Neis <neis@chromium.org> Tue Apr 14 14:18:40 2020
committer Commit Bot <commit-bot@chromium.org> Mon Apr 20 08:53:47 2020
tree 14151b58918ad81696cd51f3a868b37ad842ff34
parent 3df19d312c35a32f40bd94fd8e79658a1ea44688

[turbofan] Harden type-based elimination of MaybeGrowFastElements

... against potential typer bugs. Instead of simply eliminating the
operation, insert an aborting bounds check instead.

Also move this transformation out of SimplifiedLowering, which is
already complex enough without doing these kinds of optimizations.
Unfortunately this will result in some missed optimization opportunities
because we may have more precise types during SimplifiedLowering. Let's
see if this makes a visible performance difference.

Change-Id: I9c16ad33104c29fdba39bf729d69ee03fc9797d6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2129633
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67222}

src/compiler/simplified-lowering.cc

diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc
index aa222af..30c0ee9 100644
--- a/src/compiler/simplified-lowering.cc
+++ b/src/compiler/simplified-lowering.cc
@@ -3580,25 +3580,12 @@
         return VisitBinop(node, UseInfo::AnyTagged(),
                           MachineRepresentation::kTaggedPointer);
       case IrOpcode::kMaybeGrowFastElements: {
-        Type const index_type = TypeOf(node->InputAt(2));
-        Type const length_type = TypeOf(node->InputAt(3));
         ProcessInput(node, 0, UseInfo::AnyTagged());         // object
         ProcessInput(node, 1, UseInfo::AnyTagged());         // elements
         ProcessInput(node, 2, UseInfo::TruncatingWord32());  // index
         ProcessInput(node, 3, UseInfo::TruncatingWord32());  // length
         ProcessRemainingInputs(node, 4);
         SetOutput(node, MachineRepresentation::kTaggedPointer);
-        if (lower()) {
-          // If the index is known to be less than the length (or if
-          // we're in dead code), we know that we don't need to grow
-          // the elements, so we can just remove this operation all
-          // together and replace it with the elements that we have
-          // on the inputs.
-          if (index_type.IsNone() || length_type.IsNone() ||
-              index_type.Max() < length_type.Min()) {
-            DeferReplacement(node, node->InputAt(1));
-          }
-        }
         return;
       }
 

src/compiler/typed-optimization.cc

diff --git a/src/compiler/typed-optimization.cc b/src/compiler/typed-optimization.cc
index 50e2a64..ce55504 100644
--- a/src/compiler/typed-optimization.cc
+++ b/src/compiler/typed-optimization.cc
@@ -38,6 +38,8 @@
   switch (node->opcode()) {
     case IrOpcode::kConvertReceiver:
       return ReduceConvertReceiver(node);
+    case IrOpcode::kMaybeGrowFastElements:
+      return ReduceMaybeGrowFastElements(node);
     case IrOpcode::kCheckHeapObject:
       return ReduceCheckHeapObject(node);
     case IrOpcode::kCheckNotTaggedHole:
@@ -159,6 +161,31 @@
   return NoChange();
 }
 
+Reduction TypedOptimization::ReduceMaybeGrowFastElements(Node* node) {
+  Node* const elements = NodeProperties::GetValueInput(node, 1);
+  Node* const index = NodeProperties::GetValueInput(node, 2);
+  Node* const length = NodeProperties::GetValueInput(node, 3);
+  Node* const effect = NodeProperties::GetEffectInput(node);
+  Node* const control = NodeProperties::GetControlInput(node);
+
+  Type const index_type = NodeProperties::GetType(index);
+  Type const length_type = NodeProperties::GetType(length);
+  CHECK(index_type.Is(Type::Unsigned31()));
+  CHECK(length_type.Is(Type::Unsigned31()));
+
+  if (!index_type.IsNone() && !length_type.IsNone() &&
+      index_type.Max() < length_type.Min()) {
+    Node* check_bounds = graph()->NewNode(
+        simplified()->CheckBounds(FeedbackSource{},
+                                  CheckBoundsParameters::kAbortOnOutOfBounds),
+        index, length, effect, control);
+    ReplaceWithValue(node, elements);
+    return Replace(check_bounds);
+  }
+
+  return NoChange();
+}
+
 Reduction TypedOptimization::ReduceCheckNotTaggedHole(Node* node) {
   Node* const input = NodeProperties::GetValueInput(node, 0);
   Type const input_type = NodeProperties::GetType(input);
@@ -285,7 +312,7 @@
       //   NumberToUint32(NumberDivide(lhs, rhs))
       //
       // and just smash the type [0...lhs.Max] on the {node},
-      // as the truncated result must be loewr than {lhs}'s maximum
+      // as the truncated result must be lower than {lhs}'s maximum
       // value (note that {rhs} cannot be less than 1 due to the
       // plain-number type constraint on the {node}).
       NodeProperties::ChangeOp(node, simplified()->NumberToUint32());

src/compiler/typed-optimization.h

diff --git a/src/compiler/typed-optimization.h b/src/compiler/typed-optimization.h
index 58efff9..2bb54f5 100644
--- a/src/compiler/typed-optimization.h
+++ b/src/compiler/typed-optimization.h
@@ -37,6 +37,7 @@
 
  private:
   Reduction ReduceConvertReceiver(Node* node);
+  Reduction ReduceMaybeGrowFastElements(Node* node);
   Reduction ReduceCheckHeapObject(Node* node);
   Reduction ReduceCheckMaps(Node* node);
   Reduction ReduceCheckNumber(Node* node);