From 7eb43bf494653b9852d56bd941f3a414a6b5ccd7 Mon Sep 17 00:00:00 2001 From: Qifan Pan Date: Tue, 25 Oct 2022 10:15:55 +0200 Subject: [PATCH] [turbofan] Decompose CheckBigInt64 and make it precise This CL solves two problems: - Eliminating redundant CheckBigInt/CheckBigInt64 by decomposing CheckBigInt64 to CheckBigInt and CheckedBigIntToBigInt64. - Having precise checks for SignedBigInt64 to make the range of BigInt64 consistent in CheckedBigInt64Ops and CheckedBigIntToBigInt64. Otherwise, there would be semantic difference between the subgraphs where we keep CheckBigInt64 inbetween two CheckedBigInt64Ops (e.g., the variant assert_types) and the subgraphs where we eliminate the checks. Bug: v8:9407 Change-Id: I79a5c99e12eb3f3ffc7b5cbfc51191e6792f634b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3960333 Commit-Queue: Qifan Pan Reviewed-by: Nico Hartmann Cr-Commit-Position: refs/heads/main@{#83899} --- src/compiler/effect-control-linearizer.cc | 48 ++++++++++--------- src/compiler/opcodes.h | 2 +- src/compiler/operation-typer.cc | 4 +- src/compiler/redundancy-elimination.cc | 13 +++-- src/compiler/representation-change.cc | 10 +++- src/compiler/simplified-lowering-verifier.cc | 3 +- src/compiler/simplified-lowering.cc | 18 ------- src/compiler/simplified-operator.cc | 2 +- src/compiler/simplified-operator.h | 2 +- src/compiler/verifier.cc | 4 +- src/deoptimizer/deoptimize-reason.h | 1 + .../compiler/bigint64-add-no-deopt-loop.js | 42 +++++++++++++--- test/mjsunit/mjsunit.status | 1 - 13 files changed, 87 insertions(+), 63 deletions(-) diff --git a/src/compiler/effect-control-linearizer.cc b/src/compiler/effect-control-linearizer.cc index 1036e875d1..ca7c2571ac 100644 --- a/src/compiler/effect-control-linearizer.cc +++ b/src/compiler/effect-control-linearizer.cc @@ -84,7 +84,7 @@ class EffectControlLinearizer { Node* LowerCheckReceiverOrNullOrUndefined(Node* node, Node* frame_state); Node* LowerCheckString(Node* node, Node* frame_state); Node* LowerCheckBigInt(Node* node, Node* frame_state); - Node* LowerCheckBigInt64(Node* node, Node* frame_state); + Node* LowerCheckedBigIntToBigInt64(Node* node, Node* frame_state); Node* LowerCheckSymbol(Node* node, Node* frame_state); void LowerCheckIf(Node* node, Node* frame_state); Node* LowerCheckedInt32Add(Node* node, Node* frame_state); @@ -1013,8 +1013,8 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node, case IrOpcode::kCheckBigInt: result = LowerCheckBigInt(node, frame_state); break; - case IrOpcode::kCheckBigInt64: - result = LowerCheckBigInt64(node, frame_state); + case IrOpcode::kCheckedBigIntToBigInt64: + result = LowerCheckedBigIntToBigInt64(node, frame_state); break; case IrOpcode::kCheckInternalizedString: result = LowerCheckInternalizedString(node, frame_state); @@ -2948,28 +2948,17 @@ Node* EffectControlLinearizer::LowerCheckBigInt(Node* node, Node* frame_state) { return value; } -Node* EffectControlLinearizer::LowerCheckBigInt64(Node* node, - Node* frame_state) { +Node* EffectControlLinearizer::LowerCheckedBigIntToBigInt64(Node* node, + Node* frame_state) { DCHECK(machine()->Is64()); auto done = __ MakeLabel(); auto if_not_zero = __ MakeLabel(); + auto if_may_be_out_of_range = __ MakeDeferredLabel(); Node* value = node->InputAt(0); const CheckParameters& params = CheckParametersOf(node->op()); - // Check for Smi. - Node* smi_check = ObjectIsSmi(value); - __ DeoptimizeIf(DeoptimizeReason::kSmi, params.feedback(), smi_check, - frame_state); - - // Check for BigInt. - Node* value_map = __ LoadField(AccessBuilder::ForMap(), value); - Node* bi_check = __ TaggedEqual(value_map, __ BigIntMapConstant()); - __ DeoptimizeIfNot(DeoptimizeReason::kWrongInstanceType, params.feedback(), - bi_check, frame_state); - - // Check for BigInt64. Node* bitfield = __ LoadField(AccessBuilder::ForBigIntBitfield(), value); __ GotoIfNot(__ Word32Equal(bitfield, __ Int32Constant(0)), &if_not_zero); __ Goto(&done); @@ -2980,19 +2969,32 @@ Node* EffectControlLinearizer::LowerCheckBigInt64(Node* node, Node* length = __ Word32And(bitfield, __ Int32Constant(BigInt::LengthBits::kMask)); __ DeoptimizeIfNot( - DeoptimizeReason::kWrongInstanceType, params.feedback(), + DeoptimizeReason::kNotABigInt64, params.feedback(), __ Word32Equal(length, __ Int32Constant(uint32_t{1} << BigInt::LengthBits::kShift)), frame_state); Node* lsd = __ LoadField(AccessBuilder::ForBigIntLeastSignificantDigit64(), value); - // Accepted small BigInts are in the range [-2^63 + 1, 2^63 - 1]. - // Excluding -2^63 from the range makes the check simpler and faster. - Node* bi64_check = __ Uint64LessThanOrEqual( + + Node* magnitude_check = __ Uint64LessThanOrEqual( lsd, __ Int64Constant(std::numeric_limits::max())); - __ DeoptimizeIfNot(DeoptimizeReason::kWrongInstanceType, params.feedback(), - bi64_check, frame_state); + __ Branch(magnitude_check, &done, &if_may_be_out_of_range); + + __ Bind(&if_may_be_out_of_range); + Node* sign = + __ Word32And(bitfield, __ Int32Constant(BigInt::SignBits::kMask)); + + Node* sign_check = + __ Word32Equal(sign, __ Int32Constant(BigInt::SignBits::kMask)); + __ DeoptimizeIfNot(DeoptimizeReason::kNotABigInt64, params.feedback(), + sign_check, frame_state); + + Node* min_check = __ Word64Equal( + lsd, __ Int64Constant(std::numeric_limits::min())); + __ DeoptimizeIfNot(DeoptimizeReason::kNotABigInt64, params.feedback(), + min_check, frame_state); + __ Goto(&done); } diff --git a/src/compiler/opcodes.h b/src/compiler/opcodes.h index f73bd43c52..7fe978c269 100644 --- a/src/compiler/opcodes.h +++ b/src/compiler/opcodes.h @@ -400,7 +400,7 @@ #define SIMPLIFIED_BIGINT_UNOP_LIST(V) \ V(BigIntNegate) \ V(CheckBigInt) \ - V(CheckBigInt64) + V(CheckedBigIntToBigInt64) #define SIMPLIFIED_SPECULATIVE_NUMBER_UNOP_LIST(V) V(SpeculativeToNumber) diff --git a/src/compiler/operation-typer.cc b/src/compiler/operation-typer.cc index 4279c54d1b..e57f9c744f 100644 --- a/src/compiler/operation-typer.cc +++ b/src/compiler/operation-typer.cc @@ -585,7 +585,9 @@ Type OperationTyper::SpeculativeBigIntAsUintN(Type) { Type OperationTyper::CheckBigInt(Type type) { return Type::BigInt(); } -Type OperationTyper::CheckBigInt64(Type type) { return Type::SignedBigInt64(); } +Type OperationTyper::CheckedBigIntToBigInt64(Type type) { + return Type::SignedBigInt64(); +} Type OperationTyper::NumberAdd(Type lhs, Type rhs) { DCHECK(lhs.Is(Type::Number())); diff --git a/src/compiler/redundancy-elimination.cc b/src/compiler/redundancy-elimination.cc index 9f1e15f131..9cfb03c18c 100644 --- a/src/compiler/redundancy-elimination.cc +++ b/src/compiler/redundancy-elimination.cc @@ -20,7 +20,7 @@ Reduction RedundancyElimination::Reduce(Node* node) { if (node_checks_.Get(node)) return NoChange(); switch (node->opcode()) { case IrOpcode::kCheckBigInt: - case IrOpcode::kCheckBigInt64: + case IrOpcode::kCheckedBigIntToBigInt64: case IrOpcode::kCheckBounds: case IrOpcode::kCheckClosure: case IrOpcode::kCheckEqualsInternalizedString: @@ -38,15 +38,14 @@ Reduction RedundancyElimination::Reduce(Node* node) { case IrOpcode::kCheckSymbol: // These are not really check nodes, but behave the same in that they can be // folded together if repeated with identical inputs. - case IrOpcode::kBigIntAdd: - case IrOpcode::kBigIntSubtract: case IrOpcode::kStringCharCodeAt: case IrOpcode::kStringCodePointAt: case IrOpcode::kStringFromCodePointAt: case IrOpcode::kStringSubstring: -#define SIMPLIFIED_CHECKED_OP(Opcode) case IrOpcode::k##Opcode: - SIMPLIFIED_CHECKED_OP_LIST(SIMPLIFIED_CHECKED_OP) -#undef SIMPLIFIED_CHECKED_OP +#define SIMPLIFIED_OP(Opcode) case IrOpcode::k##Opcode: + SIMPLIFIED_CHECKED_OP_LIST(SIMPLIFIED_OP) + SIMPLIFIED_BIGINT_BINOP_LIST(SIMPLIFIED_OP) +#undef SIMPLIFIED_OP return ReduceCheckNode(node); case IrOpcode::kSpeculativeNumberEqual: case IrOpcode::kSpeculativeNumberLessThan: @@ -165,7 +164,7 @@ bool CheckSubsumes(Node const* a, Node const* b) { case IrOpcode::kCheckString: case IrOpcode::kCheckNumber: case IrOpcode::kCheckBigInt: - case IrOpcode::kCheckBigInt64: + case IrOpcode::kCheckedBigIntToBigInt64: break; case IrOpcode::kCheckedInt32ToTaggedSigned: case IrOpcode::kCheckedInt64ToInt32: diff --git a/src/compiler/representation-change.cc b/src/compiler/representation-change.cc index 68e0cac877..7033337d6e 100644 --- a/src/compiler/representation-change.cc +++ b/src/compiler/representation-change.cc @@ -505,7 +505,15 @@ Node* RepresentationChanger::GetTaggedPointerRepresentationFor( if (output_type.Is(Type::SignedBigInt64())) { return node; } - op = simplified()->CheckBigInt64(use_info.feedback()); + if (!output_type.Is(Type::BigInt())) { + node = InsertConversion( + node, simplified()->CheckBigInt(use_info.feedback()), use_node); + } + op = simplified()->CheckedBigIntToBigInt64(use_info.feedback()); + } else if (output_rep == MachineRepresentation::kTaggedPointer || + !output_type.Maybe(Type::SignedSmall())) { + DCHECK_NE(output_rep, MachineRepresentation::kTaggedSigned); + return node; } else { return TypeError(node, output_rep, output_type, MachineRepresentation::kTaggedPointer); diff --git a/src/compiler/simplified-lowering-verifier.cc b/src/compiler/simplified-lowering-verifier.cc index b469a9f504..7839701abb 100644 --- a/src/compiler/simplified-lowering-verifier.cc +++ b/src/compiler/simplified-lowering-verifier.cc @@ -283,8 +283,9 @@ void SimplifiedLoweringVerifier::VisitNode(Node* node, CheckAndSet(node, input_type, InputTruncation(node, 0)); break; } - case IrOpcode::kCheckBigInt64: { + case IrOpcode::kCheckedBigIntToBigInt64: { Type input_type = InputType(node, 0); + CHECK(input_type.Is(Type::BigInt())); input_type = Type::Intersect(input_type, Type::SignedBigInt64(), graph_zone()); CheckAndSet(node, input_type, InputTruncation(node, 0)); diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc index 413ec94ca7..76dea889a3 100644 --- a/src/compiler/simplified-lowering.cc +++ b/src/compiler/simplified-lowering.cc @@ -2963,24 +2963,6 @@ class RepresentationSelector { } return; } - case IrOpcode::kCheckBigInt: { - if (InputIs(node, Type::BigInt())) { - VisitNoop(node, truncation); - } else { - VisitUnop(node, UseInfo::AnyTagged(), - MachineRepresentation::kTaggedPointer); - } - return; - } - case IrOpcode::kCheckBigInt64: { - if (InputIs(node, Type::BigInt())) { - VisitNoop(node, truncation); - } else { - VisitUnop(node, UseInfo::AnyTagged(), - MachineRepresentation::kTaggedPointer); - } - return; - } case IrOpcode::kSpeculativeBigIntAsIntN: case IrOpcode::kSpeculativeBigIntAsUintN: { const bool is_asuintn = diff --git a/src/compiler/simplified-operator.cc b/src/compiler/simplified-operator.cc index 1eeed36af9..413636bcce 100644 --- a/src/compiler/simplified-operator.cc +++ b/src/compiler/simplified-operator.cc @@ -847,7 +847,7 @@ bool operator==(CheckMinusZeroParameters const& lhs, V(CheckSmi, 1, 1) \ V(CheckString, 1, 1) \ V(CheckBigInt, 1, 1) \ - V(CheckBigInt64, 1, 1) \ + V(CheckedBigIntToBigInt64, 1, 1) \ V(CheckedInt32ToTaggedSigned, 1, 1) \ V(CheckedInt64ToInt32, 1, 1) \ V(CheckedInt64ToTaggedSigned, 1, 1) \ diff --git a/src/compiler/simplified-operator.h b/src/compiler/simplified-operator.h index 7c608c73c9..361d569609 100644 --- a/src/compiler/simplified-operator.h +++ b/src/compiler/simplified-operator.h @@ -935,7 +935,7 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final const Operator* CheckedTaggedToTaggedPointer(const FeedbackSource& feedback); const Operator* CheckedTaggedToTaggedSigned(const FeedbackSource& feedback); const Operator* CheckBigInt(const FeedbackSource& feedback); - const Operator* CheckBigInt64(const FeedbackSource& feedback); + const Operator* CheckedBigIntToBigInt64(const FeedbackSource& feedback); const Operator* CheckedTruncateTaggedToWord32(CheckTaggedInputMode, const FeedbackSource& feedback); const Operator* CheckedUint32Div(); diff --git a/src/compiler/verifier.cc b/src/compiler/verifier.cc index 14f3c1bcab..b6f340d24c 100644 --- a/src/compiler/verifier.cc +++ b/src/compiler/verifier.cc @@ -1654,8 +1654,8 @@ void Verifier::Visitor::Check(Node* node, const AllNodes& all) { CheckValueInputIs(node, 0, Type::Any()); CheckTypeIs(node, Type::BigInt()); break; - case IrOpcode::kCheckBigInt64: - CheckValueInputIs(node, 0, Type::Any()); + case IrOpcode::kCheckedBigIntToBigInt64: + CheckValueInputIs(node, 0, Type::BigInt()); CheckTypeIs(node, Type::SignedBigInt64()); break; case IrOpcode::kFastApiCall: diff --git a/src/deoptimizer/deoptimize-reason.h b/src/deoptimizer/deoptimize-reason.h index 284f9ef752..1eadb8700f 100644 --- a/src/deoptimizer/deoptimize-reason.h +++ b/src/deoptimizer/deoptimize-reason.h @@ -40,6 +40,7 @@ namespace internal { V(NaN, "NaN") \ V(NoCache, "no cache") \ V(NotABigInt, "not a BigInt") \ + V(NotABigInt64, "not a BigInt64") \ V(NotAHeapNumber, "not a heap number") \ V(NotAJavaScriptObject, "not a JavaScript object") \ V(NotAJavaScriptObjectOrNullOrUndefined, \ diff --git a/test/mjsunit/compiler/bigint64-add-no-deopt-loop.js b/test/mjsunit/compiler/bigint64-add-no-deopt-loop.js index b35d67d6b9..eedad495cd 100644 --- a/test/mjsunit/compiler/bigint64-add-no-deopt-loop.js +++ b/test/mjsunit/compiler/bigint64-add-no-deopt-loop.js @@ -4,7 +4,7 @@ // Flags: --allow-natives-syntax --turbofan --no-always-turbofan -(function OptimizeAndTest() { +(function OptimizeAndTestNegativeLimit() { function f(x, y) { return x + y; } @@ -12,24 +12,54 @@ assertEquals(1n, f(0n, 1n)); assertEquals(5n, f(2n, 3n)); %OptimizeFunctionOnNextCall(f); - assertEquals(9n, f(4n, 5n)); + assertEquals(-(2n ** 63n), f(-(2n ** 63n), 0n)); assertOptimized(f); // Re-prepare the function before the first deopt to ensure type feedback is // not cleared by an umtimely gc. %PrepareFunctionForOptimization(f); assertOptimized(f); - // CheckBigInt64 should trigger deopt. - assertEquals(-(2n ** 63n), f(-(2n ** 63n), 0n)); + // CheckBigInt64 should trigger deopt on INT_MIN - 1. + assertEquals(-(2n ** 63n) - 1n, f(-(2n ** 63n) - 1n, 0n)); if (%Is64Bit()) { assertUnoptimized(f); assertEquals(1n, f(0n, 1n)); assertEquals(5n, f(2n, 3n)); %OptimizeFunctionOnNextCall(f); - assertEquals(9n, f(4n, 5n)); + assertEquals(-(2n ** 63n), f(-(2n ** 63n), 0n)); assertOptimized(f); // Ensure there is no deopt loop. - assertEquals(-(2n ** 63n), f(-(2n ** 63n), 0n)); + assertEquals(-(2n ** 63n) - 1n, f(-(2n ** 63n) - 1n, 0n)); + assertOptimized(f); + } +})(); + +(function OptimizeAndTestPositiveLimit() { + function f(x, y) { + return x + y; + } + %PrepareFunctionForOptimization(f); + assertEquals(1n, f(0n, 1n)); + assertEquals(5n, f(2n, 3n)); + %OptimizeFunctionOnNextCall(f); + assertEquals(2n ** 63n - 1n, f(2n ** 63n - 1n, 0n)); + assertOptimized(f); + // Re-prepare the function before the first deopt to ensure type feedback is + // not cleared by an umtimely gc. + %PrepareFunctionForOptimization(f); + assertOptimized(f); + // CheckBigInt64 should trigger deopt on INT_MAX + 1. + assertEquals(2n ** 63n, f(2n ** 63n, 0n)); + if (%Is64Bit()) { + assertUnoptimized(f); + + assertEquals(1n, f(0n, 1n)); + assertEquals(5n, f(2n, 3n)); + %OptimizeFunctionOnNextCall(f); + assertEquals(2n ** 63n - 1n, f(2n ** 63n - 1n, 0n)); + assertOptimized(f); + // Ensure there is no deopt loop. + assertEquals(2n ** 63n, f(2n ** 63n, 0n)); assertOptimized(f); } })(); diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index 15a692b7b5..17d1426d4f 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -1339,7 +1339,6 @@ # which in turn can lead to different deopt behavior. 'compiler/number-abs': [SKIP], 'compiler/number-toboolean': [SKIP], - 'compiler/bigint64-div-no-deopt-loop': [SKIP], # Type assertions block the propagation of word64 truncation useinfo, # leading to differences in representation selection. 'compiler/bigint-multiply-truncate': [SKIP],