From 90fe7dc9ce484a4f2bae35c7aef7af0c06a7b799 Mon Sep 17 00:00:00 2001 From: Qifan Pan Date: Wed, 30 Nov 2022 12:38:48 +0100 Subject: [PATCH] [turbofan] Fix BigInt shift operations This CL fixed missing instance type checks for constant shift amounts and corrected the use info for the lhs. Bug: chromium:1393865, v8:9407 Change-Id: Id6e65f4e26a0436960b12196f29663429876398b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4061075 Commit-Queue: Qifan Pan Reviewed-by: Nico Hartmann Cr-Commit-Position: refs/heads/main@{#84596} --- src/compiler/representation-change.cc | 4 +- src/compiler/simplified-lowering.cc | 152 +++++++++--------- .../compiler/test-representation-change.cc | 7 + test/mjsunit/regress/regress-1393865.js | 28 ++++ 4 files changed, 114 insertions(+), 77 deletions(-) create mode 100644 test/mjsunit/regress/regress-1393865.js diff --git a/src/compiler/representation-change.cc b/src/compiler/representation-change.cc index 37c4cc9282..a2ae3c4420 100644 --- a/src/compiler/representation-change.cc +++ b/src/compiler/representation-change.cc @@ -1245,7 +1245,9 @@ Node* RepresentationChanger::GetWord64RepresentationFor( ((use_info.truncation().IsUsedAsWord64() && (use_info.type_check() == TypeCheckKind::kBigInt || output_type.Is(Type::BigInt()))) || - use_info.type_check() == TypeCheckKind::kBigInt64)) { + (use_info.type_check() == TypeCheckKind::kBigInt64 || + output_type.Is(Type::SignedBigInt64()) || + output_type.Is(Type::UnsignedBigInt64())))) { node = GetTaggedPointerRepresentationFor(node, output_rep, output_type, use_node, use_info); op = simplified()->TruncateBigIntToWord64(); diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc index 641db73a05..6d9f6de5b5 100644 --- a/src/compiler/simplified-lowering.cc +++ b/src/compiler/simplified-lowering.cc @@ -3354,88 +3354,88 @@ class RepresentationSelector { Type shift_amount_type = GetUpperBound(node->InputAt(1)); if (shift_amount_type.IsHeapConstant()) { - BigIntRef bigint = - shift_amount_type.AsHeapConstant()->Ref().AsBigInt(); - bool lossless = false; - int64_t shift_amount = bigint.AsInt64(&lossless); + HeapObjectRef ref = shift_amount_type.AsHeapConstant()->Ref(); + if (ref.IsBigInt()) { + BigIntRef bigint = ref.AsBigInt(); + bool lossless = false; + int64_t shift_amount = bigint.AsInt64(&lossless); - // Canonicalize {shift_amount}. - bool is_shift_left = - node->opcode() == IrOpcode::kSpeculativeBigIntShiftLeft; - if (shift_amount < 0) { - is_shift_left = !is_shift_left; - shift_amount = -shift_amount; - } - DCHECK_GE(shift_amount, 0); + // Canonicalize {shift_amount}. + bool is_shift_left = + node->opcode() == IrOpcode::kSpeculativeBigIntShiftLeft; + if (shift_amount < 0) { + is_shift_left = !is_shift_left; + shift_amount = -shift_amount; + } + DCHECK_GE(shift_amount, 0); - // If the operation is a *real* left shift, propagate truncation. - // If it is a *real* right shift, the output representation is - // word64 only if we know the input type is BigInt64. - // Otherwise, fall through to using BigIntOperationHint. - if (is_shift_left) { - // The rhs is a leaf node and will be discarded regardless - // so it is safe to propagate truncation to rhs. - VisitBinop( - node, - UseInfo::CheckedBigIntTruncatingWord64(FeedbackSource{}), - MachineRepresentation::kWord64); - if (lower()) { - if (!lossless || shift_amount > 63) { - DeferReplacement(node, jsgraph_->Int64Constant(0)); - } else if (shift_amount == 0) { - DeferReplacement(node, node->InputAt(0)); - } else { - DCHECK_GE(shift_amount, 1); - DCHECK_LE(shift_amount, 63); - ReplaceWithPureNode( - node, - graph()->NewNode(lowering->machine()->Word64Shl(), - node->InputAt(0), - jsgraph_->Int64Constant(shift_amount))); + // If the operation is a *real* left shift, propagate truncation. + // If it is a *real* right shift, the output representation is + // word64 only if we know the input type is BigInt64. + // Otherwise, fall through to using BigIntOperationHint. + if (is_shift_left) { + VisitBinop( + node, + UseInfo::CheckedBigIntTruncatingWord64(FeedbackSource{}), + UseInfo::Any(), MachineRepresentation::kWord64); + if (lower()) { + if (!lossless || shift_amount > 63) { + DeferReplacement(node, jsgraph_->Int64Constant(0)); + } else if (shift_amount == 0) { + DeferReplacement(node, node->InputAt(0)); + } else { + DCHECK_GE(shift_amount, 1); + DCHECK_LE(shift_amount, 63); + ReplaceWithPureNode( + node, + graph()->NewNode( + lowering->machine()->Word64Shl(), node->InputAt(0), + jsgraph_->Int64Constant(shift_amount))); + } } - } - return; - } else if (input_type.Is(Type::SignedBigInt64())) { - VisitBinop(node, UseInfo::Any(), - MachineRepresentation::kWord64); - if (lower()) { - if (!lossless || shift_amount > 63) { - ReplaceWithPureNode( - node, graph()->NewNode(lowering->machine()->Word64Sar(), - node->InputAt(0), - jsgraph_->Int64Constant(63))); - } else if (shift_amount == 0) { - DeferReplacement(node, node->InputAt(0)); - } else { - DCHECK_GE(shift_amount, 1); - DCHECK_LE(shift_amount, 63); - ReplaceWithPureNode( - node, - graph()->NewNode(lowering->machine()->Word64Sar(), - node->InputAt(0), - jsgraph_->Int64Constant(shift_amount))); + return; + } else if (input_type.Is(Type::SignedBigInt64())) { + VisitBinop(node, UseInfo::Word64(), UseInfo::Any(), + MachineRepresentation::kWord64); + if (lower()) { + if (!lossless || shift_amount > 63) { + ReplaceWithPureNode( + node, graph()->NewNode(lowering->machine()->Word64Sar(), + node->InputAt(0), + jsgraph_->Int64Constant(63))); + } else if (shift_amount == 0) { + DeferReplacement(node, node->InputAt(0)); + } else { + DCHECK_GE(shift_amount, 1); + DCHECK_LE(shift_amount, 63); + ReplaceWithPureNode( + node, + graph()->NewNode( + lowering->machine()->Word64Sar(), node->InputAt(0), + jsgraph_->Int64Constant(shift_amount))); + } } - } - return; - } else if (input_type.Is(Type::UnsignedBigInt64())) { - VisitBinop(node, UseInfo::Any(), - MachineRepresentation::kWord64); - if (lower()) { - if (!lossless || shift_amount > 63) { - DeferReplacement(node, jsgraph_->Int64Constant(0)); - } else if (shift_amount == 0) { - DeferReplacement(node, node->InputAt(0)); - } else { - DCHECK_GE(shift_amount, 1); - DCHECK_LE(shift_amount, 63); - ReplaceWithPureNode( - node, - graph()->NewNode(lowering->machine()->Word64Shr(), - node->InputAt(0), - jsgraph_->Int64Constant(shift_amount))); + return; + } else if (input_type.Is(Type::UnsignedBigInt64())) { + VisitBinop(node, UseInfo::Word64(), UseInfo::Any(), + MachineRepresentation::kWord64); + if (lower()) { + if (!lossless || shift_amount > 63) { + DeferReplacement(node, jsgraph_->Int64Constant(0)); + } else if (shift_amount == 0) { + DeferReplacement(node, node->InputAt(0)); + } else { + DCHECK_GE(shift_amount, 1); + DCHECK_LE(shift_amount, 63); + ReplaceWithPureNode( + node, + graph()->NewNode( + lowering->machine()->Word64Shr(), node->InputAt(0), + jsgraph_->Int64Constant(shift_amount))); + } } + return; } - return; } } } diff --git a/test/cctest/compiler/test-representation-change.cc b/test/cctest/compiler/test-representation-change.cc index b1e1d51d2a..a9fee37e14 100644 --- a/test/cctest/compiler/test-representation-change.cc +++ b/test/cctest/compiler/test-representation-change.cc @@ -530,6 +530,13 @@ TEST(Word64) { IrOpcode::kChangeInt64ToFloat64, IrOpcode::kChangeFloat64ToTaggedPointer, MachineRepresentation::kWord64, TypeCache::Get()->kSafeInteger, MachineRepresentation::kTaggedPointer); + + CheckChange(IrOpcode::kTruncateBigIntToWord64, + MachineRepresentation::kTaggedPointer, Type::SignedBigInt64(), + MachineRepresentation::kWord64); + CheckChange(IrOpcode::kTruncateBigIntToWord64, + MachineRepresentation::kTaggedPointer, Type::UnsignedBigInt64(), + MachineRepresentation::kWord64); } TEST(SingleChanges) { diff --git a/test/mjsunit/regress/regress-1393865.js b/test/mjsunit/regress/regress-1393865.js new file mode 100644 index 0000000000..b4e19f02a3 --- /dev/null +++ b/test/mjsunit/regress/regress-1393865.js @@ -0,0 +1,28 @@ +// Copyright 2022 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 + +function aux(a, b) { + if (a) { + a >> b; + } +} + +function opt() { + let p = Promise; + ++p; + // {p} can be anything that evaluates to false but is not inlined. + return aux(p, "number"); +} + +%PrepareFunctionForOptimization(aux); +aux(1n, 1n); +%OptimizeFunctionOnNextCall(aux); +aux(1n, 1n); + +%PrepareFunctionForOptimization(opt); +opt(); +%OptimizeFunctionOnNextCall(opt); +opt();