Revert of [turbofan] Improve typed lowering of JSBitwiseAnd. (patchset #1 id:1 of...
authorbmeurer <bmeurer@chromium.org>
Wed, 14 Jan 2015 09:06:50 +0000 (01:06 -0800)
committerCommit bot <commit-bot@chromium.org>
Wed, 14 Jan 2015 09:07:07 +0000 (09:07 +0000)
Reason for revert:
Breaks SQLite

Original issue's description:
> [turbofan] Improve typed lowering of JSBitwiseAnd.
>
> TEST=unittests
> R=jarin@chromium.org
>
> Committed: https://crrev.com/284e1108182995abe85f580bc813d26491642b8c
> Cr-Commit-Position: refs/heads/master@{#26046}

TBR=jarin@chromium.org
NOTREECHECKS=true
NOTRY=true

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

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

src/compiler/js-typed-lowering.cc
src/compiler/js-typed-lowering.h
test/unittests/compiler/js-typed-lowering-unittest.cc
test/unittests/compiler/node-test-utils.cc
test/unittests/compiler/node-test-utils.h

index e2f3044..7618375 100644 (file)
@@ -33,11 +33,10 @@ JSTypedLowering::JSTypedLowering(JSGraph* jsgraph, Zone* zone)
     : jsgraph_(jsgraph), simplified_(graph()->zone()), conversions_(zone) {
   Handle<Object> zero = factory()->NewNumber(0.0);
   Handle<Object> one = factory()->NewNumber(1.0);
-  zero_range_ = Type::Range(zero, zero, zone);
-  one_range_ = Type::Range(one, one, zone);
-  zero_one_range_ = Type::Range(zero, one, zone);
+  zero_range_ = Type::Range(zero, zero, graph()->zone());
+  one_range_ = Type::Range(one, one, graph()->zone());
   Handle<Object> thirtyone = factory()->NewNumber(31.0);
-  zero_thirtyone_range_ = Type::Range(zero, thirtyone, zone);
+  zero_thirtyone_range_ = Type::Range(zero, thirtyone, graph()->zone());
   // TODO(jarin): Can we have a correctification of the stupid type system?
   // These stupid work-arounds are just stupid!
   shifted_int32_ranges_[0] = Type::Signed32();
@@ -46,13 +45,13 @@ JSTypedLowering::JSTypedLowering(JSGraph* jsgraph, Zone* zone)
     for (size_t k = 2; k < arraysize(shifted_int32_ranges_); ++k) {
       Handle<Object> min = factory()->NewNumber(kMinInt / (1 << k));
       Handle<Object> max = factory()->NewNumber(kMaxInt / (1 << k));
-      shifted_int32_ranges_[k] = Type::Range(min, max, zone);
+      shifted_int32_ranges_[k] = Type::Range(min, max, graph()->zone());
     }
   } else {
     for (size_t k = 1; k < arraysize(shifted_int32_ranges_); ++k) {
       Handle<Object> min = factory()->NewNumber(kMinInt / (1 << k));
       Handle<Object> max = factory()->NewNumber(kMaxInt / (1 << k));
-      shifted_int32_ranges_[k] = Type::Range(min, max, zone);
+      shifted_int32_ranges_[k] = Type::Range(min, max, graph()->zone());
     }
   }
 }
@@ -112,11 +111,6 @@ class JSBinopReduction FINAL {
     std::swap(left_type_, right_type_);
   }
 
-  void ReplaceLeftInput(Node* l) {
-    node_->ReplaceInput(0, l);
-    left_type_ = NodeProperties::GetBounds(l).upper;
-  }
-
   // Remove all effect and control inputs and outputs to this node and change
   // to the pure operator {op}, possibly inserting a boolean inversion.
   Reduction ChangeToPureOperator(const Operator* op, bool invert = false,
@@ -124,17 +118,16 @@ class JSBinopReduction FINAL {
     DCHECK_EQ(0, op->EffectInputCount());
     DCHECK_EQ(false, OperatorProperties::HasContextInput(op));
     DCHECK_EQ(0, op->ControlInputCount());
-    DCHECK_LE(1, op->ValueInputCount());
-    DCHECK_GE(2, op->ValueInputCount());
+    DCHECK_EQ(2, op->ValueInputCount());
 
     // Remove the effects from the node, if any, and update its effect usages.
     if (node_->op()->EffectInputCount() > 0) {
       RelaxEffects(node_);
     }
-    // Finally, update the operator to the new one.
-    node_->set_op(op);
     // Remove the inputs corresponding to context, effect, and control.
     NodeProperties::RemoveNonValueInputs(node_);
+    // Finally, update the operator to the new one.
+    node_->set_op(op);
 
     // TODO(jarin): Replace the explicit typing hack with a call to some method
     // that encapsulates changing the operator and re-typing.
@@ -156,18 +149,11 @@ class JSBinopReduction FINAL {
     return ChangeToPureOperator(op, false, type);
   }
 
-  Reduction ReplaceWithLeftInput() {
-    // Remove the effects from the node, if any, and update its effect usages.
-    if (node_->op()->EffectInputCount() > 0) {
-      RelaxEffects(node_);
-    }
-    return lowering_->Replace(left());
-  }
+  bool OneInputIs(Type* t) { return left_type_->Is(t) || right_type_->Is(t); }
 
-  bool LeftInputIs(Type* t) { return left_type_->Is(t); }
-  bool RightInputIs(Type* t) { return right_type_->Is(t); }
-  bool OneInputIs(Type* t) { return LeftInputIs(t) || RightInputIs(t); }
-  bool BothInputsAre(Type* t) { return LeftInputIs(t) && RightInputIs(t); }
+  bool BothInputsAre(Type* t) {
+    return left_type_->Is(t) && right_type_->Is(t);
+  }
 
   bool OneInputCannotBe(Type* t) {
     return !left_type_->Maybe(t) || !right_type_->Maybe(t);
@@ -279,41 +265,6 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
 }
 
 
-Reduction JSTypedLowering::ReduceJSBitwiseAnd(Node* node) {
-  JSBinopReduction r(this, node);
-  if (r.LeftInputIs(one_range_)) {
-    if (r.RightInputIs(zero_one_range_)) {
-      // JSBitwiseAnd(1, x:[0,1]) => x
-      r.SwapInputs();
-      return r.ReplaceWithLeftInput();
-    } else if (r.RightInputIs(Type::Boolean())) {
-      // JSBitwiseAnd(1, x:boolean) => BooleanToNumber(x)
-      r.SwapInputs();
-      return r.ChangeToPureOperator(simplified()->BooleanToNumber());
-    }
-  }
-  if (r.RightInputIs(one_range_)) {
-    if (r.LeftInputIs(zero_one_range_)) {
-      // JSBitwiseAnd(x:[0,1], 1) => x
-      return r.ReplaceWithLeftInput();
-    } else if (r.LeftInputIs(Type::Boolean())) {
-      // JSBitwiseAnd(x:boolean, 1) => BooleanToNumber(x)
-      return r.ChangeToPureOperator(simplified()->BooleanToNumber());
-    }
-  }
-  if (r.BothInputsAre(Type::Primitive())) {
-    // TODO(titzer): some Smi bitwise operations don't really require going
-    // all the way to int32, which can save tagging/untagging for some
-    // operations
-    // on some platforms.
-    // TODO(turbofan): make this heuristic configurable for code size.
-    r.ConvertInputsToUI32(kSigned, kSigned);
-    return r.ChangeToPureOperator(machine()->Word32And(), Type::Integral32());
-  }
-  return NoChange();
-}
-
-
 Reduction JSTypedLowering::ReduceJSBitwiseOr(Node* node) {
   JSBinopReduction r(this, node);
   if (r.BothInputsAre(Type::Primitive()) || r.OneInputIs(zero_range_)) {
@@ -936,7 +887,7 @@ Reduction JSTypedLowering::Reduce(Node* node) {
     case IrOpcode::kJSBitwiseXor:
       return ReduceInt32Binop(node, machine()->Word32Xor());
     case IrOpcode::kJSBitwiseAnd:
-      return ReduceJSBitwiseAnd(node);
+      return ReduceInt32Binop(node, machine()->Word32And());
     case IrOpcode::kJSShiftLeft:
       return ReduceUI32Shift(node, kSigned, machine()->Word32Shl());
     case IrOpcode::kJSShiftRight:
index fe879dd..838085e 100644 (file)
@@ -32,7 +32,6 @@ class JSTypedLowering FINAL : public Reducer {
 
   Reduction ReplaceEagerly(Node* old, Node* node);
   Reduction ReduceJSAdd(Node* node);
-  Reduction ReduceJSBitwiseAnd(Node* node);
   Reduction ReduceJSBitwiseOr(Node* node);
   Reduction ReduceJSMultiply(Node* node);
   Reduction ReduceJSComparison(Node* node);
@@ -73,7 +72,6 @@ class JSTypedLowering FINAL : public Reducer {
   ZoneVector<Node*> conversions_;  // Cache inserted JSToXXX() conversions.
   Type* zero_range_;
   Type* one_range_;
-  Type* zero_one_range_;
   Type* zero_thirtyone_range_;
   Type* shifted_int32_ranges_[4];
 };
index af70284..97ff106 100644 (file)
@@ -397,51 +397,6 @@ TEST_F(JSTypedLoweringTest, JSStrictEqualWithTheHole) {
 
 
 // -----------------------------------------------------------------------------
-// JSBitwiseAnd
-
-
-TEST_F(JSTypedLoweringTest, JSBitwiseAndWithBitish) {
-  Node* const context = Parameter(Type::Any(), 2);
-  Node* const effect = graph()->start();
-  Node* const control = graph()->start();
-  Handle<Object> zero = factory()->NewNumber(0);
-  Handle<Object> one = factory()->NewNumber(1);
-  {
-    Node* const lhs = Parameter(Type::Range(one, one, zone()), 0);
-    Node* const rhs = Parameter(Type::Range(zero, one, zone()), 1);
-    Reduction r = Reduce(graph()->NewNode(javascript()->BitwiseAnd(), lhs, rhs,
-                                          context, effect, control));
-    ASSERT_TRUE(r.Changed());
-    EXPECT_EQ(rhs, r.replacement());
-  }
-  {
-    Node* const lhs = Parameter(Type::Range(one, one, zone()), 0);
-    Node* const rhs = Parameter(Type::Boolean(), 1);
-    Reduction r = Reduce(graph()->NewNode(javascript()->BitwiseAnd(), lhs, rhs,
-                                          context, effect, control));
-    ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsBooleanToNumber(rhs));
-  }
-  {
-    Node* const lhs = Parameter(Type::Range(zero, one, zone()), 0);
-    Node* const rhs = Parameter(Type::Range(one, one, zone()), 1);
-    Reduction r = Reduce(graph()->NewNode(javascript()->BitwiseAnd(), lhs, rhs,
-                                          context, effect, control));
-    ASSERT_TRUE(r.Changed());
-    EXPECT_EQ(lhs, r.replacement());
-  }
-  {
-    Node* const lhs = Parameter(Type::Boolean(), 0);
-    Node* const rhs = Parameter(Type::Range(one, one, zone()), 1);
-    Reduction r = Reduce(graph()->NewNode(javascript()->BitwiseAnd(), lhs, rhs,
-                                          context, effect, control));
-    ASSERT_TRUE(r.Changed());
-    EXPECT_THAT(r.replacement(), IsBooleanToNumber(lhs));
-  }
-}
-
-
-// -----------------------------------------------------------------------------
 // JSShiftLeft
 
 
index 7499cd6..74afda9 100644 (file)
@@ -1292,7 +1292,6 @@ IS_BINOP_MATCHER(Float64Sub)
   }
 IS_UNOP_MATCHER(AnyToBoolean)
 IS_UNOP_MATCHER(BooleanNot)
-IS_UNOP_MATCHER(BooleanToNumber)
 IS_UNOP_MATCHER(ChangeFloat64ToInt32)
 IS_UNOP_MATCHER(ChangeFloat64ToUint32)
 IS_UNOP_MATCHER(ChangeInt32ToFloat64)
index 9b4245c..02b6e43 100644 (file)
@@ -77,7 +77,6 @@ Matcher<Node*> IsCall(const Matcher<CallDescriptor*>& descriptor_matcher,
 
 Matcher<Node*> IsAnyToBoolean(const Matcher<Node*>& value_matcher);
 Matcher<Node*> IsBooleanNot(const Matcher<Node*>& value_matcher);
-Matcher<Node*> IsBooleanToNumber(const Matcher<Node*>& value_matcher);
 Matcher<Node*> IsNumberEqual(const Matcher<Node*>& lhs_matcher,
                              const Matcher<Node*>& rhs_matcher);
 Matcher<Node*> IsNumberLessThan(const Matcher<Node*>& lhs_matcher,