AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[turbofan] Revert "Avoid overflow checks on SpeculativeNumberAdd/Subtract/Multiply."

Browse Source 
The optimization is not correct for unsigned output types, and we the
overall complexity seems too high. We need to find a better way to
take into account the input/output type restrictions.

Also added a regression test for the unsigned output bug.

BUG=v8:5267,v8:5270,v8:5357
TBR=jarin@chromium.org

Review-Url: https://codereview.chromium.org/2320013002
Cr-Commit-Position: refs/heads/master@{#39262}
This commit is contained in:
bmeurer 2016-09-07 21:20:09 -07:00 committed by Commit bot
parent b4f8a7c900
commit 91ed540ee6
4 changed files with 86 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/compiler/representation-change.cc
View File

@ -751,10 +751,8 @@ const Operator* RepresentationChanger::Int32OverflowOperatorFor(
const Operator* RepresentationChanger::Uint32OperatorFor(
IrOpcode::Value opcode) {
switch (opcode) {
case IrOpcode::kSpeculativeNumberAdd:
case IrOpcode::kNumberAdd:
return machine()->Int32Add();
case IrOpcode::kSpeculativeNumberSubtract:
case IrOpcode::kNumberSubtract:
return machine()->Int32Sub();
case IrOpcode::kSpeculativeNumberMultiply:

4
src/compiler/representation-change.h
View File

@ -35,10 +35,6 @@ class Truncation final {
bool IsUsedAsFloat64() const {
return LessGeneral(kind_, TruncationKind::kFloat64);
}
bool IdentifiesMinusZeroAndZero() {
return LessGeneral(kind_, TruncationKind::kWord32) ||
LessGeneral(kind_, TruncationKind::kBool);
}
bool IdentifiesNaNAndZero() {
return LessGeneral(kind_, TruncationKind::kWord32) ||
LessGeneral(kind_, TruncationKind::kBool);

147
src/compiler/simplified-lowering.cc
View File

@ -241,12 +241,6 @@ class RepresentationSelector {
bool weakened() const { return weakened_; }
void set_restriction_type(Type* type) { restriction_type_ = type; }
Type* restriction_type() const { return restriction_type_; }
void set_unrestricted_feedback_type(Type* type) {
unrestricted_feedback_type_ = type;
}
Type* unrestricted_feedback_type() const {
return unrestricted_feedback_type_;
}
private:
enum State : uint8_t { kUnvisited, kPushed, kVisited, kQueued };
@ -257,7 +251,6 @@ class RepresentationSelector {
Type* restriction_type_ = Type::Any();
Type* feedback_type_ = nullptr;
Type* unrestricted_feedback_type_ = nullptr;
bool weakened_ = false;
};
@ -371,11 +364,6 @@ class RepresentationSelector {
return type == nullptr ? Type::None() : type;
}
Type* UnrestrictedFeedbackTypeOf(Node* node) {
Type* type = GetInfo(node)->unrestricted_feedback_type();
return type == nullptr ? NodeProperties::GetType(node) : type;
}
Type* TypePhi(Node* node) {
int arity = node->op()->ValueInputCount();
Type* type = FeedbackTypeOf(node->InputAt(0));
@ -418,11 +406,13 @@ class RepresentationSelector {
SIMPLIFIED_NUMBER_BINOP_LIST(DECLARE_CASE)
#undef DECLARE_CASE
#define DECLARE_CASE(Name) \
case IrOpcode::k##Name: { \
new_type = op_typer_.Name(FeedbackTypeOf(node->InputAt(0)), \
FeedbackTypeOf(node->InputAt(1))); \
break; \
#define DECLARE_CASE(Name) \
case IrOpcode::k##Name: { \
new_type = \
Type::Intersect(op_typer_.Name(FeedbackTypeOf(node->InputAt(0)), \
FeedbackTypeOf(node->InputAt(1))), \
info->restriction_type(), graph_zone()); \
break; \
}
SIMPLIFIED_SPECULATIVE_NUMBER_BINOP_LIST(DECLARE_CASE)
#undef DECLARE_CASE
@ -461,17 +451,11 @@ class RepresentationSelector {
default:
// Shortcut for operations that we do not handle.
if (type == nullptr) {
type = GetUpperBound(node);
info->set_unrestricted_feedback_type(type);
info->set_feedback_type(
Type::Intersect(type, info->restriction_type(), graph_zone()));
GetInfo(node)->set_feedback_type(NodeProperties::GetType(node));
return true;
}
return false;
}
Type* unrestricted_feedback_type = new_type;
new_type =
Type::Intersect(new_type, info->restriction_type(), graph_zone());
// We need to guarantee that the feedback type is a subtype of the upper
// bound. Naively that should hold, but weakening can actually produce
// a bigger type if we are unlucky with ordering of phi typing. To be
@ -479,8 +463,7 @@ class RepresentationSelector {
new_type = Type::Intersect(GetUpperBound(node), new_type, graph_zone());
if (type != nullptr && new_type->Is(type)) return false;
info->set_unrestricted_feedback_type(unrestricted_feedback_type);
info->set_feedback_type(new_type);
GetInfo(node)->set_feedback_type(new_type);
if (FLAG_trace_representation) {
PrintNodeFeedbackType(node);
}
@ -1117,6 +1100,22 @@ class RepresentationSelector {
return jsgraph_->simplified();
}
void LowerToCheckedInt32Mul(Node* node, Truncation truncation,
Type* input0_type, Type* input1_type) {
// If one of the inputs is positive and/or truncation is being applied,
// there is no need to return -0.
CheckForMinusZeroMode mz_mode =
truncation.IsUsedAsWord32() ||
(input0_type->Is(Type::OrderedNumber()) &&
input0_type->Min() > 0) ||
(input1_type->Is(Type::OrderedNumber()) &&
input1_type->Min() > 0)
? CheckForMinusZeroMode::kDontCheckForMinusZero
: CheckForMinusZeroMode::kCheckForMinusZero;
NodeProperties::ChangeOp(node, simplified()->CheckedInt32Mul(mz_mode));
}
void ChangeToInt32OverflowOp(Node* node) {
NodeProperties::ChangeOp(node, Int32OverflowOp(node));
}
@ -1144,32 +1143,29 @@ class RepresentationSelector {
return;
}
// We always take SignedSmall/Signed32 feedback otherwise.
NumberOperationHint const hint = NumberOperationHintOf(node->op());
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
// Check if we can guarantee truncations for the inputs.
if (BothInputsAre(node, Type::Signed32()) ||
(BothInputsAre(node, Type::Signed32OrMinusZero()) &&
NodeProperties::GetType(node)->Is(type_cache_.kSafeInteger))) {
// Try to use type feedback.
NumberOperationHint hint = NumberOperationHintOf(node->op());
// Handle the case when no int32 checks on inputs are necessary
// (but an overflow check is needed on the output).
if (BothInputsAre(node, Type::Signed32()) ||
(BothInputsAre(node, Type::Signed32OrMinusZero()) &&
NodeProperties::GetType(node)->Is(type_cache_.kSafeInteger))) {
// If both the inputs the feedback are int32, use the overflow op.
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
VisitBinop(node, UseInfo::TruncatingWord32(),
MachineRepresentation::kWord32, Type::Signed32());
} else {
VisitBinop(node, CheckedUseInfoAsWord32FromHint(hint),
MachineRepresentation::kWord32, Type::Signed32());
}
if (lower()) {
// Avoid overflow checks if the unrestricted feedback type of {node}
// suggests that the result will fit into Signed32/Unsigned32 range.
Type* const type = UnrestrictedFeedbackTypeOf(node);
if (type->Is(Type::Unsigned32())) {
ChangeToPureOp(node, Uint32Op(node));
} else if (type->Is(Type::Signed32())) {
ChangeToPureOp(node, Int32Op(node));
} else {
ChangeToInt32OverflowOp(node);
}
if (lower()) ChangeToInt32OverflowOp(node);
return;
}
}
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
VisitBinop(node, CheckedUseInfoAsWord32FromHint(hint),
MachineRepresentation::kWord32, Type::Signed32());
if (lower()) ChangeToInt32OverflowOp(node);
return;
}
@ -1179,6 +1175,7 @@ class RepresentationSelector {
if (lower()) {
ChangeToPureOp(node, Float64Op(node));
}
return;
}
void VisitSpeculativeNumberModulus(Node* node, Truncation truncation,
@ -1520,39 +1517,33 @@ class RepresentationSelector {
if (lower()) ChangeToPureOp(node, Int32Op(node));
return;
}
// Try to use type feedback.
NumberOperationHint hint = NumberOperationHintOf(node->op());
Type* input0_type = TypeOf(node->InputAt(0));
Type* input1_type = TypeOf(node->InputAt(1));
// We always take SignedSmall/Signed32 feedback otherwise.
NumberOperationHint const hint = NumberOperationHintOf(node->op());
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
// Handle the case when no int32 checks on inputs are necessary
// (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.
// Handle the case when no int32 checks on inputs are necessary
// (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::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
VisitBinop(node, UseInfo::TruncatingWord32(),
MachineRepresentation::kWord32, Type::Signed32());
} else {
VisitBinop(node, CheckedUseInfoAsWord32FromHint(hint),
MachineRepresentation::kWord32, Type::Signed32());
}
if (lower()) {
// Avoid overflow checks if the unrestricted feedback type of {node}
// suggests that the result will fit into Signed32/Unsigned32 range,
// or at least try to avoid the expensive minus zero checks.
Type* const type = UnrestrictedFeedbackTypeOf(node);
if (type->Is(Type::Unsigned32())) {
ChangeToPureOp(node, Uint32Op(node));
} else if (type->Is(Type::Signed32())) {
ChangeToPureOp(node, Int32Op(node));
} else {
CheckForMinusZeroMode const mode =
(truncation.IdentifiesMinusZeroAndZero() ||
!type->Maybe(Type::MinusZero()))
? CheckForMinusZeroMode::kDontCheckForMinusZero
: CheckForMinusZeroMode::kCheckForMinusZero;
NodeProperties::ChangeOp(node,
simplified()->CheckedInt32Mul(mode));
if (lower()) {
LowerToCheckedInt32Mul(node, truncation, input0_type,
input1_type);
}
return;
}
}
if (hint == NumberOperationHint::kSignedSmall ||
hint == NumberOperationHint::kSigned32) {
VisitBinop(node, CheckedUseInfoAsWord32FromHint(hint),
MachineRepresentation::kWord32, Type::Signed32());
if (lower()) {
LowerToCheckedInt32Mul(node, truncation, input0_type, input1_type);
}
return;
}

17
test/mjsunit/regress/regress-5357.js Normal file
View File

@ -0,0 +1,17 @@
// Copyright 2016 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 foo(a) {
a++;
a = Math.max(0, a);
a++;
return a;
}
foo(0);
foo(0);
%OptimizeFunctionOnNextCall(foo);
assertEquals(2147483648, foo(2147483646));