[turbofan] Insert appropriate conversions for typed array stores.
authorjarin <jarin@chromium.org>
Tue, 25 Nov 2014 08:40:18 +0000 (00:40 -0800)
committerCommit bot <commit-bot@chromium.org>
Tue, 25 Nov 2014 08:40:29 +0000 (08:40 +0000)
BUG=

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

Cr-Commit-Position: refs/heads/master@{#25496}

src/compiler/js-typed-lowering.cc
test/mjsunit/compiler/truncating-store.js
test/mjsunit/mjsunit.status
test/unittests/compiler/js-typed-lowering-unittest.cc
test/unittests/compiler/node-test-utils.cc
test/unittests/compiler/node-test-utils.h

index 745aafd..0813136 100644 (file)
@@ -725,10 +725,36 @@ Reduction JSTypedLowering::ReduceJSStoreProperty(Node* node) {
         Node* length = jsgraph()->Constant(array->length()->Number());
         Node* effect = NodeProperties::GetEffectInput(node);
         Node* control = NodeProperties::GetControlInput(node);
-        Node* store = graph()->NewNode(
-            simplified()->StoreElement(
-                AccessBuilder::ForTypedArrayElement(type, true)),
-            pointer, key, length, value, effect, control);
+
+        ElementAccess access = AccessBuilder::ForTypedArrayElement(type, true);
+        Type* value_type = NodeProperties::GetBounds(value).upper;
+        // If the value input does not have the required type, insert the
+        // appropriate conversion.
+
+        // Convert to a number first.
+        if (!value_type->Is(Type::Number())) {
+          Reduction number_reduction = ReduceJSToNumberInput(value);
+          if (number_reduction.Changed()) {
+            value = number_reduction.replacement();
+          } else {
+            Node* context = NodeProperties::GetContextInput(node);
+            value = graph()->NewNode(javascript()->ToNumber(), value, context,
+                                     effect, control);
+            effect = value;
+          }
+        }
+        // For integer-typed arrays, convert to the integer type.
+        if (access.type->Is(Type::Signed32()) &&
+            !value_type->Is(Type::Signed32())) {
+          value = graph()->NewNode(simplified()->NumberToInt32(), value);
+        } else if (access.type->Is(Type::Unsigned32()) &&
+                   !value_type->Is(Type::Unsigned32())) {
+          value = graph()->NewNode(simplified()->NumberToUint32(), value);
+        }
+
+        Node* store =
+            graph()->NewNode(simplified()->StoreElement(access), pointer, key,
+                             length, value, effect, control);
         return ReplaceEagerly(node, store);
       }
     }
