Skip to content

Commit 609d2b9

Browse files
targosBethGriggs
authored andcommitted
deps: V8: backport f27ac28
Original commit message: [turbofan] Pin pure unreachable values to effect chain (in rep selection) Currently, if we lower to a pure computation that is unreachable because of some runtime check, we just rename it with DeadValue. This is problematic if the pure computation gets later eliminated - that allows the DeadValue node float above the check that makes it dead. As we conservatively lower DeadValues to debug-break (i.e., crash), we might induce crash where we should not. With this CL, whenever we lower an impossible effectful node (i.e., with Type::None) to a pure node in simplified lowering, we insert an Unreachable node there (pinned to the effect chain) and mark the impossible node dead (and make it depend on the Unreachable node). Bug: chromium:910838 Change-Id: I218991c79b9e283a9dd5beb4d3f0c4664be76cb2 Reviewed-on: https://chromium-review.googlesource.com/c/1365274 Reviewed-by: Benedikt Meurer <bmeurer@chromium.org> Commit-Queue: Jaroslav Sevcik <jarin@chromium.org> Cr-Commit-Position: refs/heads/master@{#58066} Refs: v8/v8@f27ac28 PR-URL: #28061 Fixes: #27107 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
1 parent 8f780e8 commit 609d2b9

File tree

4 files changed

+73
-36
lines changed

4 files changed

+73
-36
lines changed

common.gypi

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333

3434
# Reset this number to 0 on major V8 upgrades.
3535
# Increment by one for each non-official patch applied to deps/v8.
36-
'v8_embedder_string': '-node.53',
36+
'v8_embedder_string': '-node.54',
3737

3838
# Enable disassembler for `--print-code` v8 options
3939
'v8_enable_disassembler': 1,

deps/v8/src/compiler/simplified-lowering.cc

+52-34
Original file line numberDiff line numberDiff line change
@@ -171,21 +171,6 @@ void ReplaceEffectControlUses(Node* node, Node* effect, Node* control) {
171171
}
172172
}
173173

174-
void ChangeToPureOp(Node* node, const Operator* new_op) {
175-
DCHECK(new_op->HasProperty(Operator::kPure));
176-
if (node->op()->EffectInputCount() > 0) {
177-
DCHECK_LT(0, node->op()->ControlInputCount());
178-
// Disconnect the node from effect and control chains.
179-
Node* control = NodeProperties::GetControlInput(node);
180-
Node* effect = NodeProperties::GetEffectInput(node);
181-
ReplaceEffectControlUses(node, effect, control);
182-
node->TrimInputCount(new_op->ValueInputCount());
183-
} else {
184-
DCHECK_EQ(0, node->op()->ControlInputCount());
185-
}
186-
NodeProperties::ChangeOp(node, new_op);
187-
}
188-
189174
#ifdef DEBUG
190175
// Helpers for monotonicity checking.
191176
class InputUseInfos {
@@ -750,6 +735,31 @@ class RepresentationSelector {
750735
!GetUpperBound(node->InputAt(1)).Maybe(type);
751736
}
752737

738+
void ChangeToPureOp(Node* node, const Operator* new_op) {
739+
DCHECK(new_op->HasProperty(Operator::kPure));
740+
if (node->op()->EffectInputCount() > 0) {
741+
DCHECK_LT(0, node->op()->ControlInputCount());
742+
Node* control = NodeProperties::GetControlInput(node);
743+
Node* effect = NodeProperties::GetEffectInput(node);
744+
if (TypeOf(node).IsNone()) {
745+
// If the node is unreachable, insert an Unreachable node and mark the
746+
// value dead.
747+
// TODO(jarin,tebbi) Find a way to unify/merge this insertion with
748+
// InsertUnreachableIfNecessary.
749+
Node* unreachable = effect = graph()->NewNode(
750+
jsgraph_->common()->Unreachable(), effect, control);
751+
new_op = jsgraph_->common()->DeadValue(GetInfo(node)->representation());
752+
node->ReplaceInput(0, unreachable);
753+
}
754+
// Rewire the effect and control chains.
755+
node->TrimInputCount(new_op->ValueInputCount());
756+
ReplaceEffectControlUses(node, effect, control);
757+
} else {
758+
DCHECK_EQ(0, node->op()->ControlInputCount());
759+
}
760+
NodeProperties::ChangeOp(node, new_op);
761+
}
762+
753763
// Converts input {index} of {node} according to given UseInfo {use},
754764
// assuming the type of the input is {input_type}. If {input_type} is null,
755765
// it takes the input from the input node {TypeOf(node->InputAt(index))}.
@@ -1052,6 +1062,15 @@ class RepresentationSelector {
10521062
}
10531063
}
10541064

