Commit 248fd5ff authored by Benedikt Meurer's avatar Benedikt Meurer Committed by Commit Bot

Revert "[turbofan] Do not consume SignedSmall feedback in TurboFan anymore."

This reverts commit 4fd92b25.

Reason for revert: Significant tankage on the no-mitigations bots (bad timing on the regular bots)

Original change's description:
> [turbofan] Do not consume SignedSmall feedback in TurboFan anymore.
> 
> This changes TurboFan to treat SignedSmall feedback similar to Signed32
> feedback for binary and compare operations, in order to simplify and
> unify the machinery.
> 
> This is an experiment. If this turns out to tank performance, we will
> need to revisit and ideally revert this change.
> 
> Bug: v8:7094
> Change-Id: I885769c2fe93d8413e59838fbe844650c848c3f1
> Reviewed-on: https://chromium-review.googlesource.com/c/1261442
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#56411}

TBR=jarin@chromium.org,bmeurer@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: v8:7094
Change-Id: I9fff3b40e6dc0ceb7611b55e1ca9940089470404
Reviewed-on: https://chromium-review.googlesource.com/c/1267175Reviewed-by: 's avatarBenedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56427}
parent c73c2dea
......@@ -2828,9 +2828,9 @@ void BytecodeGraphBuilder::VisitForInContinue() {
environment()->LookupRegister(bytecode_iterator().GetRegisterOperand(0));
Node* cache_length =
environment()->LookupRegister(bytecode_iterator().GetRegisterOperand(1));
Node* exit_cond = NewNode(
simplified()->SpeculativeNumberLessThan(NumberOperationHint::kSigned32),
index, cache_length);
Node* exit_cond = NewNode(simplified()->SpeculativeNumberLessThan(
NumberOperationHint::kSignedSmall),
index, cache_length);
environment()->BindAccumulator(exit_cond);
}
......@@ -2868,9 +2868,9 @@ void BytecodeGraphBuilder::VisitForInStep() {
PrepareEagerCheckpoint();
Node* index =
environment()->LookupRegister(bytecode_iterator().GetRegisterOperand(0));
index = NewNode(
simplified()->SpeculativeSafeIntegerAdd(NumberOperationHint::kSigned32),
index, jsgraph()->OneConstant());
index = NewNode(simplified()->SpeculativeSafeIntegerAdd(
NumberOperationHint::kSignedSmall),
index, jsgraph()->OneConstant());
environment()->BindAccumulator(index, Environment::kAttachFrameState);
}
......
......@@ -20,12 +20,14 @@ namespace {
bool BinaryOperationHintToNumberOperationHint(
BinaryOperationHint binop_hint, NumberOperationHint* number_hint) {
switch (binop_hint) {
case BinaryOperationHint::kSigned32:
case BinaryOperationHint::kSignedSmall:
*number_hint = NumberOperationHint::kSigned32;
*number_hint = NumberOperationHint::kSignedSmall;
return true;
case BinaryOperationHint::kSignedSmallInputs:
*number_hint = NumberOperationHint::kSigned32Inputs;
*number_hint = NumberOperationHint::kSignedSmallInputs;
return true;
case BinaryOperationHint::kSigned32:
*number_hint = NumberOperationHint::kSigned32;
return true;
case BinaryOperationHint::kNumber:
*number_hint = NumberOperationHint::kNumber;
......@@ -75,7 +77,7 @@ class JSSpeculativeBinopBuilder final {
bool GetCompareNumberOperationHint(NumberOperationHint* hint) {
switch (GetCompareOperationHint()) {
case CompareOperationHint::kSignedSmall:
*hint = NumberOperationHint::kSigned32;
*hint = NumberOperationHint::kSignedSmall;
return true;
case CompareOperationHint::kNumber:
*hint = NumberOperationHint::kNumber;
......@@ -98,13 +100,15 @@ class JSSpeculativeBinopBuilder final {
const Operator* SpeculativeNumberOp(NumberOperationHint hint) {
switch (op_->opcode()) {
case IrOpcode::kJSAdd:
if (hint == NumberOperationHint::kSigned32) {
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
return simplified()->SpeculativeSafeIntegerAdd(hint);
} else {
return simplified()->SpeculativeNumberAdd(hint);
}
case IrOpcode::kJSSubtract:
if (hint == NumberOperationHint::kSigned32) {
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
return simplified()->SpeculativeSafeIntegerSubtract(hint);
} else {
return simplified()->SpeculativeNumberSubtract(hint);
......
......@@ -37,7 +37,7 @@ class JSBinopReduction final {
DCHECK_EQ(1, node_->op()->EffectOutputCount());
switch (CompareOperationHintOf(node_->op())) {
case CompareOperationHint::kSignedSmall:
*hint = NumberOperationHint::kSigned32;
*hint = NumberOperationHint::kSignedSmall;
return true;
case CompareOperationHint::kNumber:
*hint = NumberOperationHint::kNumber;
......
......@@ -94,8 +94,10 @@ UseInfo CheckedUseInfoAsWord32FromHint(
NumberOperationHint hint, const VectorSlotPair& feedback = VectorSlotPair(),
IdentifyZeros identify_zeros = kDistinguishZeros) {
switch (hint) {
case NumberOperationHint::kSignedSmall:
case NumberOperationHint::kSignedSmallInputs:
return UseInfo::CheckedSignedSmallAsWord32(identify_zeros, feedback);
case NumberOperationHint::kSigned32:
case NumberOperationHint::kSigned32Inputs:
return UseInfo::CheckedSigned32AsWord32(identify_zeros, feedback);
case NumberOperationHint::kNumber:
return UseInfo::CheckedNumberAsWord32(feedback);
......@@ -109,8 +111,9 @@ UseInfo CheckedUseInfoAsFloat64FromHint(
NumberOperationHint hint, const VectorSlotPair& feedback,
IdentifyZeros identify_zeros = kDistinguishZeros) {
switch (hint) {
case NumberOperationHint::kSignedSmall:
case NumberOperationHint::kSignedSmallInputs:
case NumberOperationHint::kSigned32:
case NumberOperationHint::kSigned32Inputs:
// Not used currently.
UNREACHABLE();
break;
......@@ -744,6 +747,11 @@ class RepresentationSelector {
GetUpperBound(node->InputAt(1)).Is(type);
}
bool IsNodeRepresentationTagged(Node* node) {
MachineRepresentation representation = GetInfo(node)->representation();
return IsAnyTagged(representation);
}
bool OneInputCannotBe(Node* node, Type type) {
DCHECK_EQ(2, node->op()->ValueInputCount());
return !GetUpperBound(node->InputAt(0)).Maybe(type) ||
......@@ -1311,7 +1319,9 @@ class RepresentationSelector {
// Try to use type feedback.
NumberOperationHint hint = NumberOperationHintOf(node->op());
DCHECK_EQ(NumberOperationHint::kSigned32, hint);
DCHECK(hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32);
Type left_feedback_type = TypeOf(node->InputAt(0));
Type right_feedback_type = TypeOf(node->InputAt(1));
......@@ -1405,7 +1415,8 @@ class RepresentationSelector {
// Handle the case when no uint32 checks on inputs are necessary
// (but an overflow check is needed on the output).
if (BothInputsAreUnsigned32(node)) {
if (hint == NumberOperationHint::kSigned32) {
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
VisitBinop(node, UseInfo::TruncatingWord32(),
MachineRepresentation::kWord32, Type::Unsigned32());
if (lower()) ChangeToUint32OverflowOp(node);
......@@ -1417,7 +1428,8 @@ class RepresentationSelector {
// (but an overflow check is needed on the output).
if (BothInputsAre(node, Type::Signed32())) {
// If both the inputs the feedback are int32, use the overflow op.
if (hint == NumberOperationHint::kSigned32) {
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
VisitBinop(node, UseInfo::TruncatingWord32(),
MachineRepresentation::kWord32, Type::Signed32());
if (lower()) ChangeToInt32OverflowOp(node);
......@@ -1425,7 +1437,8 @@ class RepresentationSelector {
}
}
if (hint == NumberOperationHint::kSigned32) {
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
// If the result is truncated, we only need to check the inputs.
// For the left hand side we just propagate the identify zeros
// mode of the {truncation}; and for modulus the sign of the
......@@ -1746,13 +1759,37 @@ class RepresentationSelector {
NumberOperationHint hint = NumberOperationHintOf(node->op());
switch (hint) {
case NumberOperationHint::kSigned32:
VisitBinop(node,
CheckedUseInfoAsWord32FromHint(hint, VectorSlotPair(),
kIdentifyZeros),
MachineRepresentation::kBit);
if (lower()) ChangeToPureOp(node, Int32Op(node));
case NumberOperationHint::kSignedSmall:
if (propagate()) {
VisitBinop(node,
CheckedUseInfoAsWord32FromHint(hint, VectorSlotPair(),
kIdentifyZeros),
MachineRepresentation::kBit);
} else if (retype()) {
SetOutput(node, MachineRepresentation::kBit, Type::Any());
} else {
DCHECK(lower());
Node* lhs = node->InputAt(0);
Node* rhs = node->InputAt(1);
if (IsNodeRepresentationTagged(lhs) &&
IsNodeRepresentationTagged(rhs)) {
VisitBinop(node,
UseInfo::CheckedSignedSmallAsTaggedSigned(
VectorSlotPair(), kIdentifyZeros),
MachineRepresentation::kBit);
ChangeToPureOp(
node, changer_->TaggedSignedOperatorFor(node->opcode()));
} else {
VisitBinop(node,
CheckedUseInfoAsWord32FromHint(
hint, VectorSlotPair(), kIdentifyZeros),
MachineRepresentation::kBit);
ChangeToPureOp(node, Int32Op(node));
}
}
return;
case NumberOperationHint::kSigned32Inputs:
case NumberOperationHint::kSignedSmallInputs:
// This doesn't make sense for compare operations.
UNREACHABLE();
case NumberOperationHint::kNumberOrOddball:
......@@ -1823,7 +1860,8 @@ class RepresentationSelector {
// (but an overflow check is needed on the output).
if (BothInputsAre(node, Type::Signed32())) {
// If both inputs and feedback are int32, use the overflow op.
if (hint == NumberOperationHint::kSigned32) {
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
VisitBinop(node, UseInfo::TruncatingWord32(),
MachineRepresentation::kWord32, Type::Signed32());
if (lower()) {
......@@ -1834,7 +1872,8 @@ class RepresentationSelector {
}
}
if (hint == NumberOperationHint::kSigned32) {
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
VisitBinop(node, CheckedUseInfoAsWord32FromHint(hint),
MachineRepresentation::kWord32, Type::Signed32());
if (lower()) {
......@@ -1900,7 +1939,8 @@ class RepresentationSelector {
// Handle the case when no uint32 checks on inputs are necessary
// (but an overflow check is needed on the output).
if (BothInputsAreUnsigned32(node)) {
if (hint == NumberOperationHint::kSigned32) {
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
VisitBinop(node, UseInfo::TruncatingWord32(),
MachineRepresentation::kWord32, Type::Unsigned32());
if (lower()) ChangeToUint32OverflowOp(node);
......@@ -1912,7 +1952,8 @@ class RepresentationSelector {
// (but an overflow check is needed on the output).
if (BothInputsAreSigned32(node)) {
// If both the inputs the feedback are int32, use the overflow op.
if (hint == NumberOperationHint::kSigned32) {
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
VisitBinop(node, UseInfo::TruncatingWord32(),
MachineRepresentation::kWord32, Type::Signed32());
if (lower()) ChangeToInt32OverflowOp(node);
......@@ -1921,14 +1962,15 @@ class RepresentationSelector {
}
if (hint == NumberOperationHint::kSigned32 ||
hint == NumberOperationHint::kSigned32Inputs) {
hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSignedSmallInputs) {
// If the result is truncated, we only need to check the inputs.
if (truncation.IsUsedAsWord32()) {
VisitBinop(node, CheckedUseInfoAsWord32FromHint(hint),
MachineRepresentation::kWord32);
if (lower()) DeferReplacement(node, lowering->Int32Div(node));
return;
} else if (hint != NumberOperationHint::kSigned32Inputs) {
} else if (hint != NumberOperationHint::kSignedSmallInputs) {
VisitBinop(node, CheckedUseInfoAsWord32FromHint(hint),
MachineRepresentation::kWord32, Type::Signed32());
if (lower()) ChangeToInt32OverflowOp(node);
......@@ -2090,7 +2132,8 @@ class RepresentationSelector {
NumberOperationHint hint = NumberOperationHintOf(node->op());
Type rhs_type = GetUpperBound(node->InputAt(1));
if (rhs_type.Is(type_cache_.kZeroish) &&
hint == NumberOperationHint::kSigned32 &&
(hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) &&
!truncation.IsUsedAsWord32()) {
// The SignedSmall or Signed32 feedback means that the results that we
// have seen so far were of type Unsigned31. We speculate that this
......@@ -2826,7 +2869,8 @@ class RepresentationSelector {
NumberOperationParametersOf(node->op());
switch (p.hint()) {
case NumberOperationHint::kSigned32:
case NumberOperationHint::kSigned32Inputs:
case NumberOperationHint::kSignedSmall:
case NumberOperationHint::kSignedSmallInputs:
VisitUnop(node,
CheckedUseInfoAsWord32FromHint(p.hint(), p.feedback()),
MachineRepresentation::kWord32, Type::Signed32());
......
......@@ -509,8 +509,10 @@ Handle<Map> FastMapParameterOf(const Operator* op) {
std::ostream& operator<<(std::ostream& os, NumberOperationHint hint) {
switch (hint) {
case NumberOperationHint::kSigned32Inputs:
return os << "Signed32Inputs";
case NumberOperationHint::kSignedSmall:
return os << "SignedSmall";
case NumberOperationHint::kSignedSmallInputs:
return os << "SignedSmallInputs";
case NumberOperationHint::kSigned32:
return os << "Signed32";
case NumberOperationHint::kNumber:
......@@ -1082,8 +1084,10 @@ struct SimplifiedOperatorGlobalCache final {
IrOpcode::k##Name, Operator::kFoldable | Operator::kNoThrow, \
#Name, 2, 1, 1, 1, 1, 0, kHint) {} \
}; \
Name##Operator<NumberOperationHint::kSigned32Inputs> \
k##Name##Signed32InputsOperator; \
Name##Operator<NumberOperationHint::kSignedSmall> \
k##Name##SignedSmallOperator; \
Name##Operator<NumberOperationHint::kSignedSmallInputs> \
k##Name##SignedSmallInputsOperator; \
Name##Operator<NumberOperationHint::kSigned32> k##Name##Signed32Operator; \
Name##Operator<NumberOperationHint::kNumber> k##Name##NumberOperator; \
Name##Operator<NumberOperationHint::kNumberOrOddball> \
......@@ -1101,6 +1105,8 @@ struct SimplifiedOperatorGlobalCache final {
1, 1, 1, 1, 1, 0,
NumberOperationParameters(kHint, VectorSlotPair())) {}
};
SpeculativeToNumberOperator<NumberOperationHint::kSignedSmall>
kSpeculativeToNumberSignedSmallOperator;
SpeculativeToNumberOperator<NumberOperationHint::kSigned32>
kSpeculativeToNumberSigned32Operator;
SpeculativeToNumberOperator<NumberOperationHint::kNumber>
......@@ -1327,10 +1333,12 @@ const Operator* SimplifiedOperatorBuilder::SpeculativeToNumber(
NumberOperationHint hint, const VectorSlotPair& feedback) {
if (!feedback.IsValid()) {
switch (hint) {
case NumberOperationHint::kSignedSmall:
return &cache_.kSpeculativeToNumberSignedSmallOperator;
case NumberOperationHint::kSignedSmallInputs:
break;
case NumberOperationHint::kSigned32:
return &cache_.kSpeculativeToNumberSigned32Operator;
case NumberOperationHint::kSigned32Inputs:
break;
case NumberOperationHint::kNumber:
return &cache_.kSpeculativeToNumberNumberOperator;
case NumberOperationHint::kNumberOrOddball:
......@@ -1530,8 +1538,10 @@ const Operator* SimplifiedOperatorBuilder::StringFromSingleCodePoint(
#define SPECULATIVE_NUMBER_BINOP(Name) \
const Operator* SimplifiedOperatorBuilder::Name(NumberOperationHint hint) { \
switch (hint) { \
case NumberOperationHint::kSigned32Inputs: \
return &cache_.k##Name##Signed32InputsOperator; \
case NumberOperationHint::kSignedSmall: \
return &cache_.k##Name##SignedSmallOperator; \
case NumberOperationHint::kSignedSmallInputs: \
return &cache_.k##Name##SignedSmallInputsOperator; \
case NumberOperationHint::kSigned32: \
return &cache_.k##Name##Signed32Operator; \
case NumberOperationHint::kNumber: \
......
......@@ -434,10 +434,11 @@ Type ValueTypeParameterOf(const Operator* op) V8_WARN_UNUSED_RESULT;
// A hint for speculative number operations.
enum class NumberOperationHint : uint8_t {
kSigned32, // Inputs were Signed32, output was Number.
kSigned32Inputs, // Inputs were Signed32, output was Number.
kNumber, // Inputs were Number, output was Number.
kNumberOrOddball, // Inputs were Number or Oddball, output was Number.
kSignedSmall, // Inputs were Smi, output was in Smi.
kSignedSmallInputs, // Inputs were Smi, output was Number.
kSigned32, // Inputs were Signed32, output was Number.
kNumber, // Inputs were Number, output was Number.
kNumberOrOddball, // Inputs were Number or Oddball, output was Number.
};
size_t hash_value(NumberOperationHint);
......
......@@ -585,7 +585,7 @@ TEST(LogAll) {
CHECK(logger.ContainsLine({"api,v8::Script::Run"}));
CHECK(logger.ContainsLine({"code-creation,LazyCompile,", "testAddFn"}));
if (i::FLAG_opt && !i::FLAG_always_opt) {
CHECK(logger.ContainsLine({"code-deopt,", "not a heap number"}));
CHECK(logger.ContainsLine({"code-deopt,", "not a Smi"}));
if (i::FLAG_enable_one_shot_optimization)
CHECK(logger.ContainsLine({"code-deopt,", "DeoptimizeNow"}));
CHECK(logger.ContainsLine({"timer-event-start", "V8.DeoptimizeCode"}));
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax --opt --no-always-opt --deopt-every-n-times=3
// Flags: --allow-natives-syntax --opt --no-always-opt --deopt-every-n-times=6
// Check that stress deopt count resets correctly
......@@ -14,34 +14,34 @@ function f(x) {
f(1);
%OptimizeFunctionOnNextCall(f);
// stress_deopt_count == 3
// stress_deopt_count == 6
f(1);
assertOptimized(f, undefined, undefined, false);
// stress_deopt_count == 2
// stress_deopt_count == 4
f(1);
assertOptimized(f, undefined, undefined, false);
// stress_deopt_count == 1
// stress_deopt_count == 2
f(1);
// deopt & counter reset
assertUnoptimized(f, undefined, undefined, false);
// stress_deopt_count == 3
// stress_deopt_count == 6
%OptimizeFunctionOnNextCall(f);
f(1);
assertOptimized(f, undefined, undefined, false);
// stress_deopt_count == 2
// stress_deopt_count == 4
f(1);
assertOptimized(f, undefined, undefined, false);
// stress_deopt_count == 1
// stress_deopt_count == 2
f(1);
// deopt & counter reset
......
......@@ -27,34 +27,41 @@
// Flags: --allow-natives-syntax --opt --noalways-opt
(function() {
function mul(a, b) {
return a * b;
}
mul(-1, 2);
mul(-1, 2);
%OptimizeFunctionOnNextCall(mul);
assertEquals(-2, mul(-1, 2));
assertOptimized(mul);
// Deopt on minus zero.
assertEquals(-0, mul(-1, 0));
assertUnoptimized(mul);
})();
(function() {
function mul2(a, b) {
return a * b;
}
mul2(-1, 2);
mul2(-1, 2);
%OptimizeFunctionOnNextCall(mul2);
assertEquals(-2, mul2(-1, 2));
assertOptimized(mul2);
// Deopt on 2^31.
assertEquals(1 << 31, mul2(-(1 << 31), -1));
function mul(a, b) {
return a * b;
}
mul(-1, 2);
mul(-1, 2);
%OptimizeFunctionOnNextCall(mul);
assertEquals(-2, mul(-1, 2));
assertOptimized(mul);
// Deopt on minus zero.
assertEquals(-0, mul(-1, 0));
assertUnoptimized(mul);
function mul2(a, b) {
return a * b;
}
mul2(-1, 2);
mul2(-1, 2);
%OptimizeFunctionOnNextCall(mul2);
// 2^30 is a smi boundary on arm and ia32.
var two_30 = 1 << 30;
// 2^31 is a smi boundary on x64.
var two_31 = 2 * two_30;
if (%IsValidSmi(two_31)) {
// Deopt on two_31 on x64.
assertEquals(two_31, mul2(-two_31, -1));
assertUnoptimized(mul2);
} else {
// Deopt on two_30 on ia32.
assertEquals(two_30, mul2(-two_30, -1));
assertUnoptimized(mul2);
})();
}
......@@ -72,8 +72,12 @@ const CheckTaggedInputMode kCheckTaggedInputModes[] = {
CheckTaggedInputMode::kNumber, CheckTaggedInputMode::kNumberOrOddball};
const NumberOperationHint kNumberOperationHints[] = {
NumberOperationHint::kSigned32, NumberOperationHint::kSigned32Inputs,
NumberOperationHint::kNumber, NumberOperationHint::kNumberOrOddball};
NumberOperationHint::kSignedSmall,
NumberOperationHint::kSignedSmallInputs,
NumberOperationHint::kSigned32,
NumberOperationHint::kNumber,
NumberOperationHint::kNumberOrOddball,
};
} // namespace
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment