[turbofan] Properly emit bounds checks for typed array element loads.

src/compiler/simplified-lowering.cc

 // Copyright 2014 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 "src/compiler/simplified-lowering.h"
 
 #include "src/base/bits.h"
 #include "src/code-factory.h"
 #include "src/compiler/common-operator.h"
 #include "src/compiler/graph-inl.h"
@@ skipping 567 matching lines @@
         SetOutput(node, 0);
         if (lower()) lowering->DoStoreField(node);
         break;
       }
       case IrOpcode::kLoadElement: {
         ElementAccess access = ElementAccessOf(node->op());
         ProcessInput(node, 0, changer_->TypeForBasePointer(access));
         ProcessInput(node, 1, kMachInt32);  // element index
         ProcessInput(node, 2, kMachInt32);  // length
         ProcessRemainingInputs(node, 3);
-        SetOutput(node, access.machine_type);
-        if (lower()) lowering->DoLoadElement(node);
+        // Tagged overrides everything if we have to do a typed array bounds
+        // check, because we may need to return undefined then.
+        MachineType output_type =
+            (access.bounds_check == kTypedArrayBoundsCheck &&
+             (use & kRepTagged))
+                ? kMachAnyTagged
+                : access.machine_type;
+        SetOutput(node, output_type);
+        if (lower()) lowering->DoLoadElement(node, output_type);
         break;
       }
       case IrOpcode::kStoreElement: {
         ElementAccess access = ElementAccessOf(node->op());
         ProcessInput(node, 0, changer_->TypeForBasePointer(access));
         ProcessInput(node, 1, kMachInt32);  // element index
         ProcessInput(node, 2, kMachInt32);  // length
         ProcessInput(node, 3, access.machine_type);
         ProcessRemainingInputs(node, 4);
         SetOutput(node, 0);
@@ skipping 259 matching lines @@
     index = graph()->NewNode(machine()->Int32Mul(),
                              jsgraph()->Int32Constant(element_size), index);
   }
   int fixed_offset = access.header_size - access.tag();
   if (fixed_offset == 0) return index;
   return graph()->NewNode(machine()->Int32Add(), index,
                           jsgraph()->Int32Constant(fixed_offset));
 }
 
 
-void SimplifiedLowering::DoLoadElement(Node* node) {
+void SimplifiedLowering::DoLoadElement(Node* node, MachineType output_type) {

[Michael Starzinger, 2014/10/08 09:25:31]

Instead of passing the output_type as an argument we can just load it from the
NodeInfo using "GetInfo(node)->output". This makes the signature simpler and
also makes sure those two values never get out of sync.

[Benedikt Meurer, 2014/10/08 10:54:58]

As discussed offline: Not possible.

   const ElementAccess& access = ElementAccessOf(node->op());
-  node->set_op(machine()->Load(access.machine_type));
-  node->ReplaceInput(1, ComputeIndex(access, node->InputAt(1)));
-  node->RemoveInput(2);
+  const Operator* op = machine()->Load(access.machine_type);
+  Node* key = node->InputAt(1);
+  Node* index = ComputeIndex(access, key);
+  if (access.bounds_check == kNoBoundsCheck) {
+    DCHECK_EQ(access.machine_type, output_type);
+    node->set_op(op);
+    node->ReplaceInput(1, index);
+    node->RemoveInput(2);
+  } else {
+    DCHECK_EQ(kTypedArrayBoundsCheck, access.bounds_check);
+
+    Node* base = node->InputAt(0);
+    Node* length = node->InputAt(2);
+    Node* effect = node->InputAt(3);
+    Node* control = node->InputAt(4);
+
+    Node* check = graph()->NewNode(machine()->Uint32LessThan(), key, length);
+    Node* branch = graph()->NewNode(common()->Branch(), check, control);
+
+    Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* load = graph()->NewNode(op, base, index, effect, if_true);
+    Node* result = load;
+    if (output_type & kRepTagged) {
+      // TODO(turbofan): This is ugly as hell!

[Michael Starzinger, 2014/10/08 09:25:31]

I concur. :)

+      SimplifiedOperatorBuilder simplified(graph()->zone());
+      RepresentationChanger changer(jsgraph(), &simplified,
+                                    graph()->zone()->isolate());
+      result = changer.GetTaggedRepresentationFor(result, access.machine_type);
+    }
+
+    Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+    Node* undefined;
+    if (output_type & kRepTagged) {
+      DCHECK(!(access.machine_type & kRepTagged));
+      undefined = jsgraph()->UndefinedConstant();
+    } else if (output_type & kRepFloat32) {
+      undefined = jsgraph()->Float32Constant(0.0f);
+    } else if (output_type & kRepFloat64) {
+      undefined = jsgraph()->Float64Constant(0.0);
+    } else {
+      undefined = jsgraph()->Int32Constant(0);
+    }
+
+    Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
+    Node* phi = graph()->NewNode(common()->EffectPhi(2), load, effect, merge);
+
+    // Replace effect uses of node with the effect phi.
+    for (UseIter i = node->uses().begin(); i != node->uses().end();) {
+      if (NodeProperties::IsEffectEdge(i.edge())) {
+        i = i.UpdateToAndIncrement(phi);
+      } else {
+        ++i;
+      }
+    }
+
+    node->set_op(common()->Phi(output_type, 2));
+    node->ReplaceInput(0, result);
+    node->ReplaceInput(1, undefined);
+    node->ReplaceInput(2, merge);
+    node->TrimInputCount(3);
+  }
 }
 
 
 void SimplifiedLowering::DoStoreElement(Node* node) {
   const ElementAccess& access = ElementAccessOf(node->op());
   const Operator* op = machine()->Store(StoreRepresentation(
       access.machine_type,
       ComputeWriteBarrierKind(access.base_is_tagged, access.machine_type,
                               access.type)));
   Node* key = node->InputAt(1);
@@ skipping 80 matching lines @@
 void SimplifiedLowering::DoStringLessThanOrEqual(Node* node) {
   node->set_op(machine()->IntLessThanOrEqual());
   node->ReplaceInput(0, StringComparison(node, true));
   node->ReplaceInput(1, jsgraph()->SmiConstant(EQUAL));
 }
 
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8