[turbofan] Introduce new Select operator to improve bounds checking.

TEST=mjsunit,unittests
R=dcarney@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#24980}
git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24980 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/compiler/common-operator.cc

@@ -50,6 +50,32 @@ BranchHint BranchHintOf(const Operator* const op) {
 }
 
 
+bool operator==(SelectParameters const& lhs, SelectParameters const& rhs) {
+  return lhs.type() == rhs.type() && lhs.hint() == rhs.hint();
+}
+
+
+bool operator!=(SelectParameters const& lhs, SelectParameters const& rhs) {
+  return !(lhs == rhs);
+}
+
+
+size_t hash_value(SelectParameters const& p) {
+  return base::hash_combine(p.type(), p.hint());
+}
+
+
+std::ostream& operator<<(std::ostream& os, SelectParameters const& p) {
+  return os << p.type() << "|" << p.hint();
+}
+
+
+SelectParameters const& SelectParametersOf(const Operator* const op) {
+  DCHECK_EQ(IrOpcode::kSelect, op->opcode());
+  return OpParameter<SelectParameters>(op);
+}
+
+
 size_t hash_value(OutputFrameStateCombine const& sc) {
   return base::hash_combine(sc.kind_, sc.parameter_);
 }
@@ -219,6 +245,14 @@ const Operator* CommonOperatorBuilder::HeapConstant(
 }
 
 
