[turbofan] Fix lowering of typed loads/stores.

Only JSLoadProperty/JSStoreProperty nodes with external typed arrays can
be lowered to LoadElement/StoreElement, because lowering of non-external
typed arrays would require a map check.

TEST=cctest,unittests,mjsunit
R=svenpanne@chromium.org

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

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

src/compiler/access-builder.cc

@@ -11,43 +11,43 @@ namespace compiler {
 
 // static
 FieldAccess AccessBuilder::ForMap() {
-  return {kTaggedBase, HeapObject::kMapOffset, Handle<Name>(), Type::Any(),
+  return {kTaggedBase, HeapObject::kMapOffset, MaybeHandle<Name>(), Type::Any(),
           kMachAnyTagged};
 }
 
 
 // static
 FieldAccess AccessBuilder::ForJSObjectProperties() {
-  return {kTaggedBase, JSObject::kPropertiesOffset, Handle<Name>(), Type::Any(),
-          kMachAnyTagged};
+  return {kTaggedBase, JSObject::kPropertiesOffset, MaybeHandle<Name>(),
+          Type::Any(), kMachAnyTagged};
 }
 
 
 // static
 FieldAccess AccessBuilder::ForJSObjectElements() {
-  return {kTaggedBase, JSObject::kElementsOffset, Handle<Name>(),
+  return {kTaggedBase, JSObject::kElementsOffset, MaybeHandle<Name>(),
           Type::Internal(), kMachAnyTagged};
 }
 
 
 // static
 FieldAccess AccessBuilder::ForJSFunctionContext() {
-  return {kTaggedBase, JSFunction::kContextOffset, Handle<Name>(),
+  return {kTaggedBase, JSFunction::kContextOffset, MaybeHandle<Name>(),
           Type::Internal(), kMachAnyTagged};
 }
 
 
 // static
 FieldAccess AccessBuilder::ForJSArrayBufferBackingStore() {
-  return {kTaggedBase, JSArrayBuffer::kBackingStoreOffset, Handle<Name>(),
+  return {kTaggedBase, JSArrayBuffer::kBackingStoreOffset, MaybeHandle<Name>(),
           Type::UntaggedPtr(), kMachPtr};
 }
 
 
 // static
 FieldAccess AccessBuilder::ForExternalArrayPointer() {
-  return {kTaggedBase, ExternalArray::kExternalPointerOffset, Handle<Name>(),
-          Type::UntaggedPtr(), kMachPtr};
+  return {kTaggedBase, ExternalArray::kExternalPointerOffset,
+          MaybeHandle<Name>(), Type::UntaggedPtr(), kMachPtr};
 }
 
 

src/compiler/js-typed-lowering.cc

@@ -533,35 +533,31 @@ Reduction JSTypedLowering::ReduceJSLoadProperty(Node* node) {
   Type* key_type = NodeProperties::GetBounds(key).upper;
   Type* base_type = NodeProperties::GetBounds(base).upper;
   // TODO(mstarzinger): This lowering is not correct if:
-  //   a) The typed array turns external (i.e. MaterializeArrayBuffer)
-  //   b) The typed array or it's buffer is neutered.
-  //   c) The index is out of bounds.
+  //   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)
-    JSTypedArray* array = JSTypedArray::cast(*base_type->AsConstant()->Value());
-    ElementsKind elements_kind = array->map()->elements_kind();
+    Handle<JSTypedArray> array =
+        Handle<JSTypedArray>::cast(base_type->AsConstant()->Value());
+    if (IsExternalArrayElementsKind(array->map()->elements_kind())) {
       ExternalArrayType type = array->type();
       uint32_t length;
       CHECK(array->length()->ToUint32(&length));
-    ElementAccess element_access;
       Node* elements = graph()->NewNode(
           simplified()->LoadField(AccessBuilder::ForJSObjectElements()), base,
-        NodeProperties::GetEffectInput(node));
-    if (IsExternalArrayElementsKind(elements_kind)) {
-      elements = graph()->NewNode(
+          graph()->start());
+      Node* pointer = graph()->NewNode(
           simplified()->LoadField(AccessBuilder::ForExternalArrayPointer()),
-          elements, NodeProperties::GetEffectInput(node));
-      element_access = AccessBuilder::ForTypedArrayElement(type, true);
-    } else {
-      DCHECK(IsFixedTypedArrayElementsKind(elements_kind));
-      element_access = AccessBuilder::ForTypedArrayElement(type, false);
+          elements, elements);
+      Node* effect = NodeProperties::GetEffectInput(node);
+      Node* control = NodeProperties::GetControlInput(node);
+      Node* load = graph()->NewNode(
+          simplified()->LoadElement(
+              AccessBuilder::ForTypedArrayElement(type, true)),
+          pointer, key, jsgraph()->Uint32Constant(length), effect, control);
+      return ReplaceEagerly(node, load);
     }
-    Node* value = graph()->NewNode(
-        simplified()->LoadElement(element_access), elements, key,
-        jsgraph()->Uint32Constant(length), NodeProperties::GetEffectInput(node),
-        NodeProperties::GetControlInput(node));
-    return ReplaceEagerly(node, value);
   }
   return NoChange();
 }
@@ -574,36 +570,32 @@ Reduction JSTypedLowering::ReduceJSStoreProperty(Node* node) {
   Type* key_type = NodeProperties::GetBounds(key).upper;
   Type* base_type = NodeProperties::GetBounds(base).upper;
   // TODO(mstarzinger): This lowering is not correct if:
-  //   a) The typed array turns external (i.e. MaterializeArrayBuffer)
-  //   b) The typed array or its buffer is neutered.
+  //   a) The typed array or its buffer is neutered.
   if (key_type->Is(Type::Integral32()) && base_type->IsConstant() &&
       base_type->AsConstant()->Value()->IsJSTypedArray()) {
     // JSStoreProperty(typed-array, int32, value)
-    JSTypedArray* array = JSTypedArray::cast(*base_type->AsConstant()->Value());
-    ElementsKind elements_kind = array->map()->elements_kind();
+    Handle<JSTypedArray> array =
+        Handle<JSTypedArray>::cast(base_type->AsConstant()->Value());
+    if (IsExternalArrayElementsKind(array->map()->elements_kind())) {
       ExternalArrayType type = array->type();
       uint32_t length;
       CHECK(array->length()->ToUint32(&length));
-    ElementAccess element_access;
       Node* elements = graph()->NewNode(
           simplified()->LoadField(AccessBuilder::ForJSObjectElements()), base,
-        NodeProperties::GetEffectInput(node));
-    if (IsExternalArrayElementsKind(elements_kind)) {
-      elements = graph()->NewNode(
+          graph()->start());
+      Node* pointer = graph()->NewNode(
           simplified()->LoadField(AccessBuilder::ForExternalArrayPointer()),
-          elements, NodeProperties::GetEffectInput(node));
-      element_access = AccessBuilder::ForTypedArrayElement(type, true);
-    } else {
-      DCHECK(IsFixedTypedArrayElementsKind(elements_kind));
-      element_access = AccessBuilder::ForTypedArrayElement(type, false);
-    }
+          elements, elements);
+      Node* effect = NodeProperties::GetEffectInput(node);
+      Node* control = NodeProperties::GetControlInput(node);
       Node* store =
-        graph()->NewNode(simplified()->StoreElement(element_access), elements,
-                         key, jsgraph()->Uint32Constant(length), value,
-                         NodeProperties::GetEffectInput(node),
-                         NodeProperties::GetControlInput(node));
+          graph()->NewNode(simplified()->StoreElement(
+                               AccessBuilder::ForTypedArrayElement(type, true)),
+                           pointer, key, jsgraph()->Uint32Constant(length),
+                           value, effect, control);
       return ReplaceEagerly(node, store);
     }
+  }
   return NoChange();
 }
 

src/compiler/simplified-operator.cc

@@ -27,6 +27,32 @@ std::ostream& operator<<(std::ostream& os, BaseTaggedness base_taggedness) {
 }
 
 
+bool operator==(FieldAccess const& lhs, FieldAccess const& rhs) {
+  return lhs.base_is_tagged == rhs.base_is_tagged && lhs.offset == rhs.offset &&
+         lhs.type == rhs.type && lhs.machine_type == rhs.machine_type;
+}
+
+
+bool operator!=(FieldAccess const& lhs, FieldAccess const& rhs) {
+  return !(lhs == rhs);
+}
+
+
+std::ostream& operator<<(std::ostream& os, FieldAccess const& access) {
+  os << "[" << access.base_is_tagged << ", " << access.offset << ", ";
+#ifdef OBJECT_PRINT
+  Handle<Name> name;
+  if (access.name.ToHandle(&name)) {
+    name->Print(os);
+    os << ", ";
+  }
+#endif
+  access.type->PrintTo(os);
+  os << ", " << access.machine_type << "]";
+  return os;
+}
+
+
 std::ostream& operator<<(std::ostream& os, BoundsCheckMode bounds_check_mode) {
   switch (bounds_check_mode) {
     case kNoBoundsCheck:

src/compiler/simplified-operator.h

@@ -38,13 +38,18 @@ std::ostream& operator<<(std::ostream&, BaseTaggedness);
 struct FieldAccess {
   BaseTaggedness base_is_tagged;  // specifies if the base pointer is tagged.
   int offset;                     // offset of the field, without tag.
-  Handle<Name> name;              // debugging only.
+  MaybeHandle<Name> name;         // debugging only.
   Type* type;                     // type of the field.
   MachineType machine_type;       // machine type of the field.
 
   int tag() const { return base_is_tagged == kTaggedBase ? kHeapObjectTag : 0; }
 };
 
+bool operator==(FieldAccess const& lhs, FieldAccess const& rhs);
+bool operator!=(FieldAccess const& lhs, FieldAccess const& rhs);
+
+std::ostream& operator<<(std::ostream&, FieldAccess const&);
+
 
 enum BoundsCheckMode { kNoBoundsCheck, kTypedArrayBoundsCheck };
 

src/factory.cc

@@ -1716,7 +1716,22 @@ Handle<JSDataView> Factory::NewJSDataView() {
 }
 
 
-static JSFunction* GetTypedArrayFun(ExternalArrayType type, Isolate* isolate) {
+namespace {
+
+ElementsKind GetExternalArrayElementsKind(ExternalArrayType type) {
+  switch (type) {
+#define TYPED_ARRAY_CASE(Type, type, TYPE, ctype, size) \
+  case kExternal##Type##Array:                          \
+    return EXTERNAL_##TYPE##_ELEMENTS;
+    TYPED_ARRAYS(TYPED_ARRAY_CASE)
+  }
+  UNREACHABLE();
+  return FIRST_EXTERNAL_ARRAY_ELEMENTS_KIND;
+#undef TYPED_ARRAY_CASE
+}
+
+
+JSFunction* GetTypedArrayFun(ExternalArrayType type, Isolate* isolate) {
   Context* native_context = isolate->context()->native_context();
   switch (type) {
 #define TYPED_ARRAY_FUN(Type, type, TYPE, ctype, size)                        \
@@ -1732,6 +1747,8 @@ static JSFunction* GetTypedArrayFun(ExternalArrayType type, Isolate* isolate) {
   }
 }
 
+}  // namespace
+
 
 Handle<JSTypedArray> Factory::NewJSTypedArray(ExternalArrayType type) {
   Handle<JSFunction> typed_array_fun_handle(GetTypedArrayFun(type, isolate()));
@@ -1743,6 +1760,28 @@ Handle<JSTypedArray> Factory::NewJSTypedArray(ExternalArrayType type) {
 }
 
 
+Handle<JSTypedArray> Factory::NewJSTypedArray(ExternalArrayType type,
+                                              Handle<JSArrayBuffer> buffer,
+                                              size_t length) {
+  DCHECK(length <= static_cast<size_t>(kMaxInt));
+  Handle<JSTypedArray> array = NewJSTypedArray(type);
+  array->set_buffer(*buffer);
+  array->set_weak_next(buffer->weak_first_view());
+  buffer->set_weak_first_view(*array);
+  array->set_byte_offset(Smi::FromInt(0));
+  array->set_byte_length(buffer->byte_length());
+  Handle<Object> length_handle = NewNumberFromSize(length);
+  array->set_length(*length_handle);
+  Handle<ExternalArray> elements =
+      NewExternalArray(static_cast<int>(length), type, buffer->backing_store());
+  JSObject::SetMapAndElements(array,
+                              JSObject::GetElementsTransitionMap(
+                                  array, GetExternalArrayElementsKind(type)),
+                              elements);
+  return array;
+}
+
+
 Handle<JSProxy> Factory::NewJSProxy(Handle<Object> handler,
                                     Handle<Object> prototype) {
   // Allocate map.

src/factory.h

@@ -434,6 +434,11 @@ class Factory FINAL {
 
   Handle<JSTypedArray> NewJSTypedArray(ExternalArrayType type);
 
+  // Creates a new JSTypedArray with the specified buffer.
+  Handle<JSTypedArray> NewJSTypedArray(ExternalArrayType type,
+                                       Handle<JSArrayBuffer> buffer,
+                                       size_t length);
+
   Handle<JSDataView> NewJSDataView();
 
   // Allocates a Harmony proxy.

src/handles.h

@@ -54,7 +54,7 @@ class MaybeHandle {
 
   // Convert to a Handle with a type that can be upcasted to.
   template <class S>
-  INLINE(bool ToHandle(Handle<S>* out)) {
+  V8_INLINE bool ToHandle(Handle<S>* out) const {
     if (location_ == NULL) {
       *out = Handle<T>::null();
       return false;

test/unittests/compiler/graph-unittest.h

@@ -15,12 +15,19 @@ namespace v8 {
 namespace internal {
 
 // Forward declarations.
+template <class T>
+class Handle;
 class HeapObject;
 template <class T>
 class Unique;
 
 namespace compiler {
 
+// Forward declarations.
+struct ElementAccess;
+struct FieldAccess;
+
+
 using ::testing::Matcher;
 
 
@@ -36,9 +43,11 @@ class GraphTest : public TestWithContext, public TestWithZone {
   Node* Int32Constant(int32_t value);
   Node* Int64Constant(int64_t value);
   Node* NumberConstant(volatile double value);
+  Node* HeapConstant(const Handle<HeapObject>& value);
   Node* HeapConstant(const Unique<HeapObject>& value);
   Node* FalseConstant();
   Node* TrueConstant();
+  Node* UndefinedConstant();
 
   Matcher<Node*> IsFalseConstant();
   Matcher<Node*> IsTrueConstant();
@@ -88,6 +97,22 @@ Matcher<Node*> IsNumberLessThan(const Matcher<Node*>& lhs_matcher,
                                 const Matcher<Node*>& rhs_matcher);
 Matcher<Node*> IsNumberSubtract(const Matcher<Node*>& lhs_matcher,
                                 const Matcher<Node*>& rhs_matcher);
+Matcher<Node*> IsLoadField(const Matcher<FieldAccess>& access_matcher,
+                           const Matcher<Node*>& base_matcher,
+                           const Matcher<Node*>& effect_matcher);
+Matcher<Node*> IsLoadElement(const Matcher<ElementAccess>& access_matcher,
+                             const Matcher<Node*>& base_matcher,
+                             const Matcher<Node*>& index_matcher,
+                             const Matcher<Node*>& length_matcher,
+                             const Matcher<Node*>& effect_matcher,
+                             const Matcher<Node*>& control_matcher);
+Matcher<Node*> IsStoreElement(const Matcher<ElementAccess>& access_matcher,
+                              const Matcher<Node*>& base_matcher,
+                              const Matcher<Node*>& index_matcher,
+                              const Matcher<Node*>& length_matcher,
+                              const Matcher<Node*>& value_matcher,
+                              const Matcher<Node*>& effect_matcher,
+                              const Matcher<Node*>& control_matcher);
 
 Matcher<Node*> IsLoad(const Matcher<LoadRepresentation>& rep_matcher,
                       const Matcher<Node*>& base_matcher,

test/unittests/compiler/js-builtin-reducer-unittest.cc

@@ -35,11 +35,6 @@ class JSBuiltinReducerTest : public GraphTest {
     return n;
   }
 
-  Node* UndefinedConstant() {
-    return HeapConstant(
-        Unique<HeapObject>::CreateImmovable(factory()->undefined_value()));
-  }
-
   JSOperatorBuilder* javascript() { return &javascript_; }
 
  private:

test/unittests/compiler/js-typed-lowering-unittest.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/access-builder.h"
+#include "src/compiler/js-graph.h"
+#include "src/compiler/js-operator.h"
+#include "src/compiler/js-typed-lowering.h"
+#include "src/compiler/machine-operator.h"
+#include "src/compiler/node-properties-inl.h"
+#include "src/compiler/typer.h"
+#include "test/unittests/compiler/compiler-test-utils.h"
+#include "test/unittests/compiler/graph-unittest.h"
+#include "testing/gmock-support.h"
+
+using testing::_;
+using testing::AllOf;
+using testing::Capture;
+using testing::CaptureEq;
+
+namespace v8 {
+namespace internal {
+namespace compiler {
+
+namespace {
+
+const ExternalArrayType kExternalArrayTypes[] = {
+#define TYPED_ARRAY_CASE(Type, type, TYPE, ctype, size) kExternal##Type##Array,
+    TYPED_ARRAYS(TYPED_ARRAY_CASE)
+#undef TYPED_ARRAY_CASE
+};
+
+
+const StrictMode kStrictModes[] = {SLOPPY, STRICT};
+
+}  // namespace
+
+
+class JSTypedLoweringTest : public GraphTest {
+ public:
+  JSTypedLoweringTest() : GraphTest(3), javascript_(zone()) {}
+  virtual ~JSTypedLoweringTest() {}
+
+ protected:
+  Reduction Reduce(Node* node) {
+    Typer typer(zone());
+    MachineOperatorBuilder machine;
+    JSGraph jsgraph(graph(), common(), javascript(), &typer, &machine);
+    JSTypedLowering reducer(&jsgraph);
+    return reducer.Reduce(node);
+  }
+
+  Node* Parameter(Type* type, int index = 0) {
+    Node* node = graph()->NewNode(common()->Parameter(index), graph()->start());
+    NodeProperties::SetBounds(node, Bounds(Type::None(), type));
+    return node;
+  }
+
+  Handle<JSArrayBuffer> NewArrayBuffer(void* bytes, size_t byte_length) {
+    Handle<JSArrayBuffer> buffer = factory()->NewJSArrayBuffer();
+    Runtime::SetupArrayBuffer(isolate(), buffer, true, bytes, byte_length);
+    return buffer;
+  }
+
+  JSOperatorBuilder* javascript() { return &javascript_; }
+
+ private:
+  JSOperatorBuilder javascript_;
+};
+
+
+// -----------------------------------------------------------------------------
+// JSLoadProperty
+
+
+TEST_F(JSTypedLoweringTest, JSLoadPropertyFromExternalTypedArray) {
+  const size_t kLength = 17;
+  uint8_t backing_store[kLength * 8];
+  Handle<JSArrayBuffer> buffer =
+      NewArrayBuffer(backing_store, arraysize(backing_store));
+  TRACED_FOREACH(ExternalArrayType, type, kExternalArrayTypes) {
+    Handle<JSTypedArray> array =
+        factory()->NewJSTypedArray(type, buffer, kLength);
+
+    Node* key = Parameter(Type::Integral32());
+    Node* base = HeapConstant(array);
+    Node* context = UndefinedConstant();
+    Node* effect = graph()->start();
+    Node* control = graph()->start();
+    Node* node =
+        graph()->NewNode(javascript()->LoadProperty(), base, key, context);
+    if (FLAG_turbo_deoptimization) {
+      node->AppendInput(zone(), UndefinedConstant());
+    }
+    node->AppendInput(zone(), effect);
+    node->AppendInput(zone(), control);
+    Reduction r = Reduce(node);
+
+    Capture<Node*> elements;
+    ASSERT_TRUE(r.Changed());
+    EXPECT_THAT(
+        r.replacement(),
+        IsLoadElement(
+            AccessBuilder::ForTypedArrayElement(type, true),
+            IsLoadField(AccessBuilder::ForExternalArrayPointer(),
+                        AllOf(CaptureEq(&elements),
+                              IsLoadField(AccessBuilder::ForJSObjectElements(),
+                                          base, _)),
+                        CaptureEq(&elements)),
+            key, IsInt32Constant(static_cast<int>(kLength)), effect, control));
+  }
+}
+
+
+// -----------------------------------------------------------------------------
+// JSStoreProperty
+
+
+TEST_F(JSTypedLoweringTest, JSStorePropertyToExternalTypedArray) {
+  const size_t kLength = 17;
+  uint8_t backing_store[kLength * 8];
+  Handle<JSArrayBuffer> buffer =
+      NewArrayBuffer(backing_store, arraysize(backing_store));
+  TRACED_FOREACH(ExternalArrayType, type, kExternalArrayTypes) {
+    TRACED_FOREACH(StrictMode, strict_mode, kStrictModes) {
+      Handle<JSTypedArray> array =
+          factory()->NewJSTypedArray(type, buffer, kLength);
+
+      Node* key = Parameter(Type::Integral32());
+      Node* base = HeapConstant(array);
+      Node* value = Parameter(Type::Any());
+      Node* context = UndefinedConstant();
+      Node* effect = graph()->start();
+      Node* control = graph()->start();
+      Node* node = graph()->NewNode(javascript()->StoreProperty(strict_mode),
+                                    base, key, value, context);
+      if (FLAG_turbo_deoptimization) {
+        node->AppendInput(zone(), UndefinedConstant());
+      }
+      node->AppendInput(zone(), effect);
+      node->AppendInput(zone(), control);
+      Reduction r = Reduce(node);
+
+      Capture<Node*> elements;
+      ASSERT_TRUE(r.Changed());
+      EXPECT_THAT(
+          r.replacement(),
+          IsStoreElement(
+              AccessBuilder::ForTypedArrayElement(type, true),
+              IsLoadField(
+                  AccessBuilder::ForExternalArrayPointer(),
+                  AllOf(CaptureEq(&elements),
+                        IsLoadField(AccessBuilder::ForJSObjectElements(), base,
+                                    _)),
+                  CaptureEq(&elements)),
+              key, IsInt32Constant(static_cast<int>(kLength)), value, effect,
+              control));
+    }
+  }
+}
+
+}  // namespace compiler
+}  // namespace internal
+}  // namespace v8

test/unittests/test-utils.cc

@@ -10,6 +10,32 @@
 
 namespace v8 {
 
+std::ostream& operator<<(std::ostream& os, ExternalArrayType type) {
+  switch (type) {
+    case kExternalInt8Array:
+      return os << "ExternalInt8Array";
+    case kExternalUint8Array:
+      return os << "ExternalUint8Array";
+    case kExternalInt16Array:
+      return os << "ExternalInt16Array";
+    case kExternalUint16Array:
+      return os << "ExternalUint16Array";
+    case kExternalInt32Array:
+      return os << "ExternalInt32Array";
+    case kExternalUint32Array:
+      return os << "ExternalUint32Array";
+    case kExternalFloat32Array:
+      return os << "ExternalFloat32Array";
+    case kExternalFloat64Array:
+      return os << "ExternalFloat64Array";
+    case kExternalUint8ClampedArray:
+      return os << "ExternalUint8ClampedArray";
+  }
+  UNREACHABLE();
+  return os;
+}
+
+
 // static
 Isolate* TestWithIsolate::isolate_ = NULL;
 

test/unittests/test-utils.h

@@ -13,6 +13,9 @@
 
 namespace v8 {
 
+std::ostream& operator<<(std::ostream&, ExternalArrayType);
+
+
 class TestWithIsolate : public ::testing::Test {
  public:
   TestWithIsolate();

test/unittests/unittests.gyp

@@ -45,6 +45,7 @@
         'compiler/instruction-selector-unittest.h',
         'compiler/js-builtin-reducer-unittest.cc',
         'compiler/js-operator-unittest.cc',
+        'compiler/js-typed-lowering-unittest.cc',
         'compiler/machine-operator-reducer-unittest.cc',
         'compiler/machine-operator-unittest.cc',
         'compiler/simplified-operator-reducer-unittest.cc',