From 15928708228db0e02218746c37086f58576be206 Mon Sep 17 00:00:00 2001 From: svenpanne Date: Thu, 2 Apr 2015 01:32:48 -0700 Subject: [PATCH] Fixed the range information for string lengths. Currently, this doesn't really help to generate better code, nevertheless this is the right thing to do. When our type system(s) are fixed, this should avoid falling back to floating point operations in various cases. Review URL: https://codereview.chromium.org/1057813002 Cr-Commit-Position: refs/heads/master@{#27578} --- src/compiler/access-builder.cc | 5 ++--- src/compiler/access-builder.h | 2 +- src/compiler/js-intrinsic-lowering.cc | 3 ++- src/compiler/js-typed-lowering.cc | 4 ++-- src/compiler/typer.cc | 4 +--- .../compiler/js-intrinsic-lowering-unittest.cc | 5 +++-- .../compiler/js-typed-lowering-unittest.cc | 13 +++++++------ 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/compiler/access-builder.cc b/src/compiler/access-builder.cc index c69f22cb87..9e5a0bbb8d 100644 --- a/src/compiler/access-builder.cc +++ b/src/compiler/access-builder.cc @@ -59,10 +59,9 @@ FieldAccess AccessBuilder::ForMapInstanceType() { // static -FieldAccess AccessBuilder::ForStringLength() { +FieldAccess AccessBuilder::ForStringLength(Zone* zone) { return {kTaggedBase, String::kLengthOffset, Handle(), - Type::Intersect(Type::UnsignedSmall(), Type::TaggedSigned()), - kMachAnyTagged}; + Type::Range(0, String::kMaxLength, zone), kMachAnyTagged}; } diff --git a/src/compiler/access-builder.h b/src/compiler/access-builder.h index ddbad8c64e..6297b28b1e 100644 --- a/src/compiler/access-builder.h +++ b/src/compiler/access-builder.h @@ -38,7 +38,7 @@ class AccessBuilder FINAL : public AllStatic { static FieldAccess ForMapInstanceType(); // Provides access to String::length() field. - static FieldAccess ForStringLength(); + static FieldAccess ForStringLength(Zone* zone); // Provides access to JSValue::value() field. static FieldAccess ForValue(); diff --git a/src/compiler/js-intrinsic-lowering.cc b/src/compiler/js-intrinsic-lowering.cc index 71fd0e9573..6795317cff 100644 --- a/src/compiler/js-intrinsic-lowering.cc +++ b/src/compiler/js-intrinsic-lowering.cc @@ -294,7 +294,8 @@ Reduction JSIntrinsicLowering::ReduceStringGetLength(Node* node) { Node* value = NodeProperties::GetValueInput(node, 0); Node* effect = NodeProperties::GetEffectInput(node); Node* control = NodeProperties::GetControlInput(node); - return Change(node, simplified()->LoadField(AccessBuilder::ForStringLength()), + return Change(node, simplified()->LoadField( + AccessBuilder::ForStringLength(graph()->zone())), value, effect, control); } diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc index 244cfe2dba..3d80a275f3 100644 --- a/src/compiler/js-typed-lowering.cc +++ b/src/compiler/js-typed-lowering.cc @@ -541,7 +541,7 @@ Reduction JSTypedLowering::ReduceJSUnaryNot(Node* node) { return Changed(node); } else if (input_type->Is(Type::String())) { // JSUnaryNot(x:string) => NumberEqual(x.length,#0) - FieldAccess const access = AccessBuilder::ForStringLength(); + FieldAccess const access = AccessBuilder::ForStringLength(graph()->zone()); // It is safe for the load to be effect-free (i.e. not linked into effect // chain) because we assume String::length to be immutable. Node* length = graph()->NewNode(simplified()->LoadField(access), input, @@ -572,7 +572,7 @@ Reduction JSTypedLowering::ReduceJSToBoolean(Node* node) { return Changed(node); } else if (input_type->Is(Type::String())) { // JSToBoolean(x:string) => NumberLessThan(#0,x.length) - FieldAccess const access = AccessBuilder::ForStringLength(); + FieldAccess const access = AccessBuilder::ForStringLength(graph()->zone()); // It is safe for the load to be effect-free (i.e. not linked into effect // chain) because we assume String::length to be immutable. Node* length = graph()->NewNode(simplified()->LoadField(access), input, diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc index 9da27443d9..1507f20dc8 100644 --- a/src/compiler/typer.cc +++ b/src/compiler/typer.cc @@ -1580,9 +1580,7 @@ Bounds Typer::Visitor::TypeJSCallRuntime(Node* node) { case Runtime::kInlineMathClz32: return Bounds(Type::None(), Type::Range(0, 32, zone())); case Runtime::kInlineStringGetLength: - // The string::length property is always an unsigned smi. - return Bounds(Type::None(), Type::Intersect(Type::UnsignedSmall(), - Type::TaggedSigned())); + return Bounds(Type::None(), Type::Range(0, String::kMaxLength, zone())); default: break; } diff --git a/test/unittests/compiler/js-intrinsic-lowering-unittest.cc b/test/unittests/compiler/js-intrinsic-lowering-unittest.cc index 582d78ad3d..7ef004d975 100644 --- a/test/unittests/compiler/js-intrinsic-lowering-unittest.cc +++ b/test/unittests/compiler/js-intrinsic-lowering-unittest.cc @@ -316,8 +316,9 @@ TEST_F(JSIntrinsicLoweringTest, InlineStringGetLength) { javascript()->CallRuntime(Runtime::kInlineStringGetLength, 1), input, context, effect, control)); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(r.replacement(), IsLoadField(AccessBuilder::ForStringLength(), - input, effect, control)); + EXPECT_THAT(r.replacement(), + IsLoadField(AccessBuilder::ForStringLength(zone()), input, effect, + control)); } diff --git a/test/unittests/compiler/js-typed-lowering-unittest.cc b/test/unittests/compiler/js-typed-lowering-unittest.cc index d347c4139b..8bb5798bd2 100644 --- a/test/unittests/compiler/js-typed-lowering-unittest.cc +++ b/test/unittests/compiler/js-typed-lowering-unittest.cc @@ -190,10 +190,11 @@ TEST_F(JSTypedLoweringTest, JSUnaryNotWithString) { Reduction r = Reduce(graph()->NewNode(javascript()->UnaryNot(), input, context)); ASSERT_TRUE(r.Changed()); - EXPECT_THAT(r.replacement(), - IsNumberEqual(IsLoadField(AccessBuilder::ForStringLength(), input, - graph()->start(), graph()->start()), - IsNumberConstant(0.0))); + EXPECT_THAT( + r.replacement(), + IsNumberEqual(IsLoadField(AccessBuilder::ForStringLength(zone()), input, + graph()->start(), graph()->start()), + IsNumberConstant(0.0))); } @@ -392,8 +393,8 @@ TEST_F(JSTypedLoweringTest, JSToBooleanWithString) { EXPECT_THAT( r.replacement(), IsNumberLessThan(IsNumberConstant(0.0), - IsLoadField(AccessBuilder::ForStringLength(), input, - graph()->start(), graph()->start()))); + IsLoadField(AccessBuilder::ForStringLength(zone()), + input, graph()->start(), graph()->start()))); }