+const Operator* CommonOperatorBuilder::Select(MachineType type,
+                                              BranchHint hint) {
+  return new (zone())
+      Operator1<SelectParameters>(IrOpcode::kSelect, Operator::kPure, 3, 1,
+                                  "Select", SelectParameters(type, hint));
+}
+
+
 const Operator* CommonOperatorBuilder::Phi(MachineType type, int arguments) {
   DCHECK(arguments > 0);  // Disallow empty phis.
   return new (zone()) Operator1<MachineType>(IrOpcode::kPhi, Operator::kPure,

src/compiler/common-operator.h

@@ -33,6 +33,30 @@ std::ostream& operator<<(std::ostream&, BranchHint);
 BranchHint BranchHintOf(const Operator* const);
 
 
+class SelectParameters FINAL {
+ public:
+  explicit SelectParameters(MachineType type,
+                            BranchHint hint = BranchHint::kNone)
+      : type_(type), hint_(hint) {}
+
+  MachineType type() const { return type_; }
+  BranchHint hint() const { return hint_; }
+
+ private:
+  const MachineType type_;
+  const BranchHint hint_;
+};
+
+bool operator==(SelectParameters const&, SelectParameters const&);
+bool operator!=(SelectParameters const&, SelectParameters const&);
+
+size_t hash_value(SelectParameters const& p);
+
+std::ostream& operator<<(std::ostream&, SelectParameters const& p);
+
+SelectParameters const& SelectParametersOf(const Operator* const);
+
+
 // Flag that describes how to combine the current environment with
 // the output of a node to obtain a framestate for lazy bailout.
 class OutputFrameStateCombine {
@@ -157,6 +181,7 @@ class CommonOperatorBuilder FINAL : public ZoneObject {
   const Operator* NumberConstant(volatile double);
   const Operator* HeapConstant(const Unique<HeapObject>&);
 
+  const Operator* Select(MachineType, BranchHint = BranchHint::kNone);
   const Operator* Phi(MachineType type, int arguments);
   const Operator* EffectPhi(int arguments);
   const Operator* ValueEffect(int arguments);

src/compiler/control-reducer.cc

@@ -322,6 +322,8 @@ class ControlReducerImpl {
       case IrOpcode::kLoop:
       case IrOpcode::kMerge:
         return ReduceMerge(node);
+      case IrOpcode::kSelect:
+        return ReduceSelect(node);
       case IrOpcode::kPhi:
       case IrOpcode::kEffectPhi:
         return ReducePhi(node);
@@ -330,6 +332,32 @@ class ControlReducerImpl {
     }
   }
 
+  // Reduce redundant selects.
+  Node* ReduceSelect(Node* const node) {
+    Node* const cond = node->InputAt(0);
+    Node* const tvalue = node->InputAt(1);
+    Node* const fvalue = node->InputAt(2);
+    if (tvalue == fvalue) return tvalue;
+    switch (cond->opcode()) {
+      case IrOpcode::kInt32Constant:
+        return Int32Matcher(cond).Is(0) ? fvalue : tvalue;
+      case IrOpcode::kInt64Constant:
+        return Int64Matcher(cond).Is(0) ? fvalue : tvalue;
+      case IrOpcode::kNumberConstant:
+        return NumberMatcher(cond).Is(0) ? fvalue : tvalue;
+      case IrOpcode::kHeapConstant: {
+        Handle<Object> object =
+            HeapObjectMatcher<Object>(cond).Value().handle();
+        if (object->IsTrue()) return tvalue;
+        if (object->IsFalse()) return fvalue;
+        break;
+      }
+      default:
+        break;
+    }
+    return node;
+  }
+
   // Reduce redundant phis.
   Node* ReducePhi(Node* node) {
     int n = node->InputCount();
@@ -406,6 +434,9 @@ class ControlReducerImpl {
       case IrOpcode::kInt32Constant:
         is_true = !Int32Matcher(cond).Is(0);
         break;
+      case IrOpcode::kInt64Constant:
+        is_true = !Int64Matcher(cond).Is(0);
+        break;
       case IrOpcode::kNumberConstant:
         is_true = !NumberMatcher(cond).Is(0);
         break;

src/compiler/js-generic-lowering.cc

@@ -64,6 +64,7 @@ Reduction JSGenericLowering::Reduce(Node* node) {
     Lower##x(node);     \
     break;
     DECLARE_CASE(Branch)
+    DECLARE_CASE(Select)
     JS_OP_LIST(DECLARE_CASE)
 #undef DECLARE_CASE
     default:
@@ -241,6 +242,23 @@ void JSGenericLowering::LowerBranch(Node* node) {
 }
 
 
+void JSGenericLowering::LowerSelect(Node* node) {
+  // TODO(bmeurer): This should probably be moved into a separate file.
+  SelectParameters const& p = SelectParametersOf(node->op());
+  Node* branch = graph()->NewNode(common()->Branch(p.hint()), node->InputAt(0),
+                                  graph()->start());
+  Node* if_true = graph()->NewNode(common()->IfTrue(), branch);
+  Node* vtrue = node->InputAt(1);
+  Node* if_false = graph()->NewNode(common()->IfFalse(), branch);
+  Node* vfalse = node->InputAt(2);
+  Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false);
+  node->set_op(common()->Phi(p.type(), 2));
+  node->ReplaceInput(0, vtrue);
+  node->ReplaceInput(1, vfalse);
+  node->ReplaceInput(2, merge);
+}
+
+
 void JSGenericLowering::LowerJSUnaryNot(Node* node) {
   Callable callable = CodeFactory::ToBoolean(
       isolate(), ToBooleanStub::RESULT_AS_INVERSE_ODDBALL);

src/compiler/js-typed-lowering.cc

@@ -612,7 +612,7 @@ Reduction JSTypedLowering::ReduceJSLoadProperty(Node* node) {
             Handle<ExternalArray>::cast(handle(array->elements()));
         Node* pointer = jsgraph()->IntPtrConstant(
             bit_cast<intptr_t>(elements->external_pointer()));
-        Node* length = jsgraph()->Constant(byte_length / array->element_size());
+        Node* length = jsgraph()->Constant(array->length()->Number());
         Node* effect = NodeProperties::GetEffectInput(node);
         Node* load = graph()->NewNode(
             simplified()->LoadElement(
@@ -647,7 +647,7 @@ Reduction JSTypedLowering::ReduceJSStoreProperty(Node* node) {
             Handle<ExternalArray>::cast(handle(array->elements()));
         Node* pointer = jsgraph()->IntPtrConstant(
             bit_cast<intptr_t>(elements->external_pointer()));
-        Node* length = jsgraph()->Constant(byte_length / array->element_size());
+        Node* length = jsgraph()->Constant(array->length()->Number());
         Node* effect = NodeProperties::GetEffectInput(node);
         Node* control = NodeProperties::GetControlInput(node);
         Node* store = graph()->NewNode(

src/compiler/machine-type.h

@@ -50,6 +50,8 @@ enum MachineType {
   kMachUint32 = kRepWord32 | kTypeUint32,
   kMachInt64 = kRepWord64 | kTypeInt64,
   kMachUint64 = kRepWord64 | kTypeUint64,
+  kMachIntPtr = (kPointerSize == 4) ? kMachInt32 : kMachInt64,
+  kMachUintPtr = (kPointerSize == 4) ? kMachUint32 : kMachUint64,
   kMachPtr = (kPointerSize == 4) ? kRepWord32 : kRepWord64,
   kMachAnyTagged = kRepTagged | kTypeAny
 };

src/compiler/node-matchers.h

@@ -81,6 +81,13 @@ typedef IntMatcher<int32_t, IrOpcode::kInt32Constant> Int32Matcher;
 typedef IntMatcher<uint32_t, IrOpcode::kInt32Constant> Uint32Matcher;
 typedef IntMatcher<int64_t, IrOpcode::kInt64Constant> Int64Matcher;
 typedef IntMatcher<uint64_t, IrOpcode::kInt64Constant> Uint64Matcher;
+#if V8_HOST_ARCH_32_BIT
+typedef Int32Matcher IntPtrMatcher;
+typedef Uint32Matcher UintPtrMatcher;
+#else
+typedef Int64Matcher IntPtrMatcher;
+typedef Uint64Matcher UintPtrMatcher;
+#endif
 
 
 // A pattern matcher for floating point constants.
@@ -138,6 +145,8 @@ typedef BinopMatcher<Int32Matcher, Int32Matcher> Int32BinopMatcher;
 typedef BinopMatcher<Uint32Matcher, Uint32Matcher> Uint32BinopMatcher;
 typedef BinopMatcher<Int64Matcher, Int64Matcher> Int64BinopMatcher;
 typedef BinopMatcher<Uint64Matcher, Uint64Matcher> Uint64BinopMatcher;
+typedef BinopMatcher<IntPtrMatcher, IntPtrMatcher> IntPtrBinopMatcher;
+typedef BinopMatcher<UintPtrMatcher, UintPtrMatcher> UintPtrBinopMatcher;
 typedef BinopMatcher<Float64Matcher, Float64Matcher> Float64BinopMatcher;
 typedef BinopMatcher<NumberMatcher, NumberMatcher> NumberBinopMatcher;
 

src/compiler/opcodes.h

@@ -33,6 +33,7 @@
   V(HeapConstant)
 
 #define INNER_OP_LIST(V) \
+  V(Select)              \
   V(Phi)                 \
   V(EffectPhi)           \
   V(ValueEffect)         \

src/compiler/simplified-lowering.h

@@ -40,7 +40,7 @@ class SimplifiedLowering FINAL {
   Node* IsTagged(Node* node);
   Node* Untag(Node* node);
   Node* OffsetMinusTagConstant(int32_t offset);
-  Node* ComputeIndex(const ElementAccess& access, Node* index);
+  Node* ComputeIndex(const ElementAccess& access, Node* const key);
   Node* StringComparison(Node* node, bool requires_ordering);
   Node* Int32Div(Node* const node);
   Node* Int32Mod(Node* const node);

src/compiler/typer.cc

@@ -535,6 +535,11 @@ Bounds Typer::Visitor::TypeExternalConstant(Node* node) {
 }
 
 
+Bounds Typer::Visitor::TypeSelect(Node* node) {
+  return Bounds::Either(Operand(node, 1), Operand(node, 2), zone());
+}
+
+
 Bounds Typer::Visitor::TypePhi(Node* node) {
   int arity = OperatorProperties::GetValueInputCount(node->op());
   Bounds bounds = Operand(node, 0);

src/compiler/verifier.cc

@@ -310,6 +310,12 @@ GenericGraphVisit::Control Verifier::Visitor::Pre(Node* node) {
       CheckUpperIs(node, Type::Any());
       break;
     }
+    case IrOpcode::kSelect: {
+      CHECK_EQ(0, effect_count);
+      CHECK_EQ(0, control_count);
+      CHECK_EQ(3, value_count);
+      break;
+    }
     case IrOpcode::kPhi: {
       // Phi input count matches parent control node.
       CHECK_EQ(0, effect_count);

test/mjsunit/asm/int16array-outofbounds.js

@@ -10,16 +10,28 @@ function Module(stdlib, foreign, heap) {
     i = MEM16[i >> 1] | 0;
     return i;
   }
+  function loadm1() {
+    return MEM16[-1] | 0;
+  }
   function store(i, v) {
     i = i|0;
     v = v|0;
     MEM16[i >> 1] = v;
   }
-  return { load: load, store: store };
+  function storem1(v) {
+    v = v|0;
+    MEM16[-1] = v;
+  }
+  return {load: load, loadm1: loadm1, store: store, storem1: storem1};
 }
 
 var m = Module(this, {}, new ArrayBuffer(2));
 
+m.store(-1000, 4);
+assertEquals(0, m.load(-1000));
+assertEquals(0, m.loadm1());
+m.storem1(1);
+assertEquals(0, m.loadm1());
 m.store(0, 32767);
 for (var i = 1; i < 64; ++i) {
   m.store(i * 2 * 32 * 1024, i);

test/unittests/compiler/common-operator-unittest.cc

@@ -156,16 +156,19 @@ const double kDoubleValues[] = {-std::numeric_limits<double>::infinity(),
                                 std::numeric_limits<double>::quiet_NaN(),
                                 std::numeric_limits<double>::signaling_NaN()};
 
+
+const BranchHint kHints[] = {BranchHint::kNone, BranchHint::kTrue,
+                             BranchHint::kFalse};
+
 }  // namespace
 
 
 TEST_F(CommonOperatorTest, Branch) {
-  static const BranchHint kHints[] = {BranchHint::kNone, BranchHint::kTrue,
-                                      BranchHint::kFalse};
   TRACED_FOREACH(BranchHint, hint, kHints) {
     const Operator* const op = common()->Branch(hint);
     EXPECT_EQ(IrOpcode::kBranch, op->opcode());
     EXPECT_EQ(Operator::kFoldable, op->properties());
+    EXPECT_EQ(hint, BranchHintOf(op));
     EXPECT_EQ(1, OperatorProperties::GetValueInputCount(op));
     EXPECT_EQ(0, OperatorProperties::GetEffectInputCount(op));
     EXPECT_EQ(1, OperatorProperties::GetControlInputCount(op));
@@ -177,6 +180,30 @@ TEST_F(CommonOperatorTest, Branch) {
 }
 
 
+TEST_F(CommonOperatorTest, Select) {
+  static const MachineType kTypes[] = {
+      kMachInt8,    kMachUint8,   kMachInt16,    kMachUint16,
+      kMachInt32,   kMachUint32,  kMachInt64,    kMachUint64,
+      kMachFloat32, kMachFloat64, kMachAnyTagged};
+  TRACED_FOREACH(MachineType, type, kTypes) {
+    TRACED_FOREACH(BranchHint, hint, kHints) {
+      const Operator* const op = common()->Select(type, hint);
+      EXPECT_EQ(IrOpcode::kSelect, op->opcode());
+      EXPECT_EQ(Operator::kPure, op->properties());
+      EXPECT_EQ(type, SelectParametersOf(op).type());
+      EXPECT_EQ(hint, SelectParametersOf(op).hint());
+      EXPECT_EQ(3, OperatorProperties::GetValueInputCount(op));
+      EXPECT_EQ(0, OperatorProperties::GetEffectInputCount(op));
+      EXPECT_EQ(0, OperatorProperties::GetControlInputCount(op));
+      EXPECT_EQ(3, OperatorProperties::GetTotalInputCount(op));
+      EXPECT_EQ(1, OperatorProperties::GetValueOutputCount(op));
+      EXPECT_EQ(0, OperatorProperties::GetEffectOutputCount(op));
+      EXPECT_EQ(0, OperatorProperties::GetControlOutputCount(op));
+    }
+  }
+}
+
+
 TEST_F(CommonOperatorTest, Float32Constant) {
   TRACED_FOREACH(float, value, kFloatValues) {
     const Operator* op = common()->Float32Constant(value);

test/unittests/compiler/js-typed-lowering-unittest.cc

@@ -114,9 +114,9 @@ TEST_F(JSTypedLoweringTest, JSToBooleanWithOrderedNumberAndBoolean) {
 
 TEST_F(JSTypedLoweringTest, JSLoadPropertyFromExternalTypedArray) {
   const size_t kLength = 17;
-  uint8_t backing_store[kLength * 8];
+  double backing_store[kLength];
   Handle<JSArrayBuffer> buffer =
-      NewArrayBuffer(backing_store, arraysize(backing_store));
+      NewArrayBuffer(backing_store, sizeof(backing_store));
   VectorSlotPair feedback(Handle<TypeFeedbackVector>::null(),
                           FeedbackVectorICSlot::Invalid());
   TRACED_FOREACH(ExternalArrayType, type, kExternalArrayTypes) {
@@ -138,12 +138,11 @@ TEST_F(JSTypedLoweringTest, JSLoadPropertyFromExternalTypedArray) {
     Reduction r = Reduce(node);
 
     ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(
-        r.replacement(),
-        IsLoadElement(AccessBuilder::ForTypedArrayElement(type, true),
-                      IsIntPtrConstant(bit_cast<intptr_t>(&backing_store[0])),
-                      key, IsNumberConstant(static_cast<double>(kLength)),
-                      effect));
+    EXPECT_THAT(r.replacement(),
+                IsLoadElement(
+                    AccessBuilder::ForTypedArrayElement(type, true),
+                    IsIntPtrConstant(bit_cast<intptr_t>(&backing_store[0])),
+                    key, IsNumberConstant(array->length()->Number()), effect));
   }
 }
 
@@ -154,9 +153,9 @@ TEST_F(JSTypedLoweringTest, JSLoadPropertyFromExternalTypedArray) {
 
 TEST_F(JSTypedLoweringTest, JSStorePropertyToExternalTypedArray) {
   const size_t kLength = 17;
-  uint8_t backing_store[kLength * 8];
+  double backing_store[kLength];
   Handle<JSArrayBuffer> buffer =
-      NewArrayBuffer(backing_store, arraysize(backing_store));
+      NewArrayBuffer(backing_store, sizeof(backing_store));
   TRACED_FOREACH(ExternalArrayType, type, kExternalArrayTypes) {
     TRACED_FOREACH(StrictMode, strict_mode, kStrictModes) {
       Handle<JSTypedArray> array =
@@ -182,8 +181,8 @@ TEST_F(JSTypedLoweringTest, JSStorePropertyToExternalTypedArray) {
                   IsStoreElement(
                       AccessBuilder::ForTypedArrayElement(type, true),
                       IsIntPtrConstant(bit_cast<intptr_t>(&backing_store[0])),
-                      key, IsNumberConstant(static_cast<double>(kLength)),
-                      value, effect, control));
+                      key, IsNumberConstant(array->length()->Number()), value,
+                      effect, control));
     }
   }
 }

test/unittests/compiler/node-test-utils.cc

@@ -208,6 +208,52 @@ class IsConstantMatcher FINAL : public NodeMatcher {
 };
 
 
+class IsSelectMatcher FINAL : public NodeMatcher {
+ public:
+  IsSelectMatcher(const Matcher<MachineType>& type_matcher,
+                  const Matcher<Node*>& value0_matcher,
+                  const Matcher<Node*>& value1_matcher,
+                  const Matcher<Node*>& value2_matcher)
+      : NodeMatcher(IrOpcode::kSelect),
+        type_matcher_(type_matcher),
+        value0_matcher_(value0_matcher),
+        value1_matcher_(value1_matcher),
+        value2_matcher_(value2_matcher) {}
+
+  virtual void DescribeTo(std::ostream* os) const OVERRIDE {
+    NodeMatcher::DescribeTo(os);
+    *os << " whose type (";
+    type_matcher_.DescribeTo(os);
+    *os << "), value0 (";
+    value0_matcher_.DescribeTo(os);
+    *os << "), value1 (";
+    value1_matcher_.DescribeTo(os);
+    *os << ") and value2 (";
+    value2_matcher_.DescribeTo(os);
+    *os << ")";
+  }
+
+  virtual bool MatchAndExplain(Node* node,
+                               MatchResultListener* listener) const OVERRIDE {
+    return (NodeMatcher::MatchAndExplain(node, listener) &&
+            PrintMatchAndExplain(OpParameter<MachineType>(node), "type",
+                                 type_matcher_, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetValueInput(node, 0),
+                                 "value0", value0_matcher_, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetValueInput(node, 1),
+                                 "value1", value1_matcher_, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetValueInput(node, 2),
+                                 "value2", value2_matcher_, listener));
+  }
+
+ private:
+  const Matcher<MachineType> type_matcher_;
+  const Matcher<Node*> value0_matcher_;
+  const Matcher<Node*> value1_matcher_;
+  const Matcher<Node*> value2_matcher_;
+};
+
+
 class IsPhiMatcher FINAL : public NodeMatcher {
  public:
   IsPhiMatcher(const Matcher<MachineType>& type_matcher,
@@ -765,6 +811,15 @@ Matcher<Node*> IsNumberConstant(const Matcher<double>& value_matcher) {
 }
 
 
+Matcher<Node*> IsSelect(const Matcher<MachineType>& type_matcher,
+                        const Matcher<Node*>& value0_matcher,
+                        const Matcher<Node*>& value1_matcher,
+                        const Matcher<Node*>& value2_matcher) {
+  return MakeMatcher(new IsSelectMatcher(type_matcher, value0_matcher,
+                                         value1_matcher, value2_matcher));
+}
+
+
 Matcher<Node*> IsPhi(const Matcher<MachineType>& type_matcher,
                      const Matcher<Node*>& value0_matcher,
                      const Matcher<Node*>& value1_matcher,
@@ -857,6 +912,7 @@ Matcher<Node*> IsStore(const Matcher<StoreRepresentation>& rep_matcher,
 IS_BINOP_MATCHER(NumberEqual)
 IS_BINOP_MATCHER(NumberLessThan)
 IS_BINOP_MATCHER(NumberSubtract)
+IS_BINOP_MATCHER(NumberMultiply)
 IS_BINOP_MATCHER(Word32And)
 IS_BINOP_MATCHER(Word32Sar)
 IS_BINOP_MATCHER(Word32Shl)

test/unittests/compiler/node-test-utils.h

@@ -48,6 +48,10 @@ Matcher<Node*> IsFloat64Constant(const Matcher<double>& value_matcher);
 Matcher<Node*> IsInt32Constant(const Matcher<int32_t>& value_matcher);
 Matcher<Node*> IsInt64Constant(const Matcher<int64_t>& value_matcher);
 Matcher<Node*> IsNumberConstant(const Matcher<double>& value_matcher);
+Matcher<Node*> IsSelect(const Matcher<MachineType>& type_matcher,
+                        const Matcher<Node*>& value0_matcher,
+                        const Matcher<Node*>& value1_matcher,
+                        const Matcher<Node*>& value2_matcher);
 Matcher<Node*> IsPhi(const Matcher<MachineType>& type_matcher,
                      const Matcher<Node*>& value0_matcher,
                      const Matcher<Node*>& value1_matcher,
@@ -69,6 +73,8 @@ 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*> IsNumberMultiply(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,