Commit 34c9d7d8 authored by Tobias Tebbi's avatar Tobias Tebbi Committed by Commit Bot

[turbofan] BranchElimination: mark branches as safety checks when removing safety checks

Bug: chromium:798964
Change-Id: Ia34e901ed04daae62e6ec82c972225fb5de68419
Reviewed-on: https://chromium-review.googlesource.com/892443
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51062}
parent 610a3610
...@@ -57,17 +57,23 @@ Reduction BranchElimination::ReduceBranch(Node* node) { ...@@ -57,17 +57,23 @@ Reduction BranchElimination::ReduceBranch(Node* node) {
Node* condition = node->InputAt(0); Node* condition = node->InputAt(0);
Node* control_input = NodeProperties::GetControlInput(node, 0); Node* control_input = NodeProperties::GetControlInput(node, 0);
ControlPathConditions from_input = node_conditions_.Get(control_input); ControlPathConditions from_input = node_conditions_.Get(control_input);
Maybe<bool> condition_value = from_input.LookupCondition(condition); Node* branch;
bool condition_value;
// If we know the condition we can discard the branch. // If we know the condition we can discard the branch.
if (condition_value.IsJust()) { if (from_input.LookupCondition(condition, &branch, &condition_value)) {
bool known_value = condition_value.FromJust(); // Mark the branch as a safety check.
if (IsSafetyCheckOf(node->op()) == IsSafetyCheck::kSafetyCheck) {
NodeProperties::ChangeOp(branch,
common()->MarkAsSafetyCheck(branch->op()));
}
for (Node* const use : node->uses()) { for (Node* const use : node->uses()) {
switch (use->opcode()) { switch (use->opcode()) {
case IrOpcode::kIfTrue: case IrOpcode::kIfTrue:
Replace(use, known_value ? control_input : dead()); Replace(use, condition_value ? control_input : dead());
break; break;
case IrOpcode::kIfFalse: case IrOpcode::kIfFalse:
Replace(use, known_value ? dead() : control_input); Replace(use, condition_value ? dead() : control_input);
break; break;
default: default:
UNREACHABLE(); UNREACHABLE();
...@@ -95,10 +101,17 @@ Reduction BranchElimination::ReduceDeoptimizeConditional(Node* node) { ...@@ -95,10 +101,17 @@ Reduction BranchElimination::ReduceDeoptimizeConditional(Node* node) {
} }
ControlPathConditions conditions = node_conditions_.Get(control); ControlPathConditions conditions = node_conditions_.Get(control);
Maybe<bool> condition_value = conditions.LookupCondition(condition); bool condition_value;
if (condition_value.IsJust()) { Node* branch;
if (conditions.LookupCondition(condition, &branch, &condition_value)) {
// Mark the branch as a safety check.
if (p.is_safety_check() == IsSafetyCheck::kSafetyCheck) {
NodeProperties::ChangeOp(branch,
common()->MarkAsSafetyCheck(branch->op()));
}
// If we know the condition we can discard the branch. // If we know the condition we can discard the branch.
if (condition_is_true == condition_value.FromJust()) { if (condition_is_true == condition_value) {
// We don't update the conditions here, because we're replacing {node} // We don't update the conditions here, because we're replacing {node}
// with the {control} node that already contains the right information. // with the {control} node that already contains the right information.
ReplaceWithValue(node, dead(), effect, control); ReplaceWithValue(node, dead(), effect, control);
...@@ -112,7 +125,7 @@ Reduction BranchElimination::ReduceDeoptimizeConditional(Node* node) { ...@@ -112,7 +125,7 @@ Reduction BranchElimination::ReduceDeoptimizeConditional(Node* node) {
} }
return Replace(dead()); return Replace(dead());
} }
return UpdateConditions(node, conditions, condition, condition_is_true); return UpdateConditions(node, conditions, condition, node, condition_is_true);
} }
Reduction BranchElimination::ReduceIf(Node* node, bool is_true_branch) { Reduction BranchElimination::ReduceIf(Node* node, bool is_true_branch) {
...@@ -126,7 +139,7 @@ Reduction BranchElimination::ReduceIf(Node* node, bool is_true_branch) { ...@@ -126,7 +139,7 @@ Reduction BranchElimination::ReduceIf(Node* node, bool is_true_branch) {
return NoChange(); return NoChange();
} }
Node* condition = branch->InputAt(0); Node* condition = branch->InputAt(0);
return UpdateConditions(node, from_branch, condition, is_true_branch); return UpdateConditions(node, from_branch, condition, branch, is_true_branch);
} }
...@@ -198,36 +211,40 @@ Reduction BranchElimination::UpdateConditions( ...@@ -198,36 +211,40 @@ Reduction BranchElimination::UpdateConditions(
Reduction BranchElimination::UpdateConditions( Reduction BranchElimination::UpdateConditions(
Node* node, ControlPathConditions prev_conditions, Node* current_condition, Node* node, ControlPathConditions prev_conditions, Node* current_condition,
bool is_true_branch) { Node* current_branch, bool is_true_branch) {
ControlPathConditions original = node_conditions_.Get(node); ControlPathConditions original = node_conditions_.Get(node);
// The control path for the node is the path obtained by appending the // The control path for the node is the path obtained by appending the
// current_condition to the prev_conditions. Use the original control path as // current_condition to the prev_conditions. Use the original control path as
// a hint to avoid allocations. // a hint to avoid allocations.
prev_conditions.AddCondition(zone_, current_condition, is_true_branch, prev_conditions.AddCondition(zone_, current_condition, current_branch,
original); is_true_branch, original);
return UpdateConditions(node, prev_conditions); return UpdateConditions(node, prev_conditions);
} }
void BranchElimination::ControlPathConditions::AddCondition( void BranchElimination::ControlPathConditions::AddCondition(
Zone* zone, Node* condition, bool is_true, ControlPathConditions hint) { Zone* zone, Node* condition, Node* branch, bool is_true,
DCHECK(LookupCondition(condition).IsNothing()); ControlPathConditions hint) {
PushFront({condition, is_true}, zone, hint); DCHECK_EQ(false, LookupCondition(condition, nullptr, nullptr));
PushFront({condition, branch, is_true}, zone, hint);
} }
bool BranchElimination::ControlPathConditions::LookupCondition(
Maybe<bool> BranchElimination::ControlPathConditions::LookupCondition( Node* condition, Node** branch, bool* is_true) const {
Node* condition) const {
for (BranchCondition element : *this) { for (BranchCondition element : *this) {
if (element.condition == condition) return Just<bool>(element.is_true); if (element.condition == condition) {
*is_true = element.is_true;
*branch = element.branch;
return true;
}
}
return false;
} }
return Nothing<bool>();
}
Graph* BranchElimination::graph() const { return jsgraph()->graph(); } Graph* BranchElimination::graph() const { return jsgraph()->graph(); }
CommonOperatorBuilder* BranchElimination::common() const { CommonOperatorBuilder* BranchElimination::common() const {
return jsgraph()->common(); return jsgraph()->common();
} }
} // namespace compiler } // namespace compiler
} // namespace internal } // namespace internal
......
...@@ -32,10 +32,12 @@ class V8_EXPORT_PRIVATE BranchElimination final ...@@ -32,10 +32,12 @@ class V8_EXPORT_PRIVATE BranchElimination final
private: private:
struct BranchCondition { struct BranchCondition {
Node* condition; Node* condition;
Node* branch;
bool is_true; bool is_true;
bool operator==(BranchCondition other) const { bool operator==(BranchCondition other) const {
return condition == other.condition && is_true == other.is_true; return condition == other.condition && branch == other.branch &&
is_true == other.is_true;
} }
bool operator!=(BranchCondition other) const { return !(*this == other); } bool operator!=(BranchCondition other) const { return !(*this == other); }
}; };
...@@ -45,8 +47,8 @@ class V8_EXPORT_PRIVATE BranchElimination final ...@@ -45,8 +47,8 @@ class V8_EXPORT_PRIVATE BranchElimination final
// (true or false). // (true or false).
class ControlPathConditions : public FunctionalList<BranchCondition> { class ControlPathConditions : public FunctionalList<BranchCondition> {
public: public:
Maybe<bool> LookupCondition(Node* condition) const; bool LookupCondition(Node* condition, Node** branch, bool* is_true) const;
void AddCondition(Zone* zone, Node* condition, bool is_true, void AddCondition(Zone* zone, Node* condition, Node* branch, bool is_true,
ControlPathConditions hint); ControlPathConditions hint);
private: private:
...@@ -64,7 +66,8 @@ class V8_EXPORT_PRIVATE BranchElimination final ...@@ -64,7 +66,8 @@ class V8_EXPORT_PRIVATE BranchElimination final
Reduction TakeConditionsFromFirstControl(Node* node); Reduction TakeConditionsFromFirstControl(Node* node);
Reduction UpdateConditions(Node* node, ControlPathConditions conditions); Reduction UpdateConditions(Node* node, ControlPathConditions conditions);
Reduction UpdateConditions(Node* node, ControlPathConditions prev_conditions, Reduction UpdateConditions(Node* node, ControlPathConditions prev_conditions,
Node* current_condition, bool is_true_branch); Node* current_condition, Node* current_branch,
bool is_true_branch);
Node* dead() const { return dead_; } Node* dead() const { return dead_; }
Graph* graph() const; Graph* graph() const;
......
...@@ -96,6 +96,26 @@ IsSafetyCheck IsSafetyCheckOf(const Operator* op) { ...@@ -96,6 +96,26 @@ IsSafetyCheck IsSafetyCheckOf(const Operator* op) {
return DeoptimizeParametersOf(op).is_safety_check(); return DeoptimizeParametersOf(op).is_safety_check();
} }
const Operator* CommonOperatorBuilder::MarkAsSafetyCheck(const Operator* op) {
if (op->opcode() == IrOpcode::kBranch) {
BranchOperatorInfo info = BranchOperatorInfoOf(op);
if (info.is_safety_check == IsSafetyCheck::kSafetyCheck) return op;
return Branch(info.hint, IsSafetyCheck::kSafetyCheck);
}
DeoptimizeParameters p = DeoptimizeParametersOf(op);
if (p.is_safety_check() == IsSafetyCheck::kSafetyCheck) return op;
switch (op->opcode()) {
case IrOpcode::kDeoptimizeIf:
return DeoptimizeIf(p.kind(), p.reason(), p.feedback(),
IsSafetyCheck::kSafetyCheck);
case IrOpcode::kDeoptimizeUnless:
return DeoptimizeUnless(p.kind(), p.reason(), p.feedback(),
IsSafetyCheck::kSafetyCheck);
default:
UNREACHABLE();
}
}
bool operator==(SelectParameters const& lhs, SelectParameters const& rhs) { bool operator==(SelectParameters const& lhs, SelectParameters const& rhs) {
return lhs.representation() == rhs.representation() && return lhs.representation() == rhs.representation() &&
lhs.hint() == rhs.hint(); lhs.hint() == rhs.hint();
......
...@@ -476,6 +476,8 @@ class V8_EXPORT_PRIVATE CommonOperatorBuilder final ...@@ -476,6 +476,8 @@ class V8_EXPORT_PRIVATE CommonOperatorBuilder final
FrameStateType type, int parameter_count, int local_count, FrameStateType type, int parameter_count, int local_count,
Handle<SharedFunctionInfo> shared_info); Handle<SharedFunctionInfo> shared_info);
const Operator* MarkAsSafetyCheck(const Operator* op);
private: private:
Zone* zone() const { return zone_; } Zone* zone() const { return zone_; }
......
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