Commit 86b30420 authored by bmeurer's avatar bmeurer Committed by Commit bot

[turbofan] Enable typed lowering of string addition.

Unfortunately StringAdd is not pure in V8 because we might throw an
exception if the resulting string length is outside the valid bounds, so
there's no point in having a simplified StringAdd operator.

R=jarin@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#28747}
parent 298cb976
......@@ -370,20 +370,20 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
r.ConvertInputsToNumber(frame_state);
return r.ChangeToPureOperator(simplified()->NumberAdd(), Type::Number());
}
#if 0
// TODO(turbofan): Lowering of StringAdd is disabled for now because:
// a) The inserted ToString operation screws up valueOf vs. toString order.
// b) Deoptimization at ToString doesn't have corresponding bailout id.
// c) Our current StringAddStub is actually non-pure and requires context.
if ((r.OneInputIs(Type::String()) && !r.IsStrong()) ||
r.BothInputsAre(Type::String())) {
// JSAdd(x:string, y:string) => StringAdd(x, y)
// JSAdd(x:string, y) => StringAdd(x, ToString(y))
// JSAdd(x, y:string) => StringAdd(ToString(x), y)
r.ConvertInputsToString();
return r.ChangeToPureOperator(simplified()->StringAdd());
}
#endif
if (r.BothInputsAre(Type::String())) {
// JSAdd(x:string, y:string) => CallStub[StringAdd](x, y)
Callable const callable =
CodeFactory::StringAdd(isolate(), STRING_ADD_CHECK_NONE, NOT_TENURED);
CallDescriptor const* const desc = Linkage::GetStubCallDescriptor(
isolate(), graph()->zone(), callable.descriptor(), 0,
CallDescriptor::kNeedsFrameState, node->op()->properties());
DCHECK_EQ(2, OperatorProperties::GetFrameStateInputCount(node->op()));
node->RemoveInput(NodeProperties::FirstFrameStateIndex(node) + 1);
node->InsertInput(graph()->zone(), 0,
jsgraph()->HeapConstant(callable.code()));
node->set_op(common()->Call(desc));
return Changed(node);
}
return NoChange();
}
......@@ -1467,6 +1467,9 @@ Factory* JSTypedLowering::factory() const { return jsgraph()->factory(); }
Graph* JSTypedLowering::graph() const { return jsgraph()->graph(); }
Isolate* JSTypedLowering::isolate() const { return jsgraph()->isolate(); }
JSOperatorBuilder* JSTypedLowering::javascript() const {
return jsgraph()->javascript();
}
......
......@@ -72,6 +72,7 @@ class JSTypedLowering final : public AdvancedReducer {
Factory* factory() const;
Graph* graph() const;
JSGraph* jsgraph() const { return jsgraph_; }
Isolate* isolate() const;
JSOperatorBuilder* javascript() const;
CommonOperatorBuilder* common() const;
SimplifiedOperatorBuilder* simplified() { return &simplified_; }
......
......@@ -172,7 +172,6 @@
V(NumberToInt32) \
V(NumberToUint32) \
V(PlainPrimitiveToNumber) \
V(StringAdd) \
V(ChangeTaggedToInt32) \
V(ChangeTaggedToUint32) \
V(ChangeTaggedToFloat64) \
......
......@@ -782,11 +782,6 @@ class RepresentationSelector {
if (lower()) lowering->DoStringLessThanOrEqual(node);
break;
}
case IrOpcode::kStringAdd: {
VisitBinop(node, kMachAnyTagged, kMachAnyTagged);
if (lower()) lowering->DoStringAdd(node);
break;
}
case IrOpcode::kAllocate: {
ProcessInput(node, 0, kMachAnyTagged);
ProcessRemainingInputs(node, 1);
......@@ -1313,23 +1308,6 @@ void SimplifiedLowering::DoStoreElement(Node* node) {
}
void SimplifiedLowering::DoStringAdd(Node* node) {
Operator::Properties properties = node->op()->properties();
Callable callable = CodeFactory::StringAdd(
jsgraph()->isolate(), STRING_ADD_CHECK_NONE, NOT_TENURED);
CallDescriptor::Flags flags = CallDescriptor::kNoFlags;
CallDescriptor* desc = Linkage::GetStubCallDescriptor(
jsgraph()->isolate(), zone(), callable.descriptor(), 0, flags,
properties);
node->set_op(common()->Call(desc));
node->InsertInput(graph()->zone(), 0,
jsgraph()->HeapConstant(callable.code()));
node->AppendInput(graph()->zone(), jsgraph()->UndefinedConstant());
node->AppendInput(graph()->zone(), graph()->start());
node->AppendInput(graph()->zone(), graph()->start());
}
Node* SimplifiedLowering::StringComparison(Node* node, bool requires_ordering) {
Runtime::FunctionId f =
requires_ordering ? Runtime::kStringCompareRT : Runtime::kStringEquals;
......
......@@ -38,7 +38,6 @@ class SimplifiedLowering final {
void DoStoreBuffer(Node* node);
void DoLoadElement(Node* node);
void DoStoreElement(Node* node);
void DoStringAdd(Node* node);
void DoStringEqual(Node* node);
void DoStringLessThan(Node* node);
void DoStringLessThanOrEqual(Node* node);
......
......@@ -174,7 +174,6 @@ const ElementAccess& ElementAccessOf(const Operator* op) {
V(StringEqual, Operator::kCommutative, 2) \
V(StringLessThan, Operator::kNoProperties, 2) \
V(StringLessThanOrEqual, Operator::kNoProperties, 2) \
V(StringAdd, Operator::kNoProperties, 2) \
V(ChangeTaggedToInt32, Operator::kNoProperties, 1) \
V(ChangeTaggedToUint32, Operator::kNoProperties, 1) \
V(ChangeTaggedToFloat64, Operator::kNoProperties, 1) \
......
......@@ -149,7 +149,6 @@ class SimplifiedOperatorBuilder final {
const Operator* StringEqual();
const Operator* StringLessThan();
const Operator* StringLessThanOrEqual();
const Operator* StringAdd();
const Operator* ChangeTaggedToInt32();
const Operator* ChangeTaggedToUint32();
......
......@@ -1729,11 +1729,6 @@ Bounds Typer::Visitor::TypeStringLessThanOrEqual(Node* node) {
}
Bounds Typer::Visitor::TypeStringAdd(Node* node) {
return Bounds(Type::None(zone()), Type::String(zone()));
}
namespace {
Type* ChangeRepresentation(Type* type, Type* rep, Zone* zone) {
......
......@@ -644,12 +644,6 @@ void Verifier::Visitor::Check(Node* node) {
CheckValueInputIs(node, 1, Type::String());
CheckUpperIs(node, Type::Boolean());
break;
case IrOpcode::kStringAdd:
// (String, String) -> String
CheckValueInputIs(node, 0, Type::String());
CheckValueInputIs(node, 1, Type::String());
CheckUpperIs(node, Type::String());
break;
case IrOpcode::kReferenceEqual: {
// (Unique, Any) -> Boolean and
// (Any, Unique) -> Boolean
......
......@@ -92,9 +92,6 @@ class SimplifiedGraphBuilder : public GraphBuilder {
Node* StringLessThanOrEqual(Node* a, Node* b) {
return NewNode(simplified()->StringLessThanOrEqual(), a, b);
}
Node* StringAdd(Node* a, Node* b) {
return NewNode(simplified()->StringAdd(), a, b);
}
Node* ChangeTaggedToInt32(Node* a) {
return NewNode(simplified()->ChangeTaggedToInt32(), a);
......
......@@ -1266,7 +1266,6 @@ TEST(LowerStringOps_to_call_and_compare) {
t.CheckLoweringBinop(compare_eq, t.simplified()->StringEqual());
t.CheckLoweringBinop(compare_lt, t.simplified()->StringLessThan());
t.CheckLoweringBinop(compare_le, t.simplified()->StringLessThanOrEqual());
t.CheckLoweringBinop(IrOpcode::kCall, t.simplified()->StringAdd());
}
}
......
......@@ -86,12 +86,6 @@ class JSTypedLoweringTest : public TypedGraphTest {
return reducer.Reduce(node);
}
Node* EmptyFrameState() {
MachineOperatorBuilder machine(zone());
JSGraph jsgraph(isolate(), graph(), common(), javascript(), &machine);
return jsgraph.EmptyFrameState();
}
Handle<JSArrayBuffer> NewArrayBuffer(void* bytes, size_t byte_length) {
Handle<JSArrayBuffer> buffer = factory()->NewJSArrayBuffer();
Runtime::SetupArrayBuffer(isolate(), buffer, true, bytes, byte_length);
......@@ -902,11 +896,39 @@ TEST_F(JSTypedLoweringTest, JSLoadNamedGlobalConstants) {
}
}
#if V8_TURBOFAN_TARGET
// -----------------------------------------------------------------------------
// JSAdd
TEST_F(JSTypedLoweringTest, JSAddWithString) {
TRACED_FOREACH(LanguageMode, language_mode, kLanguageModes) {
Node* lhs = Parameter(Type::String(), 0);
Node* rhs = Parameter(Type::String(), 1);
Node* context = Parameter(Type::Any(), 2);
Node* frame_state0 = EmptyFrameState();
Node* frame_state1 = EmptyFrameState();
Node* effect = graph()->start();
Node* control = graph()->start();
Reduction r = Reduce(graph()->NewNode(javascript()->Add(language_mode), lhs,
rhs, context, frame_state0,
frame_state1, effect, control));
ASSERT_TRUE(r.Changed());
EXPECT_THAT(
r.replacement(),
IsCall(_, IsHeapConstant(Unique<HeapObject>::CreateImmovable(
CodeFactory::StringAdd(isolate(), STRING_ADD_CHECK_NONE,
NOT_TENURED).code())),
lhs, rhs, context, frame_state0, effect, control));
}
}
// -----------------------------------------------------------------------------
// JSCreateClosure
#if V8_TURBOFAN_TARGET
TEST_F(JSTypedLoweringTest, JSCreateClosure) {
Node* const context = UndefinedConstant();
Node* const effect = graph()->start();
......@@ -973,7 +995,8 @@ TEST_F(JSTypedLoweringTest, JSCreateLiteralObject) {
CodeFactory::FastCloneShallowObject(isolate(), 6).code())),
input0, input1, input2, _, context, frame_state, effect, control));
}
#endif
#endif // V8_TURBOFAN_TARGET
// -----------------------------------------------------------------------------
......
......@@ -54,7 +54,6 @@ const PureOperator kPureOperators[] = {
PURE(StringEqual, Operator::kCommutative, 2),
PURE(StringLessThan, Operator::kNoProperties, 2),
PURE(StringLessThanOrEqual, Operator::kNoProperties, 2),
PURE(StringAdd, 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