index 2af9e3b..9e3dd38 100644 (file)
@@ -2,72 +2,97 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-var asm = (function Module(global, env, buffer) {
-  "use asm";
-
-  var i8 = new global.Int8Array(buffer);
-  var u8 = new global.Uint8Array(buffer);
-  var i16 = new global.Int16Array(buffer);
-  var u16 = new global.Uint16Array(buffer);
-  var i32 = new global.Int32Array(buffer);
-  var u32 = new global.Uint32Array(buffer);
-
-  var H = 0;
-
-  function store_i8() {
-    H = 4294967295;
-    i8[0 >> 0]= H;
-    return i8[0 >> 0];
-  }
-
-  function store_u8() {
-    H = 4294967295;
-    u8[0 >> 0]= H;
-    return u8[0 >> 0];
-  }
-
-  function store_i16() {
-    H = 4294967295;
-    i16[0 >> 0]= H;
-    return i16[0 >> 0];
-  }
-
-  function store_u16() {
-    H = 4294967295;
-    u16[0 >> 0]= H;
-    return u16[0 >> 0];
-  }
-
-  function store_i32() {
-    H = 4294967295;
-    i32[0 >> 0]= H;
-    return i32[0 >> 0];
-  }
-
-  function store_u32() {
-    H = 4294967295;
-    u32[0 >> 0]= H;
-    return u32[0 >> 0];
-  }
-
-  return { store_i8: store_i8,
-           store_u8: store_u8,
-           store_i16: store_i16,
-           store_u16: store_u16,
-           store_i32: store_i32,
-           store_u32: store_u32 };
-})({
-  "Int8Array": Int8Array,
-  "Uint8Array": Uint8Array,
-  "Int16Array": Int16Array,
-  "Uint16Array": Uint16Array,
-  "Int32Array": Int32Array,
-  "Uint32Array": Uint32Array
-}, {}, new ArrayBuffer(64 * 1024));
-
-assertEquals(-1, asm.store_i8());
-assertEquals(255, asm.store_u8());
-assertEquals(-1, asm.store_i16());
-assertEquals(65535, asm.store_u16());
-assertEquals(-1, asm.store_i32());
-assertEquals(4294967295, asm.store_u32());
+(function() {
+  var asm = (function Module(global, env, buffer) {
+    "use asm";
+
+    var i8 = new global.Int8Array(buffer);
+    var u8 = new global.Uint8Array(buffer);
+    var i16 = new global.Int16Array(buffer);
+    var u16 = new global.Uint16Array(buffer);
+    var i32 = new global.Int32Array(buffer);
+    var u32 = new global.Uint32Array(buffer);
+
+    var H = 0;
+
+    function store_i8() {
+      H = 4294967295;
+      i8[0 >> 0]= H;
+      return i8[0 >> 0];
+    }
+
+    function store_u8() {
+      H = 4294967295;
+      u8[0 >> 0]= H;
+      return u8[0 >> 0];
+    }
+
+    function store_i16() {
+      H = 4294967295;
+      i16[0 >> 0]= H;
+      return i16[0 >> 0];
+    }
+
+    function store_u16() {
+      H = 4294967295;
+      u16[0 >> 0]= H;
+      return u16[0 >> 0];
+    }
+
+    function store_i32() {
+      H = 4294967295;
+      i32[0 >> 0]= H;
+      return i32[0 >> 0];
+    }
+
+    function store_u32() {
+      H = 4294967295;
+      u32[0 >> 0]= H;
+      return u32[0 >> 0];
+    }
+
+    return { store_i8: store_i8,
+             store_u8: store_u8,
+             store_i16: store_i16,
+             store_u16: store_u16,
+             store_i32: store_i32,
+             store_u32: store_u32 };
+  })({
+    "Int8Array": Int8Array,
+    "Uint8Array": Uint8Array,
+    "Int16Array": Int16Array,
+    "Uint16Array": Uint16Array,
+    "Int32Array": Int32Array,
+    "Uint32Array": Uint32Array
+  }, {}, new ArrayBuffer(64 * 1024));
+
+  assertEquals(-1, asm.store_i8());
+  assertEquals(255, asm.store_u8());
+  assertEquals(-1, asm.store_i16());
+  assertEquals(65535, asm.store_u16());
+  assertEquals(-1, asm.store_i32());
+  assertEquals(4294967295, asm.store_u32());
+})();
+
+(function() {
+  var asm = (function Module(global, env, buffer) {
+    "use asm";
+
+    var i32 = new global.Int32Array(buffer);
+
+    var H = 0;
+
+    // This is not valid asm.js, but we should still generate correct code.
+    function store_i32_from_string() {
+      H = "3";
+      i32[0 >> 0]= H;
+      return i32[0 >> 0];
+    }
+
+    return { store_i32_from_string: store_i32_from_string };
+  })({
+    "Int32Array": Int32Array
+  }, {}, new ArrayBuffer(64 * 1024));
+
+  assertEquals(3, asm.store_i32_from_string());
+})();
index f6f68f5..1f6313e 100644 (file)
@@ -54,9 +54,6 @@
   ##############################################################################
   # TurboFan compiler failures.
 
-  # TODO(jarin) We should truncate instead of change for some stores.
-  'compiler/truncating-store': [PASS, FAIL],
-
   # TODO(mstarzinger): An arguments object materialized in the prologue can't
   # be accessed indirectly. Either we drop that requirement or wait for support
   # from the deoptimizer to do that.
index 18db348..4d56393 100644 (file)
@@ -285,7 +285,8 @@ TEST_F(JSTypedLoweringTest, JSStorePropertyToExternalTypedArray) {
 
       Node* key = Parameter(Type::Integral32());
       Node* base = HeapConstant(array);
-      Node* value = Parameter(Type::Any());
+      Node* value =
+          Parameter(AccessBuilder::ForTypedArrayElement(type, true).type);
       Node* context = UndefinedConstant();
       Node* effect = graph()->start();
       Node* control = graph()->start();
@@ -309,6 +310,54 @@ TEST_F(JSTypedLoweringTest, JSStorePropertyToExternalTypedArray) {
   }
 }
 
