From 7a3a6e88bd8254c6bb85e6732a9636a38773a689 Mon Sep 17 00:00:00 2001 From: Jakob Linke Date: Mon, 23 Jan 2023 07:33:00 +0000 Subject: [PATCH] Revert "[turbofan] Optimize access to the length property of functions" This reverts commit 7eb8937bca68a57067012e36457ef0e5588b5f2a. Reason for revert: crbug.com/1408957 Original change's description: > [turbofan] Optimize access to the length property of functions > > When compiling to JavaScript a language that supports curryfication, it > is convenient to be able to efficiently get the arity of a function to > check for partial application. > > Change-Id: I6611b523b2c3795f1f8fb123f63f5b6d604d793d > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4111447 > Reviewed-by: Jakob Linke > Commit-Queue: Jakob Linke > Cr-Commit-Position: refs/heads/main@{#85409} Fixed: chromium:1408957 Change-Id: I5200392af7532a864afd73fb0e88be9a2153a312 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4187075 Commit-Queue: Jakob Linke Bot-Commit: Rubber Stamper Cr-Commit-Position: refs/heads/main@{#85428} --- AUTHORS | 1 - src/compiler/access-builder.cc | 13 -------- src/compiler/access-builder.h | 3 -- src/compiler/access-info.cc | 17 +--------- src/compiler/access-info.h | 5 +-- src/compiler/effect-control-linearizer.cc | 12 ------- .../js-native-context-specialization.cc | 3 -- src/compiler/opcodes.h | 1 - src/compiler/simplified-lowering.cc | 5 --- src/compiler/simplified-operator.cc | 1 - src/compiler/simplified-operator.h | 2 -- src/compiler/typer.cc | 4 --- src/compiler/verifier.cc | 4 --- src/maglev/arm64/maglev-ir-arm64.cc | 16 ---------- src/maglev/maglev-graph-builder.cc | 7 ----- src/maglev/maglev-ir.h | 20 ------------ src/maglev/x64/maglev-ir-x64.cc | 15 --------- test/mjsunit/compiler/function-length.js | 31 ------------------- 18 files changed, 2 insertions(+), 158 deletions(-) delete mode 100644 test/mjsunit/compiler/function-length.js diff --git a/AUTHORS b/AUTHORS index a8b0faafa6..21f0b915bb 100644 --- a/AUTHORS +++ b/AUTHORS @@ -147,7 +147,6 @@ Jan de Mooij Janusz Majnert Javad Amiri Jay Freeman -Jérôme Vouillon Jesper van den Ende Ji Qiu Jiawen Geng diff --git a/src/compiler/access-builder.cc b/src/compiler/access-builder.cc index 99e73f9295..a993079430 100644 --- a/src/compiler/access-builder.cc +++ b/src/compiler/access-builder.cc @@ -213,19 +213,6 @@ FieldAccess AccessBuilder::ForJSFunctionSharedFunctionInfo() { return access; } -// static -FieldAccess AccessBuilder::ForSharedFunctionInfoLength() { - FieldAccess access = {kTaggedBase, - SharedFunctionInfo::kLengthOffset, - Handle(), - MaybeHandle(), - TypeCache::Get()->kArgumentsLengthType, - MachineType::Uint16(), - kNoWriteBarrier, - "SharedFunctionInfoLength"}; - return access; -} - // static FieldAccess AccessBuilder::ForJSFunctionFeedbackCell() { FieldAccess access = {kTaggedBase, JSFunction::kFeedbackCellOffset, diff --git a/src/compiler/access-builder.h b/src/compiler/access-builder.h index 7d6562442c..cbb71faf0c 100644 --- a/src/compiler/access-builder.h +++ b/src/compiler/access-builder.h @@ -92,9 +92,6 @@ class V8_EXPORT_PRIVATE AccessBuilder final // Provides access to JSFunction::shared() field. static FieldAccess ForJSFunctionSharedFunctionInfo(); - // Provides access to SharedFunctionInfo::length() field. - static FieldAccess ForSharedFunctionInfoLength(); - // Provides access to JSFunction::feedback_cell() field. static FieldAccess ForJSFunctionFeedbackCell(); diff --git a/src/compiler/access-info.cc b/src/compiler/access-info.cc index 276b315465..1e1c8cd215 100644 --- a/src/compiler/access-info.cc +++ b/src/compiler/access-info.cc @@ -161,12 +161,6 @@ PropertyAccessInfo PropertyAccessInfo::StringLength(Zone* zone, return PropertyAccessInfo(zone, kStringLength, {}, {{receiver_map}, zone}); } -// static -PropertyAccessInfo PropertyAccessInfo::FunctionLength(Zone* zone, - MapRef receiver_map) { - return PropertyAccessInfo(zone, kFunctionLength, {}, {{receiver_map}, zone}); -} - // static PropertyAccessInfo PropertyAccessInfo::DictionaryProtoDataConstant( Zone* zone, MapRef receiver_map, JSObjectRef holder, @@ -349,8 +343,7 @@ bool PropertyAccessInfo::Merge(PropertyAccessInfo const* that, } case kNotFound: - case kStringLength: - case kFunctionLength: { + case kStringLength: { DCHECK(unrecorded_dependencies_.empty()); DCHECK(that->unrecorded_dependencies_.empty()); AppendVector(&lookup_start_object_maps_, that->lookup_start_object_maps_); @@ -1049,14 +1042,6 @@ PropertyAccessInfo AccessInfoFactory::LookupSpecialFieldAccessor( } return Invalid(); } - // Check for JSFunction::length field accessor. - if (map.object()->IsJSFunctionMap()) { - if (Name::Equals(isolate(), name.object(), - isolate()->factory()->length_string())) { - return PropertyAccessInfo::FunctionLength(zone(), map); - } - return Invalid(); - } // Check for special JSObject field accessors. FieldIndex field_index; if (Accessors::IsJSObjectFieldAccessor(isolate(), map.object(), name.object(), diff --git a/src/compiler/access-info.h b/src/compiler/access-info.h index 14f236ba36..d75e8d7b2b 100644 --- a/src/compiler/access-info.h +++ b/src/compiler/access-info.h @@ -65,8 +65,7 @@ class PropertyAccessInfo final { kFastAccessorConstant, kDictionaryProtoAccessorConstant, kModuleExport, - kStringLength, - kFunctionLength + kStringLength }; static PropertyAccessInfo NotFound(Zone* zone, MapRef receiver_map, @@ -92,7 +91,6 @@ class PropertyAccessInfo final { static PropertyAccessInfo ModuleExport(Zone* zone, MapRef receiver_map, CellRef cell); static PropertyAccessInfo StringLength(Zone* zone, MapRef receiver_map); - static PropertyAccessInfo FunctionLength(Zone* zone, MapRef receiver_map); static PropertyAccessInfo Invalid(Zone* zone); static PropertyAccessInfo DictionaryProtoDataConstant( Zone* zone, MapRef receiver_map, JSObjectRef holder, @@ -115,7 +113,6 @@ class PropertyAccessInfo final { } bool IsModuleExport() const { return kind() == kModuleExport; } bool IsStringLength() const { return kind() == kStringLength; } - bool IsFunctionLength() const { return kind() == kFunctionLength; } bool IsDictionaryProtoDataConstant() const { return kind() == kDictionaryProtoDataConstant; } diff --git a/src/compiler/effect-control-linearizer.cc b/src/compiler/effect-control-linearizer.cc index 0bf5426287..5068d6eb83 100644 --- a/src/compiler/effect-control-linearizer.cc +++ b/src/compiler/effect-control-linearizer.cc @@ -180,7 +180,6 @@ class EffectControlLinearizer { Node* LowerStringEqual(Node* node); Node* LowerStringLessThan(Node* node); Node* LowerStringLessThanOrEqual(Node* node); - Node* LowerFunctionLength(Node* node); Node* LowerBigIntAdd(Node* node, Node* frame_state); Node* LowerBigIntSubtract(Node* node, Node* frame_state); Node* LowerBigIntMultiply(Node* node, Node* frame_state); @@ -1276,9 +1275,6 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node, case IrOpcode::kStringLessThanOrEqual: result = LowerStringLessThanOrEqual(node); break; - case IrOpcode::kFunctionLength: - result = LowerFunctionLength(node); - break; case IrOpcode::kBigIntAdd: result = LowerBigIntAdd(node, frame_state); break; @@ -4581,14 +4577,6 @@ Node* EffectControlLinearizer::LowerStringLessThanOrEqual(Node* node) { Builtins::CallableFor(isolate(), Builtin::kStringLessThanOrEqual), node); } -Node* EffectControlLinearizer::LowerFunctionLength(Node* node) { - Node* subject = node->InputAt(0); - - auto shared = - __ LoadField(AccessBuilder::ForJSFunctionSharedFunctionInfo(), subject); - return __ LoadField(AccessBuilder::ForSharedFunctionInfoLength(), shared); -} - Node* EffectControlLinearizer::LowerBigIntAdd(Node* node, Node* frame_state) { Node* lhs = node->InputAt(0); Node* rhs = node->InputAt(1); diff --git a/src/compiler/js-native-context-specialization.cc b/src/compiler/js-native-context-specialization.cc index 8e387d2fa9..b2db98f03e 100644 --- a/src/compiler/js-native-context-specialization.cc +++ b/src/compiler/js-native-context-specialization.cc @@ -2798,9 +2798,6 @@ JSNativeContextSpecialization::BuildPropertyLoad( } else if (access_info.IsStringLength()) { DCHECK_EQ(receiver, lookup_start_object); value = graph()->NewNode(simplified()->StringLength(), receiver); - } else if (access_info.IsFunctionLength()) { - DCHECK_EQ(receiver, lookup_start_object); - value = graph()->NewNode(simplified()->FunctionLength(), receiver); } else { DCHECK(access_info.IsDataField() || access_info.IsFastDataConstant() || access_info.IsDictionaryProtoDataConstant()); diff --git a/src/compiler/opcodes.h b/src/compiler/opcodes.h index e0d7d1ad92..8b9ea40505 100644 --- a/src/compiler/opcodes.h +++ b/src/compiler/opcodes.h @@ -523,7 +523,6 @@ V(StringToLowerCaseIntl) \ V(StringToNumber) \ V(StringToUpperCaseIntl) \ - V(FunctionLength) \ V(ToBoolean) \ V(TransitionAndStoreElement) \ V(TransitionAndStoreNonNumberElement) \ diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc index 1b3e83913c..da4ec71f74 100644 --- a/src/compiler/simplified-lowering.cc +++ b/src/compiler/simplified-lowering.cc @@ -3595,11 +3595,6 @@ class RepresentationSelector { MachineRepresentation::kTaggedPointer); return; } - case IrOpcode::kFunctionLength: { - VisitUnop(node, UseInfo::AnyTagged(), - MachineRepresentation::kWord32); - return; - } case IrOpcode::kCheckBounds: return VisitCheckBounds(node, lowering); case IrOpcode::kCheckHeapObject: { diff --git a/src/compiler/simplified-operator.cc b/src/compiler/simplified-operator.cc index fc0d776a39..23e77c56e8 100644 --- a/src/compiler/simplified-operator.cc +++ b/src/compiler/simplified-operator.cc @@ -808,7 +808,6 @@ bool operator==(CheckMinusZeroParameters const& lhs, V(StringLength, Operator::kNoProperties, 1, 0) \ V(StringToLowerCaseIntl, Operator::kNoProperties, 1, 0) \ V(StringToUpperCaseIntl, Operator::kNoProperties, 1, 0) \ - V(FunctionLength, Operator::kNoProperties, 1, 0) \ V(TypeOf, Operator::kNoProperties, 1, 1) \ V(PlainPrimitiveToNumber, Operator::kNoProperties, 1, 0) \ V(PlainPrimitiveToWord32, Operator::kNoProperties, 1, 0) \ diff --git a/src/compiler/simplified-operator.h b/src/compiler/simplified-operator.h index b28fe2e915..f602a62e55 100644 --- a/src/compiler/simplified-operator.h +++ b/src/compiler/simplified-operator.h @@ -911,8 +911,6 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final const Operator* StringToUpperCaseIntl(); const Operator* StringSubstring(); - const Operator* FunctionLength(); - const Operator* FindOrderedHashMapEntryForInt32Key(); const Operator* FindOrderedCollectionEntry(CollectionKind collection_kind); diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc index abfc9eba54..1df9506319 100644 --- a/src/compiler/typer.cc +++ b/src/compiler/typer.cc @@ -2287,10 +2287,6 @@ Type Typer::Visitor::TypeStringLength(Node* node) { Type Typer::Visitor::TypeStringSubstring(Node* node) { return Type::String(); } -Type Typer::Visitor::TypeFunctionLength(Node* node) { - return Type::Range(0, InstructionStream::kMaxArguments, zone()); -} - Type Typer::Visitor::TypeCheckBounds(Node* node) { return typer_->operation_typer_.CheckBounds(Operand(node, 0), Operand(node, 1)); diff --git a/src/compiler/verifier.cc b/src/compiler/verifier.cc index eb7dc7d091..8a33e42e0b 100644 --- a/src/compiler/verifier.cc +++ b/src/compiler/verifier.cc @@ -1227,10 +1227,6 @@ void Verifier::Visitor::Check(Node* node, const AllNodes& all) { CheckValueInputIs(node, 2, Type::SignedSmall()); CheckTypeIs(node, Type::String()); break; - case IrOpcode::kFunctionLength: - CheckValueInputIs(node, 0, Type::Any()); - CheckTypeIs(node, TypeCache::Get()->kArgumentsLengthType); - break; case IrOpcode::kReferenceEqual: CheckTypeIs(node, Type::Boolean()); break; diff --git a/src/maglev/arm64/maglev-ir-arm64.cc b/src/maglev/arm64/maglev-ir-arm64.cc index 7b1629e84e..4d600d4068 100644 --- a/src/maglev/arm64/maglev-ir-arm64.cc +++ b/src/maglev/arm64/maglev-ir-arm64.cc @@ -2481,22 +2481,6 @@ void StringLength::GenerateCode(MaglevAssembler* masm, FieldMemOperand(object, String::kLengthOffset)); } -void FunctionLength::SetValueLocationConstraints() { - UseRegister(object_input()); - DefineAsRegister(this); -} -void FunctionLength::GenerateCode(MaglevAssembler* masm, - const ProcessingState& state) { - Register object = ToRegister(object_input()); - __ AssertFunction(object); - UseScratchRegisterScope temps(masm); - Register shared = temps.AcquireX(); - __ LoadTaggedPointerField( - shared, FieldMemOperand(object, JSFunction::kSharedFunctionInfoOffset)); - __ Ldr(ToRegister(result()).W(), - FieldMemOperand(shared, SharedFunctionInfo::kLengthOffset)); -} - void TestUndetectable::SetValueLocationConstraints() { UseRegister(value()); DefineAsRegister(this); diff --git a/src/maglev/maglev-graph-builder.cc b/src/maglev/maglev-graph-builder.cc index 276faf0173..a6795173d4 100644 --- a/src/maglev/maglev-graph-builder.cc +++ b/src/maglev/maglev-graph-builder.cc @@ -1960,13 +1960,6 @@ bool MaglevGraphBuilder::TryBuildPropertyLoad( current_interpreter_frame_.accumulator(), access_info); return true; - case compiler::PropertyAccessInfo::kFunctionLength: - DCHECK_EQ(receiver, lookup_start_object); - SetAccumulator(AddNewNode({receiver})); - RecordKnownProperty(lookup_start_object, name, - current_interpreter_frame_.accumulator(), - access_info); - return true; } } diff --git a/src/maglev/maglev-ir.h b/src/maglev/maglev-ir.h index 8d2ee01a2f..4ac5ebd2ef 100644 --- a/src/maglev/maglev-ir.h +++ b/src/maglev/maglev-ir.h @@ -211,7 +211,6 @@ class CompactInterpreterFrameState; V(SetPendingMessage) \ V(StringAt) \ V(StringLength) \ - V(FunctionLength) \ V(ToBoolean) \ V(ToBooleanLogicalNot) \ V(TaggedEqual) \ @@ -4660,25 +4659,6 @@ class StringLength : public FixedInputValueNodeT<1, StringLength> { void PrintParams(std::ostream&, MaglevGraphLabeller*) const {} }; -class FunctionLength : public FixedInputValueNodeT<1, FunctionLength> { - using Base = FixedInputValueNodeT<1, FunctionLength>; - - public: - explicit FunctionLength(uint64_t bitfield) : Base(bitfield) {} - - static constexpr OpProperties kProperties = - OpProperties::Reading() | OpProperties::Int32(); - static constexpr - typename Base::InputTypes kInputTypes{ValueRepresentation::kTagged}; - - static constexpr int kObjectIndex = 0; - Input& object_input() { return input(kObjectIndex); } - - void SetValueLocationConstraints(); - void GenerateCode(MaglevAssembler*, const ProcessingState&); - void PrintParams(std::ostream&, MaglevGraphLabeller*) const {} -}; - class DefineNamedOwnGeneric : public FixedInputValueNodeT<3, DefineNamedOwnGeneric> { using Base = FixedInputValueNodeT<3, DefineNamedOwnGeneric>; diff --git a/src/maglev/x64/maglev-ir-x64.cc b/src/maglev/x64/maglev-ir-x64.cc index 955dfd690f..4992fa0fee 100644 --- a/src/maglev/x64/maglev-ir-x64.cc +++ b/src/maglev/x64/maglev-ir-x64.cc @@ -1331,21 +1331,6 @@ void StringLength::GenerateCode(MaglevAssembler* masm, __ movl(ToRegister(result()), FieldOperand(object, String::kLengthOffset)); } -void FunctionLength::SetValueLocationConstraints() { - UseRegister(object_input()); - DefineAsRegister(this); -} -void FunctionLength::GenerateCode(MaglevAssembler* masm, - const ProcessingState& state) { - Register object = ToRegister(object_input()); - __ AssertFunction(object); - __ LoadTaggedPointerField( - kScratchRegister, - FieldOperand(object, JSFunction::kSharedFunctionInfoOffset)); - __ movl(ToRegister(result()), - FieldOperand(kScratchRegister, SharedFunctionInfo::kLengthOffset)); -} - void Int32AddWithOverflow::SetValueLocationConstraints() { UseRegister(left_input()); UseRegister(right_input()); diff --git a/test/mjsunit/compiler/function-length.js b/test/mjsunit/compiler/function-length.js deleted file mode 100644 index 19d93efa89..0000000000 --- a/test/mjsunit/compiler/function-length.js +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2023 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -// Flags: --allow-natives-syntax --turbofan --no-always-turbofan - -function f(g) { - return g.length; -} -function g(x, y) {} -function h(x, y, z) {} -function OptimizeAndTest(fn) { - %PrepareFunctionForOptimization(fn); - assertEquals(1, fn(f)); - assertEquals(2, fn(g)); - assertEquals(3, fn(h)); - - %OptimizeFunctionOnNextCall(fn); - fn(g); - assertOptimized(fn); - - assertEquals(1, fn(f)); - assertEquals(2, fn(g)); - assertEquals(3, fn(h)); - assertOptimized(fn); - - assertEquals(3, fn('abc')); - assertUnoptimized(fn); -} - -OptimizeAndTest(f);