1065+
void MaskShiftOperand(Node* node, Type rhs_type) {
1066+
if (!rhs_type.Is(type_cache_.kZeroToThirtyOne)) {
1067+
Node* const rhs = NodeProperties::GetValueInput(node, 1);
1068+
node->ReplaceInput(1,
1069+
graph()->NewNode(jsgraph_->machine()->Word32And(), rhs,
1070+
jsgraph_->Int32Constant(0x1F)));
1071+
}
1072+
}
1073+
10551074
static MachineSemantic DeoptValueSemanticOf(Type type) {
10561075
// We only need signedness to do deopt correctly.
10571076
if (type.Is(Type::Signed32())) {
@@ -1996,7 +2015,8 @@ class RepresentationSelector {
19962015
VisitBinop(node, UseInfo::TruncatingWord32(),
19972016
UseInfo::TruncatingWord32(), MachineRepresentation::kWord32);
19982017
if (lower()) {
1999-
lowering->DoShift(node, lowering->machine()->Word32Shl(), rhs_type);
2018+
MaskShiftOperand(node, rhs_type);
2019+
ChangeToPureOp(node, lowering->machine()->Word32Shl());
20002020
}
20012021
return;
20022022
}
@@ -2007,7 +2027,8 @@ class RepresentationSelector {
20072027
UseInfo::TruncatingWord32(),
20082028
MachineRepresentation::kWord32);
20092029
if (lower()) {
2010-
lowering->DoShift(node, lowering->machine()->Word32Shl(), rhs_type);
2030+
MaskShiftOperand(node, rhs_type);
2031+
ChangeToPureOp(node, lowering->machine()->Word32Shl());
20112032
}
20122033
return;
20132034
}
@@ -2016,7 +2037,8 @@ class RepresentationSelector {
20162037
VisitBinop(node, CheckedUseInfoAsWord32FromHint(hint),
20172038
MachineRepresentation::kWord32, Type::Signed32());
20182039
if (lower()) {
2019-
lowering->DoShift(node, lowering->machine()->Word32Shl(), rhs_type);
2040+
MaskShiftOperand(node, rhs_type);
2041+
ChangeToPureOp(node, lowering->machine()->Word32Shl());
20202042
}
20212043
return;
20222044
}
@@ -2025,7 +2047,8 @@ class RepresentationSelector {
20252047
VisitBinop(node, UseInfo::TruncatingWord32(),
20262048
UseInfo::TruncatingWord32(), MachineRepresentation::kWord32);
20272049
if (lower()) {
2028-
lowering->DoShift(node, lowering->machine()->Word32Sar(), rhs_type);
2050+
MaskShiftOperand(node, rhs_type);
2051+
ChangeToPureOp(node, lowering->machine()->Word32Sar());
20292052
}
20302053
return;
20312054
}
@@ -2036,7 +2059,8 @@ class RepresentationSelector {
20362059
UseInfo::TruncatingWord32(),
20372060
MachineRepresentation::kWord32);
20382061
if (lower()) {
2039-
lowering->DoShift(node, lowering->machine()->Word32Sar(), rhs_type);
2062+
MaskShiftOperand(node, rhs_type);
2063+
ChangeToPureOp(node, lowering->machine()->Word32Sar());
20402064
}
20412065
return;
20422066
}
@@ -2045,7 +2069,8 @@ class RepresentationSelector {
20452069
VisitBinop(node, CheckedUseInfoAsWord32FromHint(hint),
20462070
MachineRepresentation::kWord32, Type::Signed32());
20472071
if (lower()) {
2048-
lowering->DoShift(node, lowering->machine()->Word32Sar(), rhs_type);
2072+
MaskShiftOperand(node, rhs_type);
2073+
ChangeToPureOp(node, lowering->machine()->Word32Sar());
20492074
}
20502075
return;
20512076
}
@@ -2054,7 +2079,8 @@ class RepresentationSelector {
20542079
VisitBinop(node, UseInfo::TruncatingWord32(),
20552080
UseInfo::TruncatingWord32(), MachineRepresentation::kWord32);
20562081
if (lower()) {
2057-
lowering->DoShift(node, lowering->machine()->Word32Shr(), rhs_type);
2082+
MaskShiftOperand(node, rhs_type);
2083+
ChangeToPureOp(node, lowering->machine()->Word32Shr());
20582084
}
20592085
return;
20602086
}
@@ -2083,14 +2109,16 @@ class RepresentationSelector {
20832109
UseInfo::TruncatingWord32(),
20842110
MachineRepresentation::kWord32);
20852111
if (lower()) {
2086-
lowering->DoShift(node, lowering->machine()->Word32Shr(), rhs_type);
2112+
MaskShiftOperand(node, rhs_type);
2113+
ChangeToPureOp(node, lowering->machine()->Word32Shr());
20872114
}
20882115
return;
20892116
}
20902117
VisitBinop(node, CheckedUseInfoAsWord32FromHint(hint),
20912118
MachineRepresentation::kWord32, Type::Unsigned32());
20922119
if (lower()) {
2093-
lowering->DoShift(node, lowering->machine()->Word32Shr(), rhs_type);
2120+
MaskShiftOperand(node, rhs_type);
2121+
ChangeToPureOp(node, lowering->machine()->Word32Shr());
20942122
}
20952123
return;
20962124
}
@@ -3787,16 +3815,6 @@ void SimplifiedLowering::DoMin(Node* node, Operator const* op,
37873815
NodeProperties::ChangeOp(node, common()->Select(rep));
37883816
}
37893817

