From a1ab842dbe39904748c63177caeb82875bd80be9 Mon Sep 17 00:00:00 2001 From: Nico Hartmann Date: Fri, 27 Jan 2023 13:48:12 +0100 Subject: [PATCH] [turboshaft] Port operations from ECL to MachineLoweringReducer (2) This CL ports operations from Turbofan's EffectControlLinearizer to Turboshaft's MachineLoweringReducer: - CheckedBigIntToBigInt64 - ChangeUint64ToBigInt Bug: v8:12783 Change-Id: I9386864305397642b840d2e89a6066a3263ce25d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4198146 Reviewed-by: Darius Mercadier Commit-Queue: Nico Hartmann Cr-Commit-Position: refs/heads/main@{#85519} --- src/compiler/common-operator.cc | 6 ++ src/compiler/effect-control-linearizer.cc | 14 ++- src/compiler/turboshaft/graph-builder.cc | 7 ++ .../turboshaft/machine-lowering-reducer.h | 89 ++++++++++++++++++- src/compiler/turboshaft/operations.cc | 4 + src/compiler/turboshaft/operations.h | 9 +- 6 files changed, 119 insertions(+), 10 deletions(-) diff --git a/src/compiler/common-operator.cc b/src/compiler/common-operator.cc index d5f12e572a..c12604eba8 100644 --- a/src/compiler/common-operator.cc +++ b/src/compiler/common-operator.cc @@ -1650,14 +1650,20 @@ CommonOperatorBuilder::CreateJSToWasmFrameStateFunctionInfo( const Operator* CommonOperatorBuilder::Chained(const Operator* op) { // Use Chained only for operators that are not on the effect chain already. DCHECK_EQ(op->EffectInputCount(), 0); + DCHECK_EQ(op->ControlInputCount(), 0); const char* mnemonic; switch (op->opcode()) { case IrOpcode::kChangeInt64ToBigInt: mnemonic = "Chained[ChangeInt64ToBigInt]"; break; + case IrOpcode::kChangeUint64ToBigInt: + mnemonic = "Chained[ChangeUint64ToBigInt]"; + break; default: UNREACHABLE(); } + // TODO(nicohartmann@): Need to store operator properties once we have to + // support Operator1 operators. Operator::Properties properties = op->properties(); return zone()->New(op->opcode(), properties, mnemonic, op->ValueInputCount(), 1, 1, diff --git a/src/compiler/effect-control-linearizer.cc b/src/compiler/effect-control-linearizer.cc index 07bfe5eec2..87a9f3b2ca 100644 --- a/src/compiler/effect-control-linearizer.cc +++ b/src/compiler/effect-control-linearizer.cc @@ -1025,6 +1025,7 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node, result = LowerCheckedUint64ToInt64(node, frame_state); break; case IrOpcode::kCheckedBigIntToBigInt64: + if (v8_flags.turboshaft) return false; result = LowerCheckedBigIntToBigInt64(node, frame_state); break; case IrOpcode::kCheckInternalizedString: @@ -1144,7 +1145,16 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node, } break; case IrOpcode::kChangeUint64ToBigInt: - result = LowerChangeUint64ToBigInt(node); + if (v8_flags.turboshaft) { + DCHECK(machine()->Is64()); + // ChangeUint64ToBigInt is allocting when lowered, so we must fix its + // position in the effect chain such that it is non-floating after ECL + // and cannot mess up when rescheduling (e.g. in Turboshaft's graph + // builder). + result = gasm()->Chained(node->op(), node->InputAt(0)); + } else { + result = LowerChangeUint64ToBigInt(node); + } break; case IrOpcode::kTruncateBigIntToWord64: result = LowerTruncateBigIntToWord64(node); @@ -3002,6 +3012,7 @@ Node* EffectControlLinearizer::LowerCheckBigInt(Node* node, Node* frame_state) { Node* EffectControlLinearizer::LowerCheckedBigIntToBigInt64(Node* node, Node* frame_state) { + DCHECK(!v8_flags.turboshaft); DCHECK(machine()->Is64()); auto done = __ MakeLabel(); @@ -3181,6 +3192,7 @@ Node* EffectControlLinearizer::LowerChangeInt64ToBigInt(Node* node) { } Node* EffectControlLinearizer::LowerChangeUint64ToBigInt(Node* node) { + DCHECK(!v8_flags.turboshaft); DCHECK(machine()->Is64()); auto done = __ MakeLabel(MachineRepresentation::kTagged); diff --git a/src/compiler/turboshaft/graph-builder.cc b/src/compiler/turboshaft/graph-builder.cc index 5a75bff25d..efcb0e09b4 100644 --- a/src/compiler/turboshaft/graph-builder.cc +++ b/src/compiler/turboshaft/graph-builder.cc @@ -673,9 +673,16 @@ OpIndex GraphBuilder::Process( return assembler.Check(Map(node->InputAt(0)), dominating_frame_state, CheckOp::Kind::kCheckBigInt, CheckParametersOf(op).feedback()); + case IrOpcode::kCheckedBigIntToBigInt64: + return assembler.Check(Map(node->InputAt(0)), dominating_frame_state, + CheckOp::Kind::kBigIntIsBigInt64, + CheckParametersOf(op).feedback()); case IrOpcode::kChangeInt64ToBigInt: return assembler.ConvertToObject( Map(node->InputAt(0)), ConvertToObjectOp::Kind::kInt64ToBigInt64); + case IrOpcode::kChangeUint64ToBigInt: + return assembler.ConvertToObject( + Map(node->InputAt(0)), ConvertToObjectOp::Kind::kUint64ToBigInt64); case IrOpcode::kSelect: { OpIndex cond = Map(node->InputAt(0)); diff --git a/src/compiler/turboshaft/machine-lowering-reducer.h b/src/compiler/turboshaft/machine-lowering-reducer.h index 4e4b32074e..a61ae954be 100644 --- a/src/compiler/turboshaft/machine-lowering-reducer.h +++ b/src/compiler/turboshaft/machine-lowering-reducer.h @@ -9,6 +9,7 @@ #include "src/compiler/access-builder.h" #include "src/compiler/globals.h" #include "src/compiler/simplified-operator.h" +#include "src/compiler/turboshaft/index.h" #include "src/compiler/turboshaft/optimization-phase.h" #include "src/compiler/turboshaft/representations.h" #include "src/objects/bigint.h" @@ -50,6 +51,55 @@ class MachineLoweringReducer : public Next { DeoptimizeReason::kWrongInstanceType, feedback); return input; } + case CheckOp::Kind::kBigIntIsBigInt64: { + DCHECK(Asm().Is64()); + + Block* non_zero_block = Asm().NewBlock(); + Block* done_block = Asm().NewBlock(); + Block* maybe_out_of_range_block = Asm().NewBlock(); + + OpIndex bitfield = LoadField(input, AccessBuilder::ForBigIntBitfield()); + Asm().Branch(Asm().Word32Equal(bitfield, Asm().Word32Constant(0)), + done_block, non_zero_block); + + if (Asm().Bind(non_zero_block)) { + // Length must be 1. + OpIndex length_field = Asm().Word32BitwiseAnd( + bitfield, Asm().Word32Constant(BigInt::LengthBits::kMask)); + Asm().DeoptimizeIfNot( + Asm().Word32Equal(length_field, + Asm().Word32Constant( + uint32_t{1} << BigInt::LengthBits::kShift)), + frame_state, DeoptimizeReason::kNotABigInt64, feedback); + + // Check if it fits in 64 bit signed int. + OpIndex lsd = LoadField( + input, AccessBuilder::ForBigIntLeastSignificantDigit64()); + OpIndex magnitude_check = Asm().Uint64LessThanOrEqual( + lsd, Asm().Word64Constant(std::numeric_limits::max())); + Asm().Branch(magnitude_check, done_block, maybe_out_of_range_block); + + if (Asm().Bind(maybe_out_of_range_block)) { + // The BigInt probably doesn't fit into signed int64. The only + // exception is int64_t::min. We check for this. + OpIndex sign = Asm().Word32BitwiseAnd( + bitfield, Asm().Word32Constant(BigInt::SignBits::kMask)); + OpIndex sign_check = Asm().Word32Equal( + sign, Asm().Word32Constant(BigInt::SignBits::kMask)); + Asm().DeoptimizeIfNot(sign_check, frame_state, + DeoptimizeReason::kNotABigInt64, feedback); + + OpIndex min_check = Asm().Word64Equal( + lsd, Asm().Word64Constant(std::numeric_limits::min())); + Asm().DeoptimizeIfNot(min_check, frame_state, + DeoptimizeReason::kNotABigInt64, feedback); + Asm().Goto(done_block); + } + } + + Asm().BindReachable(done_block); + return input; + } } UNREACHABLE(); @@ -63,7 +113,7 @@ class MachineLoweringReducer : public Next { // BigInts with value 0 must be of size 0 (canonical form). Block* non_zero_block = Asm().NewBlock(); Block* zero_block = Asm().NewBlock(); - Block* merge_block = Asm().NewBlock(); + Block* done_block = Asm().NewBlock(); Variable result = Asm().NewFreshVariable(RegisterRepresentation::Tagged()); @@ -74,7 +124,7 @@ class MachineLoweringReducer : public Next { if (Asm().Bind(zero_block)) { Asm().Set(result, BuildAllocateBigInt(OpIndex::Invalid(), OpIndex::Invalid())); - Asm().Goto(merge_block); + Asm().Goto(done_block); } if (Asm().Bind(non_zero_block)) { @@ -92,10 +142,41 @@ class MachineLoweringReducer : public Next { OpIndex absolute_value = Asm().Word64Sub( Asm().Word64BitwiseXor(input, sign_mask), sign_mask); Asm().Set(result, BuildAllocateBigInt(bitfield, absolute_value)); - Asm().Goto(merge_block); + Asm().Goto(done_block); } - Asm().BindReachable(merge_block); + Asm().BindReachable(done_block); + return Asm().Get(result); + } + case ConvertToObjectOp::Kind::kUint64ToBigInt64: { + DCHECK(Asm().Is64()); + + // BigInts with value 0 must be of size 0 (canonical form). + Block* non_zero_block = Asm().NewBlock(); + Block* zero_block = Asm().NewBlock(); + Block* done_block = Asm().NewBlock(); + + Variable result = + Asm().NewFreshVariable(RegisterRepresentation::Tagged()); + + Asm().Branch( + Asm().Word64Equal(input, Asm().Word64Constant(uint64_t{0})), + zero_block, non_zero_block); + + if (Asm().Bind(zero_block)) { + Asm().Set(result, BuildAllocateBigInt(OpIndex::Invalid(), + OpIndex::Invalid())); + Asm().Goto(done_block); + } + + if (Asm().Bind(non_zero_block)) { + const auto bitfield = BigInt::LengthBits::encode(1); + Asm().Set(result, + BuildAllocateBigInt(Asm().Word32Constant(bitfield), input)); + Asm().Goto(done_block); + } + + Asm().BindReachable(done_block); return Asm().Get(result); } } diff --git a/src/compiler/turboshaft/operations.cc b/src/compiler/turboshaft/operations.cc index ff2c4106cb..2565b5c538 100644 --- a/src/compiler/turboshaft/operations.cc +++ b/src/compiler/turboshaft/operations.cc @@ -599,6 +599,8 @@ std::ostream& operator<<(std::ostream& os, CheckOp::Kind kind) { switch (kind) { case CheckOp::Kind::kCheckBigInt: return os << "CheckBigInt"; + case CheckOp::Kind::kBigIntIsBigInt64: + return os << "BigIntIsBigInt64"; } } @@ -606,6 +608,8 @@ std::ostream& operator<<(std::ostream& os, ConvertToObjectOp::Kind kind) { switch (kind) { case ConvertToObjectOp::Kind::kInt64ToBigInt64: return os << "Int64ToBigInt64"; + case ConvertToObjectOp::Kind::kUint64ToBigInt64: + return os << "Uint64ToBigInt64"; } } diff --git a/src/compiler/turboshaft/operations.h b/src/compiler/turboshaft/operations.h index 59d5861d62..8401612fd8 100644 --- a/src/compiler/turboshaft/operations.h +++ b/src/compiler/turboshaft/operations.h @@ -2120,7 +2120,8 @@ struct CheckTurboshaftTypeOfOp struct CheckOp : FixedArityOperationT<2, CheckOp> { enum class Kind { - kCheckBigInt, + kCheckBigInt, // Checks if a tagged input is a BigInt object + kBigIntIsBigInt64, // Checks if a BigInt input is in BigInt64 range }; Kind kind; FeedbackSource feedback; @@ -2155,15 +2156,13 @@ struct IsSmiTaggedOp : FixedArityOperationT<1, IsSmiTaggedOp> { struct ConvertToObjectOp : FixedArityOperationT<1, ConvertToObjectOp> { enum class Kind { kInt64ToBigInt64, + kUint64ToBigInt64, }; Kind kind; static constexpr OpProperties properties = OpProperties::PureMayAllocate(); base::Vector outputs_rep() const { - switch (kind) { - case Kind::kInt64ToBigInt64: - return RepVector(); - } + return RepVector(); } OpIndex input() const { return Base::input(0); }