Skip to content

V
V8
Commit 904ac0ae authored by jarin's avatar jarin Committed by Commit bot
Browse files
Options

[turbofan] Make string comparisons effectful. 

BUG=v8:4446
LOG=n

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

Cr-Commit-Position: refs/heads/master@{#31006}
parent d06930fc
Show whitespace changes
Inline Side-by-side
Showing with 113 additions and 41 deletions
src/compiler/js-typed-lowering.cc
View file @ 904ac0ae
......@@ -181,6 +181,32 @@ class JSBinopReduction final {
return lowering_->Changed(node_);
}
Reduction ChangeToStringComparisonOperator(const Operator* op,
bool invert = false) {
if (node_->op()->ControlInputCount() > 0) {
lowering_->RelaxControls(node_);
}
// String comparison operators need effect and control inputs, so copy them
// over.
Node* effect = NodeProperties::GetEffectInput(node_);
Node* control = NodeProperties::GetControlInput(node_);
node_->ReplaceInput(2, effect);
node_->ReplaceInput(3, control);
node_->TrimInputCount(4);
NodeProperties::ChangeOp(node_, op);
if (invert) {
// Insert a boolean-not to invert the value.
Node* value = graph()->NewNode(simplified()->BooleanNot(), node_);
node_->ReplaceUses(value);
// Note: ReplaceUses() smashes all uses, so smash it back here.
value->ReplaceInput(0, node_);
return lowering_->Replace(value);
}
return lowering_->Changed(node_);
}
Reduction ChangeToPureOperator(const Operator* op, Type* type) {
return ChangeToPureOperator(op, false, type);
}
......@@ -503,7 +529,8 @@ Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
default:
return NoChange();
}
return r.ChangeToPureOperator(stringOp);
r.ChangeToStringComparisonOperator(stringOp);
return Changed(node);
}
if (r.OneInputCannotBe(Type::StringOrReceiver())) {
const Operator* less_than;
......@@ -557,7 +584,8 @@ Reduction JSTypedLowering::ReduceJSEqual(Node* node, bool invert) {
return r.ChangeToPureOperator(simplified()->NumberEqual(), invert);
}
if (r.BothInputsAre(Type::String())) {
return r.ChangeToPureOperator(simplified()->StringEqual(), invert);
return r.ChangeToStringComparisonOperator(simplified()->StringEqual(),
invert);
}
if (r.BothInputsAre(Type::Receiver())) {
return r.ChangeToPureOperator(
......@@ -614,7 +642,8 @@ Reduction JSTypedLowering::ReduceJSStrictEqual(Node* node, bool invert) {
invert);
}
if (r.BothInputsAre(Type::String())) {
return r.ChangeToPureOperator(simplified()->StringEqual(), invert);
return r.ChangeToStringComparisonOperator(simplified()->StringEqual(),
invert);
}
if (r.BothInputsAre(Type::Number())) {
return r.ChangeToPureOperator(simplified()->NumberEqual(), invert);
......
src/compiler/simplified-lowering.cc
View file @ 904ac0ae
......@@ -1363,7 +1363,9 @@ Node* SimplifiedLowering::StringComparison(Node* node) {
return graph()->NewNode(
common()->Call(desc), jsgraph()->HeapConstant(callable.code()),
NodeProperties::GetValueInput(node, 0),
NodeProperties::GetValueInput(node, 1), jsgraph()->NoContextConstant());
NodeProperties::GetValueInput(node, 1), jsgraph()->NoContextConstant(),
NodeProperties::GetEffectInput(node),
NodeProperties::GetControlInput(node));
}
......@@ -1627,23 +1629,49 @@ void SimplifiedLowering::DoShift(Node* node, Operator const* op) {
}
namespace {
void ReplaceEffectUses(Node* node, Node* replacement) {
// Requires distinguishing between value and effect edges.
DCHECK(replacement->op()->EffectOutputCount() > 0);
for (Edge edge : node->use_edges()) {
if (NodeProperties::IsEffectEdge(edge)) {
edge.UpdateTo(replacement);
} else {
DCHECK(NodeProperties::IsValueEdge(edge));
}
}
}
} // namespace
void SimplifiedLowering::DoStringEqual(Node* node) {
node->ReplaceInput(0, StringComparison(node));
Node* comparison = StringComparison(node);
ReplaceEffectUses(node, comparison);
node->ReplaceInput(0, comparison);
node->ReplaceInput(1, jsgraph()->SmiConstant(EQUAL));
node->TrimInputCount(2);
NodeProperties::ChangeOp(node, machine()->WordEqual());
}
void SimplifiedLowering::DoStringLessThan(Node* node) {
node->ReplaceInput(0, StringComparison(node));
Node* comparison = StringComparison(node);
ReplaceEffectUses(node, comparison);
node->ReplaceInput(0, comparison);
node->ReplaceInput(1, jsgraph()->SmiConstant(EQUAL));
node->TrimInputCount(2);
NodeProperties::ChangeOp(node, machine()->IntLessThan());
}
void SimplifiedLowering::DoStringLessThanOrEqual(Node* node) {
node->ReplaceInput(0, StringComparison(node));
Node* comparison = StringComparison(node);
ReplaceEffectUses(node, comparison);
node->ReplaceInput(0, comparison);
node->ReplaceInput(1, jsgraph()->SmiConstant(EQUAL));
node->TrimInputCount(2);
NodeProperties::ChangeOp(node, machine()->IntLessThanOrEqual());
}
......
src/compiler/simplified-operator.cc
View file @ 904ac0ae
......@@ -174,9 +174,6 @@ const ElementAccess& ElementAccessOf(const Operator* op) {
V(NumberToInt32, Operator::kNoProperties, 1) \
V(NumberToUint32, Operator::kNoProperties, 1) \
V(PlainPrimitiveToNumber, Operator::kNoProperties, 1) \
V(StringEqual, Operator::kCommutative, 2) \
V(StringLessThan, Operator::kNoProperties, 2) \
V(StringLessThanOrEqual, Operator::kNoProperties, 2) \
V(ChangeTaggedToInt32, Operator::kNoProperties, 1) \
V(ChangeTaggedToUint32, Operator::kNoProperties, 1) \
V(ChangeTaggedToFloat64, Operator::kNoProperties, 1) \
......@@ -187,6 +184,10 @@ const ElementAccess& ElementAccessOf(const Operator* op) {
V(ChangeBitToBool, Operator::kNoProperties, 1) \
V(ObjectIsSmi, Operator::kNoProperties, 1)
#define NO_THROW_OP_LIST(V) \
V(StringEqual, Operator::kCommutative, 2) \
V(StringLessThan, Operator::kNoThrow, 2) \
V(StringLessThanOrEqual, Operator::kNoThrow, 2)
struct SimplifiedOperatorGlobalCache final {
#define PURE(Name, properties, input_count) \
......@@ -199,6 +200,16 @@ struct SimplifiedOperatorGlobalCache final {
PURE_OP_LIST(PURE)
#undef PURE
#define NO_THROW(Name, properties, input_count) \
struct Name##Operator final : public Operator { \
Name##Operator() \
: Operator(IrOpcode::k##Name, Operator::kNoThrow | properties, #Name, \
input_count, 1, 1, 1, 1, 0) {} \
}; \
Name##Operator k##Name;
NO_THROW_OP_LIST(NO_THROW)
#undef NO_THROW
#define BUFFER_ACCESS(Type, type, TYPE, ctype, size) \
struct LoadBuffer##Type##Operator final : public Operator1<BufferAccess> { \
LoadBuffer##Type##Operator() \
......@@ -229,10 +240,11 @@ SimplifiedOperatorBuilder::SimplifiedOperatorBuilder(Zone* zone)
: cache_(kCache.Get()), zone_(zone) {}
#define PURE(Name, properties, input_count) \
#define GET_FROM_CACHE(Name, properties, input_count) \
const Operator* SimplifiedOperatorBuilder::Name() { return &cache_.k##Name; }
PURE_OP_LIST(PURE)
#undef PURE
PURE_OP_LIST(GET_FROM_CACHE)
NO_THROW_OP_LIST(GET_FROM_CACHE)
#undef GET_FROM_CACHE
const Operator* SimplifiedOperatorBuilder::ReferenceEqual(Type* type) {
......
test/cctest/compiler/test-js-typed-lowering.cc
View file @ 904ac0ae
......@@ -105,14 +105,12 @@ class JSTypedLoweringTester : public HandleAndZoneScope {
Node* control() { return start(); }
void CheckPureBinop(IrOpcode::Value expected, Node* node) {
void CheckBinop(IrOpcode::Value expected, Node* node) {
CHECK_EQ(expected, node->opcode());
CHECK_EQ(2, node->InputCount()); // should not have context, effect, etc.
}
void CheckPureBinop(const Operator* expected, Node* node) {
void CheckBinop(const Operator* expected, Node* node) {
CHECK_EQ(expected->opcode(), node->op()->opcode());
CHECK_EQ(2, node->InputCount()); // should not have context, effect, etc.
}
Node* ReduceUnop(const Operator* op, Type* input_type) {
......@@ -249,7 +247,7 @@ TEST_WITH_STRONG(StringBinops) {
Node* add = R.Binop(R.javascript.Add(language_mode), p0, p1);
Node* r = R.reduce(add);
R.CheckPureBinop(IrOpcode::kStringAdd, r);
R.CheckBinop(IrOpcode::kStringAdd, r);
CHECK_EQ(p0, r->InputAt(0));
CHECK_EQ(p1, r->InputAt(1));
}
......@@ -266,7 +264,7 @@ TEST_WITH_STRONG(AddNumber1) {
Node* add = R.Binop(R.javascript.Add(language_mode), p0, p1);
Node* r = R.reduce(add);
R.CheckPureBinop(IrOpcode::kNumberAdd, r);
R.CheckBinop(IrOpcode::kNumberAdd, r);
CHECK_EQ(p0, r->InputAt(0));
CHECK_EQ(p1, r->InputAt(1));
}
......@@ -293,7 +291,7 @@ TEST_WITH_STRONG(NumberBinops) {
Node* add = R.Binop(ops[k], p0, p1);
Node* r = R.reduce(add);
R.CheckPureBinop(ops[k + 1], r);
R.CheckBinop(ops[k + 1], r);
CHECK_EQ(p0, r->InputAt(0));
CHECK_EQ(p1, r->InputAt(1));
}
......@@ -363,7 +361,7 @@ TEST(Int32BitwiseShifts) {
Node* add = R.Binop(R.ops[k], p0, p1);
Node* r = R.reduce(add);
R.CheckPureBinop(R.ops[k + 1], r);
R.CheckBinop(R.ops[k + 1], r);
Node* r0 = r->InputAt(0);
Node* r1 = r->InputAt(1);
......@@ -421,7 +419,7 @@ TEST(Int32BitwiseBinops) {
Node* add = R.Binop(R.ops[k], p0, p1);
Node* r = R.reduce(add);
R.CheckPureBinop(R.ops[k + 1], r);
R.CheckBinop(R.ops[k + 1], r);
CheckToI32(p0, r->InputAt(0), R.signedness[k]);
CheckToI32(p1, r->InputAt(1), R.signedness[k + 1]);
......@@ -614,7 +612,7 @@ TEST_WITH_STRONG(StringComparison) {
Node* cmp = R.Binop(ops[k], p0, p1);
Node* r = R.reduce(cmp);
R.CheckPureBinop(ops[k + 1], r);
R.CheckBinop(ops[k + 1], r);
if (k >= 4) {
// GreaterThan and GreaterThanOrEqual commute the inputs
// and use the LessThan and LessThanOrEqual operators.
......@@ -662,7 +660,7 @@ TEST_WITH_STRONG(NumberComparison) {
Node* cmp = R.Binop(ops[k], p0, p1);
Node* r = R.reduce(cmp);
R.CheckPureBinop(ops[k + 1], r);
R.CheckBinop(ops[k + 1], r);
if (k >= 4) {
// GreaterThan and GreaterThanOrEqual commute the inputs
// and use the LessThan and LessThanOrEqual operators.
......@@ -692,13 +690,13 @@ TEST_WITH_STRONG(MixedComparison1) {
Node* cmp = R.Binop(less_than, p0, p1);
Node* r = R.reduce(cmp);
if (types[i]->Is(Type::String()) && types[j]->Is(Type::String())) {
R.CheckPureBinop(R.simplified.StringLessThan(), r);
R.CheckBinop(R.simplified.StringLessThan(), r);
} else if ((types[i]->Is(Type::Number()) &&
types[j]->Is(Type::Number())) ||
(!is_strong(language_mode) &&
(!types[i]->Maybe(Type::String()) ||
!types[j]->Maybe(Type::String())))) {
R.CheckPureBinop(R.simplified.NumberLessThan(), r);
R.CheckBinop(R.simplified.NumberLessThan(), r);
} else {
// No reduction of mixed types.
CHECK_EQ(r->op(), less_than);
......@@ -839,7 +837,7 @@ void CheckEqualityReduction(JSTypedLoweringTester* R, bool strict, Node* l,
strict ? R->javascript.StrictEqual() : R->javascript.Equal();
Node* eq = R->Binop(op, p0, p1);
Node* r = R->reduce(eq);
R->CheckPureBinop(expected, r);
R->CheckBinop(expected, r);
}
{
......@@ -849,7 +847,7 @@ void CheckEqualityReduction(JSTypedLoweringTester* R, bool strict, Node* l,
Node* n = R->reduce(ne);
CHECK_EQ(IrOpcode::kBooleanNot, n->opcode());
Node* r = n->InputAt(0);
R->CheckPureBinop(expected, r);
R->CheckBinop(expected, r);
}
}
}
......@@ -920,7 +918,7 @@ TEST_WITH_STRONG(RemovePureNumberBinopEffects) {
BinopEffectsTester B(ops[j], Type::Number(), Type::Number());
CHECK_EQ(ops[j + 1]->opcode(), B.result->op()->opcode());
B.R.CheckPureBinop(B.result->opcode(), B.result);
B.R.CheckBinop(B.result->opcode(), B.result);
B.CheckNoOp(0);
B.CheckNoOp(1);
......@@ -1060,7 +1058,7 @@ TEST(Int32BinopEffects) {
BinopEffectsTester B(R.ops[j], I32Type(signed_left), I32Type(signed_right));
CHECK_EQ(R.ops[j + 1]->opcode(), B.result->op()->opcode());
B.R.CheckPureBinop(B.result->opcode(), B.result);
B.R.CheckBinop(B.result->opcode(), B.result);
B.CheckNoOp(0);
B.CheckNoOp(1);
......@@ -1073,7 +1071,7 @@ TEST(Int32BinopEffects) {
BinopEffectsTester B(R.ops[j], Type::Number(), Type::Number());
CHECK_EQ(R.ops[j + 1]->opcode(), B.result->op()->opcode());
B.R.CheckPureBinop(B.result->opcode(), B.result);
B.R.CheckBinop(B.result->opcode(), B.result);
B.CheckConvertedInput(NumberToI32(signed_left), 0, false);
B.CheckConvertedInput(NumberToI32(signed_right), 1, false);
......@@ -1085,7 +1083,7 @@ TEST(Int32BinopEffects) {
bool signed_left = R.signedness[j], signed_right = R.signedness[j + 1];
BinopEffectsTester B(R.ops[j], Type::Number(), Type::Primitive());
B.R.CheckPureBinop(B.result->opcode(), B.result);
B.R.CheckBinop(B.result->opcode(), B.result);
Node* i0 = B.CheckConvertedInput(NumberToI32(signed_left), 0, false);
Node* i1 = B.CheckConvertedInput(NumberToI32(signed_right), 1, false);
......@@ -1102,7 +1100,7 @@ TEST(Int32BinopEffects) {
bool signed_left = R.signedness[j], signed_right = R.signedness[j + 1];
BinopEffectsTester B(R.ops[j], Type::Primitive(), Type::Number());
B.R.CheckPureBinop(B.result->opcode(), B.result);
B.R.CheckBinop(B.result->opcode(), B.result);
Node* i0 = B.CheckConvertedInput(NumberToI32(signed_left), 0, false);
Node* i1 = B.CheckConvertedInput(NumberToI32(signed_right), 1, false);
......@@ -1119,7 +1117,7 @@ TEST(Int32BinopEffects) {
bool signed_left = R.signedness[j], signed_right = R.signedness[j + 1];
BinopEffectsTester B(R.ops[j], Type::Primitive(), Type::Primitive());
B.R.CheckPureBinop(B.result->opcode(), B.result);
B.R.CheckBinop(B.result->opcode(), B.result);
Node* i0 = B.CheckConvertedInput(NumberToI32(signed_left), 0, false);
Node* i1 = B.CheckConvertedInput(NumberToI32(signed_right), 1, false);
......@@ -1251,7 +1249,7 @@ TEST_WITH_STRONG(Int32Comparisons) {
} else {
expected = ops[o].num_op;
}
R.CheckPureBinop(expected, r);
R.CheckBinop(expected, r);
if (ops[o].commute) {
CHECK_EQ(p1, r->InputAt(0));
CHECK_EQ(p0, r->InputAt(1));
......
test/cctest/compiler/test-simplified-lowering.cc
View file @ 904ac0ae
......@@ -698,6 +698,13 @@ class TestingGraph : public HandleAndZoneScope, public GraphAndBuilders {
CHECK_EQ(expected, node->opcode());
}
void CheckLoweringStringBinop(IrOpcode::Value expected, const Operator* op) {
Node* node = Return(
graph()->NewNode(op, p0, p1, graph()->start(), graph()->start()));
Lower();
CHECK_EQ(expected, node->opcode());
}
void CheckLoweringTruncatedBinop(IrOpcode::Value expected, const Operator* op,
const Operator* trunc) {
Node* node = graph()->NewNode(op, p0, p1);
......@@ -1173,9 +1180,10 @@ TEST(LowerStringOps_to_call_and_compare) {
static_cast<IrOpcode::Value>(t.machine()->IntLessThan()->opcode());
IrOpcode::Value compare_le = static_cast<IrOpcode::Value>(
t.machine()->IntLessThanOrEqual()->opcode());
t.CheckLoweringBinop(compare_eq, t.simplified()->StringEqual());
t.CheckLoweringBinop(compare_lt, t.simplified()->StringLessThan());
t.CheckLoweringBinop(compare_le, t.simplified()->StringLessThanOrEqual());
t.CheckLoweringStringBinop(compare_eq, t.simplified()->StringEqual());
t.CheckLoweringStringBinop(compare_lt, t.simplified()->StringLessThan());
t.CheckLoweringStringBinop(compare_le,
t.simplified()->StringLessThanOrEqual());
}
......
test/unittests/compiler/simplified-operator-unittest.cc
View file @ 904ac0ae
......@@ -51,9 +51,6 @@ const PureOperator kPureOperators[] = {
PURE(NumberToInt32, Operator::kNoProperties, 1),
PURE(NumberToUint32, Operator::kNoProperties, 1),
PURE(PlainPrimitiveToNumber, Operator::kNoProperties, 1),
PURE(StringEqual, Operator::kCommutative, 2),
PURE(StringLessThan, Operator::kNoProperties, 2),
PURE(StringLessThanOrEqual, Operator::kNoProperties, 2),
PURE(ChangeTaggedToInt32, Operator::kNoProperties, 1),
PURE(ChangeTaggedToUint32, Operator::kNoProperties, 1),
PURE(ChangeTaggedToFloat64, Operator::kNoProperties, 1),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment