From 456202730cb6152660c5f1b1ca36298180a2d641 Mon Sep 17 00:00:00 2001 From: Darius M Date: Tue, 16 Aug 2022 18:06:13 +0200 Subject: [PATCH] [compiler] Remove map check in StringRef::length The "length" field of strings should never be mutated once it has been initialized. This means that the checks done by StringRef::length were never really useful. This CL thus removes them. Bug: chromium:1352386 Change-Id: I49f681daad119553eb0d4f1c2315bff5138197d7 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3829541 Commit-Queue: Darius Mercadier Reviewed-by: Tobias Tebbi Cr-Commit-Position: refs/heads/main@{#82536} --- src/compiler/heap-refs.cc | 11 +--------- src/compiler/heap-refs.h | 2 +- src/compiler/js-call-reducer.cc | 21 ++++++++----------- .../js-native-context-specialization.cc | 7 ++----- src/compiler/js-typed-lowering.cc | 7 ++----- src/compiler/typed-optimization.cc | 16 +++++++------- 6 files changed, 22 insertions(+), 42 deletions(-) diff --git a/src/compiler/heap-refs.cc b/src/compiler/heap-refs.cc index 7365cad65f..30dcb1ba22 100644 --- a/src/compiler/heap-refs.cc +++ b/src/compiler/heap-refs.cc @@ -1287,16 +1287,7 @@ base::Optional> StringRef::ObjectIfContentAccessible() { } } -base::Optional StringRef::length() const { - if (data_->kind() == kNeverSerializedHeapObject && !SupportedStringKind()) { - TRACE_BROKER_MISSING( - broker(), - "length for kNeverSerialized unsupported string kind " << *this); - return base::nullopt; - } else { - return object()->length(kAcquireLoad); - } -} +int StringRef::length() const { return object()->length(kAcquireLoad); } base::Optional StringRef::GetFirstChar() const { return GetChar(0); } diff --git a/src/compiler/heap-refs.h b/src/compiler/heap-refs.h index 5cee9e8f29..1ff9c99f19 100644 --- a/src/compiler/heap-refs.h +++ b/src/compiler/heap-refs.h @@ -922,7 +922,7 @@ class StringRef : public NameRef { // When concurrently accessing non-read-only non-supported strings, we return // base::nullopt for these methods. base::Optional> ObjectIfContentAccessible(); - base::Optional length() const; + int length() const; base::Optional GetFirstChar() const; base::Optional GetChar(int index) const; base::Optional ToNumber(); diff --git a/src/compiler/js-call-reducer.cc b/src/compiler/js-call-reducer.cc index 82c3b1a6d4..583ec42832 100644 --- a/src/compiler/js-call-reducer.cc +++ b/src/compiler/js-call-reducer.cc @@ -1204,7 +1204,7 @@ TNode JSCallReducerAssembler::ReduceStringPrototypeStartsWith( TNode zero = ZeroConstant(); TNode clamped_start = NumberMin(NumberMax(start_smi, zero), length); - int search_string_length = search_element_string.length().value(); + int search_string_length = search_element_string.length(); DCHECK(search_string_length <= JSCallReducer::kMaxInlineMatchSequence); auto out = MakeLabel(MachineRepresentation::kTagged); @@ -6660,17 +6660,14 @@ Reduction JSCallReducer::ReduceStringPrototypeStartsWith(Node* node) { ObjectRef target_ref = search_element_matcher.Ref(broker()); if (!target_ref.IsString()) return NoChange(); StringRef search_element_string = target_ref.AsString(); - if (search_element_string.length().has_value()) { - int length = search_element_string.length().value(); - // If search_element's length is less or equal than - // kMaxInlineMatchSequence, we inline the entire - // matching sequence. - if (length <= kMaxInlineMatchSequence) { - JSCallReducerAssembler a(this, node); - Node* subgraph = - a.ReduceStringPrototypeStartsWith(search_element_string); - return ReplaceWithSubgraph(&a, subgraph); - } + int length = search_element_string.length(); + // If search_element's length is less or equal than + // kMaxInlineMatchSequence, we inline the entire + // matching sequence. + if (length <= kMaxInlineMatchSequence) { + JSCallReducerAssembler a(this, node); + Node* subgraph = a.ReduceStringPrototypeStartsWith(search_element_string); + return ReplaceWithSubgraph(&a, subgraph); } } diff --git a/src/compiler/js-native-context-specialization.cc b/src/compiler/js-native-context-specialization.cc index 9056a6d3c8..2fb0f10794 100644 --- a/src/compiler/js-native-context-specialization.cc +++ b/src/compiler/js-native-context-specialization.cc @@ -1587,8 +1587,7 @@ Reduction JSNativeContextSpecialization::ReduceJSLoadNamed(Node* node) { } else if (object.IsString() && name.equals(MakeRef(broker(), factory()->length_string()))) { // Constant-fold "length" property on constant strings. - if (!object.AsString().length().has_value()) return NoChange(); - Node* value = jsgraph()->Constant(object.AsString().length().value()); + Node* value = jsgraph()->Constant(object.AsString().length()); ReplaceWithValue(node, value); return Replace(value); } @@ -2171,9 +2170,7 @@ Reduction JSNativeContextSpecialization::ReduceElementLoadFromHeapConstant( if (receiver_ref.IsString()) { DCHECK_NE(access_mode, AccessMode::kHas); // Ensure that {key} is less than {receiver} length. - if (!receiver_ref.AsString().length().has_value()) return NoChange(); - Node* length = - jsgraph()->Constant(receiver_ref.AsString().length().value()); + Node* length = jsgraph()->Constant(receiver_ref.AsString().length()); // Load the single character string from {receiver} or yield // undefined if the {key} is out of bounds (depending on the diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc index cb07e21bef..7563b69a4f 100644 --- a/src/compiler/js-typed-lowering.cc +++ b/src/compiler/js-typed-lowering.cc @@ -110,14 +110,11 @@ class JSBinopReduction final { JSHeapBroker* broker = lowering_->broker(); if (m.right().HasResolvedValue() && m.right().Ref(broker).IsString()) { StringRef right_string = m.right().Ref(broker).AsString(); - if (right_string.length().has_value() && - right_string.length().value() >= ConsString::kMinLength) - return true; + if (right_string.length() >= ConsString::kMinLength) return true; } if (m.left().HasResolvedValue() && m.left().Ref(broker).IsString()) { StringRef left_string = m.left().Ref(broker).AsString(); - if (left_string.length().has_value() && - left_string.length().value() >= ConsString::kMinLength) { + if (left_string.length() >= ConsString::kMinLength) { // The invariant for ConsString requires the left hand side to be // a sequential or external string if the right hand side is the // empty string. Since we don't know anything about the right hand diff --git a/src/compiler/typed-optimization.cc b/src/compiler/typed-optimization.cc index d06e13550e..bdeb44188c 100644 --- a/src/compiler/typed-optimization.cc +++ b/src/compiler/typed-optimization.cc @@ -444,7 +444,7 @@ Reduction TypedOptimization:: Node* comparison, const StringRef& string, bool inverted) { switch (comparison->opcode()) { case IrOpcode::kStringEqual: - if (string.length().has_value() && string.length().value() != 1) { + if (string.length() != 1) { // String.fromCharCode(x) always has length 1. return Replace(jsgraph()->BooleanConstant(false)); } @@ -452,7 +452,7 @@ Reduction TypedOptimization:: case IrOpcode::kStringLessThan: V8_FALLTHROUGH; case IrOpcode::kStringLessThanOrEqual: - if (string.length().has_value() && string.length().value() == 0) { + if (string.length() == 0) { // String.fromCharCode(x) <= "" is always false, // "" < String.fromCharCode(x) is always true. return Replace(jsgraph()->BooleanConstant(inverted)); @@ -500,7 +500,7 @@ TypedOptimization::TryReduceStringComparisonOfStringFromSingleCharCode( Node* number_comparison = nullptr; if (inverted) { // "x..." <= String.fromCharCode(z) is true if x < z. - if (string.length().has_value() && string.length().value() > 1 && + if (string.length() > 1 && comparison->opcode() == IrOpcode::kStringLessThanOrEqual) { comparison_op = simplified()->NumberLessThan(); } @@ -508,7 +508,7 @@ TypedOptimization::TryReduceStringComparisonOfStringFromSingleCharCode( graph()->NewNode(comparison_op, constant_repl, from_char_code_repl); } else { // String.fromCharCode(z) < "x..." is true if z <= x. - if (string.length().has_value() && string.length().value() > 1 && + if (string.length() > 1 && comparison->opcode() == IrOpcode::kStringLessThan) { comparison_op = simplified()->NumberLessThanOrEqual(); } @@ -570,11 +570,9 @@ Reduction TypedOptimization::ReduceStringLength(Node* node) { // Constant-fold the String::length of the {input}. HeapObjectMatcher m(input); if (m.Ref(broker()).IsString()) { - if (m.Ref(broker()).AsString().length().has_value()) { - uint32_t const length = m.Ref(broker()).AsString().length().value(); - Node* value = jsgraph()->Constant(length); - return Replace(value); - } + uint32_t const length = m.Ref(broker()).AsString().length(); + Node* value = jsgraph()->Constant(length); + return Replace(value); } break; }