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

Also fix an awfull bug in simplified lowering.

TEST=cctest,mjsunit/asm
R=mstarzinger@chromium.org

Review URL: https://codereview.chromium.org/640603003

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24463 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/compiler/js-typed-lowering.cc

@@ -534,7 +534,6 @@ Reduction JSTypedLowering::ReduceJSLoadProperty(Node* node) {
   Type* base_type = NodeProperties::GetBounds(base).upper;
   // TODO(mstarzinger): This lowering is not correct if:
   //   a) The typed array or it's buffer is neutered.
-  //   b) The index is out of bounds.
   if (base_type->IsConstant() && key_type->Is(Type::Integral32()) &&
       base_type->AsConstant()->Value()->IsJSTypedArray()) {
     // JSLoadProperty(typed-array, int32)

src/compiler/representation-change.h

@@ -157,7 +157,7 @@ class RepresentationChanger {
       node = jsgraph()->graph()->NewNode(op, node);
       op = machine()->TruncateFloat64ToFloat32();
     } else if (output_type & kRepFloat64) {
-      op = machine()->ChangeFloat32ToFloat64();
+      op = machine()->TruncateFloat64ToFloat32();
     } else {
       return TypeError(node, output_type, kRepFloat32);
     }

src/compiler/simplified-lowering.cc

@@ -4,6 +4,8 @@
 
 #include "src/compiler/simplified-lowering.h"
 
+#include <limits>
+
 #include "src/base/bits.h"
 #include "src/code-factory.h"
 #include "src/compiler/common-operator.h"
@@ -665,8 +667,15 @@ class RepresentationSelector {
         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: {
@@ -957,11 +966,71 @@ Node* SimplifiedLowering::ComputeIndex(const ElementAccess& access,
 }
 
 
-void SimplifiedLowering::DoLoadElement(Node* node) {
+void SimplifiedLowering::DoLoadElement(Node* node, MachineType output_type) {
   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!
+      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(std::numeric_limits<float>::quiet_NaN());
+    } else if (output_type & kRepFloat64) {
+      undefined =
+          jsgraph()->Float64Constant(std::numeric_limits<double>::quiet_NaN());
+    } 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);
+  }
 }
 
 

src/compiler/simplified-lowering.h

@@ -14,17 +14,19 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
-class SimplifiedLowering {
+class SimplifiedLowering FINAL {
  public:
   explicit SimplifiedLowering(JSGraph* jsgraph) : jsgraph_(jsgraph) {}
-  virtual ~SimplifiedLowering() {}
+  ~SimplifiedLowering() {}
 
   void LowerAllNodes();
 
   // TODO(titzer): These are exposed for direct testing. Use a friend class.
   void DoLoadField(Node* node);
   void DoStoreField(Node* node);
-  void DoLoadElement(Node* node);
+  // TODO(turbofan): The output_type can be removed once the result of the
+  // representation analysis is stored in the node bounds.
+  void DoLoadElement(Node* node, MachineType output_type);
   void DoStoreElement(Node* node);
   void DoStringAdd(Node* node);
   void DoStringEqual(Node* node);

test/cctest/compiler/test-representation-change.cc

@@ -429,6 +429,8 @@ TEST(SingleChanges) {
   CheckChange(IrOpcode::kChangeFloat64ToUint32, kRepFloat64 | kTypeUint32,
               kRepWord32);
 
+  CheckChange(IrOpcode::kTruncateFloat64ToFloat32, kRepFloat64, kRepFloat32);
+
   // Int32,Uint32 <-> Float32 require two changes.
   CheckTwoChanges(IrOpcode::kChangeInt32ToFloat64,
                   IrOpcode::kTruncateFloat64ToFloat32, kRepWord32 | kTypeInt32,

test/mjsunit/asm/float32array-outofbounds.js

+// 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.
+
+function Module(stdlib, foreign, heap) {
+  "use asm";
+  var MEM32 = new stdlib.Float32Array(heap);
+  function load(i) {
+    i = i|0;
+    i = +MEM32[i >> 2];
+    return i;
+  }
+  function store(i, v) {
+    i = i|0;
+    v = +v;
+    MEM32[i >> 2] = v;
+  }
+  return { load: load, store: store };
+}
+
+var m = Module(this, {}, new ArrayBuffer(4));
+
+m.store(0, 42.0);
+for (var i = 1; i < 64; ++i) {
+  m.store(i * 4 * 32 * 1024, i);
+}
+assertEquals(42.0, m.load(0));
+for (var i = 1; i < 64; ++i) {
+  assertEquals(NaN, m.load(i * 4 * 32 * 1024));
+}

test/mjsunit/asm/float64array-outofbounds.js

+// 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.
+
+function Module(stdlib, foreign, heap) {
+  "use asm";
+  var MEM64 = new stdlib.Float64Array(heap);
+  function load(i) {
+    i = i|0;
+    i = +MEM64[i >> 3];
+    return i;
+  }
+  function store(i, v) {
+    i = i|0;
+    v = +v;
+    MEM64[i >> 3] = v;
+  }
+  return { load: load, store: store };
+}
+
+var m = Module(this, {}, new ArrayBuffer(8));
+
+m.store(0, 3.12);
+for (var i = 1; i < 64; ++i) {
+  m.store(i * 8 * 32 * 1024, i);
+}
+assertEquals(3.12, m.load(0));
+for (var i = 1; i < 64; ++i) {
+  assertEquals(NaN, m.load(i * 8 * 32 * 1024));
+}

test/mjsunit/asm/int16array-outofbounds.js

+// 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.
+
+function Module(stdlib, foreign, heap) {
+  "use asm";
+  var MEM16 = new stdlib.Int16Array(heap);
+  function load(i) {
+    i = i|0;
+    i = MEM16[i >> 1] | 0;
+    return i;
+  }
+  function store(i, v) {
+    i = i|0;
+    v = v|0;
+    MEM16[i >> 1] = v;
+  }
+  return { load: load, store: store };
+}
+
+var m = Module(this, {}, new ArrayBuffer(2));
+
+m.store(0, 32767);
+for (var i = 1; i < 64; ++i) {
+  m.store(i * 2 * 32 * 1024, i);
+}
+assertEquals(32767, m.load(0));
+for (var i = 1; i < 64; ++i) {
+  assertEquals(0, m.load(i * 2 * 32 * 1024));
+}

test/mjsunit/asm/int32array-outofbounds.js

+// 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.
+
+function Module(stdlib, foreign, heap) {
+  "use asm";
+  var MEM32 = new stdlib.Int32Array(heap);
+  function load(i) {
+    i = i|0;
+    i = MEM32[i >> 2] | 0;
+    return i;
+  }
+  function store(i, v) {
+    i = i|0;
+    v = v|0;
+    MEM32[i >> 2] = v;
+  }
+  return { load: load, store: store };
+}
+
+var m = Module(this, {}, new ArrayBuffer(4));
+
+m.store(0, 0x12345678);
+for (var i = 1; i < 64; ++i) {
+  m.store(i * 4 * 32 * 1024, i);
+}
+assertEquals(0x12345678, m.load(0));
+for (var i = 1; i < 64; ++i) {
+  assertEquals(0, m.load(i * 4 * 32 * 1024));
+}

test/mjsunit/asm/uint8array-outofbounds.js

+// 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.
+
+function Module(stdlib, foreign, heap) {
+  "use asm";
+  var MEM8 = new stdlib.Uint8Array(heap);
+  function load(i) {
+    i = i|0;
+    i = MEM8[i] | 0;
+    return i;
+  }
+  function store(i, v) {
+    i = i|0;
+    v = v|0;
+    MEM8[i] = v;
+  }
+  return { load: load, store: store };
+}
+
+var m = Module(this, {}, new ArrayBuffer(1));
+
+m.store(0, 255);
+for (var i = 1; i < 64; ++i) {
+  m.store(i * 1 * 32 * 1024, i);
+}
+assertEquals(255, m.load(0));
+for (var i = 1; i < 64; ++i) {
+  assertEquals(0, m.load(i * 1 * 32 * 1024));
+}