+
+TEST_F(JSTypedLoweringTest, JSStorePropertyToExternalTypedArrayWithConversion) {
+  const size_t kLength = 17;
+  double backing_store[kLength];
+  Handle<JSArrayBuffer> buffer =
+      NewArrayBuffer(backing_store, sizeof(backing_store));
+  TRACED_FOREACH(ExternalArrayType, type, kExternalArrayTypes) {
+    TRACED_FOREACH(StrictMode, strict_mode, kStrictModes) {
+      Handle<JSTypedArray> array =
+          factory()->NewJSTypedArray(type, buffer, 0, 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);
+
+      Matcher<Node*> value_matcher =
+          IsToNumber(value, context, effect, control);
+      Matcher<Node*> effect_matcher = value_matcher;
+      if (AccessBuilder::ForTypedArrayElement(type, true)
+              .type->Is(Type::Signed32())) {
+        value_matcher = IsNumberToInt32(value_matcher);
+      } else if (AccessBuilder::ForTypedArrayElement(type, true)
+                     .type->Is(Type::Unsigned32())) {
+        value_matcher = IsNumberToUint32(value_matcher);
+      }
+
+      ASSERT_TRUE(r.Changed());
+      EXPECT_THAT(r.replacement(),
+                  IsStoreElement(
+                      AccessBuilder::ForTypedArrayElement(type, true),
+                      IsIntPtrConstant(bit_cast<intptr_t>(&backing_store[0])),
+                      key, IsNumberConstant(array->length()->Number()),
+                      value_matcher, effect_matcher, control));
+    }
+  }
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8
index fde7f03..cb50807 100644 (file)
@@ -706,6 +706,52 @@ class IsLoadMatcher FINAL : public NodeMatcher {
 };
 
 
+class IsToNumberMatcher FINAL : public NodeMatcher {
+ public:
+  IsToNumberMatcher(const Matcher<Node*>& base_matcher,
+                    const Matcher<Node*>& context_matcher,
+                    const Matcher<Node*>& effect_matcher,
+                    const Matcher<Node*>& control_matcher)
+      : NodeMatcher(IrOpcode::kJSToNumber),
+        base_matcher_(base_matcher),
+        context_matcher_(context_matcher),
+        effect_matcher_(effect_matcher),
+        control_matcher_(control_matcher) {}
+
+  virtual void DescribeTo(std::ostream* os) const OVERRIDE {
+    NodeMatcher::DescribeTo(os);
+    *os << " whose base (";
+    base_matcher_.DescribeTo(os);
+    *os << "), context (";
+    context_matcher_.DescribeTo(os);
+    *os << "), effect (";
+    effect_matcher_.DescribeTo(os);
+    *os << ") and control (";
+    control_matcher_.DescribeTo(os);
+    *os << ")";
+  }
+
+  virtual bool MatchAndExplain(Node* node,
+                               MatchResultListener* listener) const OVERRIDE {
+    return (NodeMatcher::MatchAndExplain(node, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetValueInput(node, 0), "base",
+                                 base_matcher_, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetContextInput(node),
+                                 "context", context_matcher_, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetEffectInput(node), "effect",
+                                 effect_matcher_, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetControlInput(node),
+                                 "control", control_matcher_, listener));
+  }
+
+ private:
+  const Matcher<Node*> base_matcher_;
+  const Matcher<Node*> context_matcher_;
+  const Matcher<Node*> effect_matcher_;
+  const Matcher<Node*> control_matcher_;
+};
+
+
 class IsStoreMatcher FINAL : public NodeMatcher {
  public:
   IsStoreMatcher(const Matcher<StoreRepresentation>& rep_matcher,
@@ -1000,6 +1046,15 @@ Matcher<Node*> IsLoad(const Matcher<LoadRepresentation>& rep_matcher,
 }
 
 
+Matcher<Node*> IsToNumber(const Matcher<Node*>& base_matcher,
+                          const Matcher<Node*>& context_matcher,
+                          const Matcher<Node*>& effect_matcher,
+                          const Matcher<Node*>& control_matcher) {
+  return MakeMatcher(new IsToNumberMatcher(base_matcher, context_matcher,
+                                           effect_matcher, control_matcher));
+}
+
+
 Matcher<Node*> IsStore(const Matcher<StoreRepresentation>& rep_matcher,
                        const Matcher<Node*>& base_matcher,
                        const Matcher<Node*>& index_matcher,
@@ -1063,6 +1118,8 @@ IS_UNOP_MATCHER(Float64Floor)
 IS_UNOP_MATCHER(Float64Ceil)
 IS_UNOP_MATCHER(Float64RoundTruncate)
 IS_UNOP_MATCHER(Float64RoundTiesAway)
+IS_UNOP_MATCHER(NumberToInt32)
+IS_UNOP_MATCHER(NumberToUint32)
 #undef IS_UNOP_MATCHER
 
 }  // namespace compiler
index 870d555..ac24b06 100644 (file)
@@ -163,6 +163,12 @@ Matcher<Node*> IsFloat64Floor(const Matcher<Node*>& input_matcher);
 Matcher<Node*> IsFloat64Ceil(const Matcher<Node*>& input_matcher);
 Matcher<Node*> IsFloat64RoundTruncate(const Matcher<Node*>& input_matcher);
 Matcher<Node*> IsFloat64RoundTiesAway(const Matcher<Node*>& input_matcher);
+Matcher<Node*> IsToNumber(const Matcher<Node*>& base_matcher,
+                          const Matcher<Node*>& context_matcher,
+                          const Matcher<Node*>& effect_matcher,
+                          const Matcher<Node*>& control_matcher);
+Matcher<Node*> IsNumberToInt32(const Matcher<Node*>& input_matcher);
+Matcher<Node*> IsNumberToUint32(const Matcher<Node*>& input_matcher);
 
 }  // namespace compiler
 }  // namespace internal