3790-
void SimplifiedLowering::DoShift(Node* node, Operator const* op,
3791-
Type rhs_type) {
3792-
if (!rhs_type.Is(type_cache_.kZeroToThirtyOne)) {
3793-
Node* const rhs = NodeProperties::GetValueInput(node, 1);
3794-
node->ReplaceInput(1, graph()->NewNode(machine()->Word32And(), rhs,
3795-
jsgraph()->Int32Constant(0x1F)));
3796-
}
3797-
ChangeToPureOp(node, op);
3798-
}
3799-
38003818
void SimplifiedLowering::DoIntegral32ToBit(Node* node) {
38013819
Node* const input = node->InputAt(0);
38023820
Node* const zero = jsgraph()->Int32Constant(0);

deps/v8/src/compiler/simplified-lowering.h

-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ class V8_EXPORT_PRIVATE SimplifiedLowering final {
3737
Node* node, RepresentationSelector* selector);
3838
void DoJSToNumberOrNumericTruncatesToWord32(Node* node,
3939
RepresentationSelector* selector);
40-
void DoShift(Node* node, Operator const* op, Type rhs_type);
4140
void DoIntegral32ToBit(Node* node);
4241
void DoOrderedNumberToBit(Node* node);
4342
void DoNumberToBit(Node* node);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2018 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// Flags: --allow-natives-syntax
6+
7+
function f(b, s, x) {
8+
if (!b) {
9+
return (x ? b : s * undefined) ? 1 : 42;
10+
}
11+
}
12+
13+
function g(b, x) {
14+
return f(b, 'abc', x);
15+
}
16+
17+
f(false, 0, 0);
18+
g(true, 0);
19+
%OptimizeFunctionOnNextCall(g);
20+
assertEquals(42, g(false, 0));

0 commit comments

Comments
 (0)