Commit ec81d82d authored by Manos Koukoutos's avatar Manos Koukoutos Committed by Commit Bot

Reland "[turbofan] Optimize TrapIf/Unless in BranchElim. and CommonOp-Reducer"

This is a reland of a3b1233e

Changes compared to original commit:
- Use a more canonical way to replace TrapIf/Unless nodes that always
  trap. This fixes the issue where their outputs were marked dead even
  if they were Merge/Loop nodes.
- Use Throw() over Return() to connect a dangling trap to End().
- Add regression test.

Original change's description:
> [turbofan] Optimize TrapIf/Unless in BranchElim. and CommonOp-Reducer
>
> Bug: v8:11510
> Change-Id: I1e8fcb54444e494c7d765ad556d09d954441361f
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2752876
> Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
> Reviewed-by: Georg Neis <neis@chromium.org>
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#73468}

Bug: v8:11510, chromium:1189454
Change-Id: I1d691a3ea299ed668cff925910ed231aad37cac6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2772601
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73537}
parent c0ee8f90
......@@ -42,6 +42,9 @@ Reduction BranchElimination::Reduce(Node* node) {
return ReduceIf(node, false);
case IrOpcode::kIfTrue:
return ReduceIf(node, true);
case IrOpcode::kTrapIf:
case IrOpcode::kTrapUnless:
return ReduceTrapConditional(node);
case IrOpcode::kStart:
return ReduceStart(node);
default:
......@@ -71,9 +74,9 @@ void BranchElimination::SimplifyBranchCondition(Node* branch) {
// | \ / \ /
// | \ / \ /
// | first_merge ==> first_merge
// | | |
// second_branch 1 0 |
// / \ \ / |
// | | / |
// second_branch 1 0 / |
// / \ \ | / |
// / \ phi |
// second_true second_false \ |
// second_branch
......@@ -154,6 +157,42 @@ Reduction BranchElimination::ReduceBranch(Node* node) {
return TakeConditionsFromFirstControl(node);
}
Reduction BranchElimination::ReduceTrapConditional(Node* node) {
DCHECK(node->opcode() == IrOpcode::kTrapIf ||
node->opcode() == IrOpcode::kTrapUnless);
bool trapping_condition = node->opcode() == IrOpcode::kTrapIf;
Node* condition = node->InputAt(0);
Node* control_input = NodeProperties::GetControlInput(node, 0);
// If we do not know anything about the predecessor, do not propagate just
// yet because we will have to recompute anyway once we compute the
// predecessor.
if (!reduced_.Get(control_input)) {
return NoChange();
}
ControlPathConditions from_input = node_conditions_.Get(control_input);
Node* branch;
bool condition_value;
if (from_input.LookupCondition(condition, &branch, &condition_value)) {
if (condition_value == trapping_condition) {
// This will always trap. Mark its outputs as dead and connect it to
// graph()->end().
ReplaceWithValue(node, dead(), dead(), dead());
Node* effect = NodeProperties::GetEffectInput(node);
Node* control = graph()->NewNode(common()->Throw(), effect, node);
NodeProperties::MergeControlToEnd(graph(), common(), control);
Revisit(graph()->end());
return Changed(node);
} else {
// This will not trap, remove it.
return Replace(control_input);
}
}
return UpdateConditions(node, from_input, condition, node,
!trapping_condition);
}
Reduction BranchElimination::ReduceDeoptimizeConditional(Node* node) {
DCHECK(node->opcode() == IrOpcode::kDeoptimizeIf ||
node->opcode() == IrOpcode::kDeoptimizeUnless);
......@@ -302,7 +341,9 @@ bool BranchElimination::ControlPathConditions::LookupCondition(
void BranchElimination::MarkAsSafetyCheckIfNeeded(Node* branch, Node* node) {
// Check if {branch} is dead because we might have a stale side-table entry.
if (!branch->IsDead() && branch->opcode() != IrOpcode::kDead) {
if (!branch->IsDead() && branch->opcode() != IrOpcode::kDead &&
branch->opcode() != IrOpcode::kTrapIf &&
branch->opcode() != IrOpcode::kTrapUnless) {
IsSafetyCheck branch_safety = IsSafetyCheckOf(branch->op());
IsSafetyCheck combined_safety =
CombineSafetyChecks(branch_safety, IsSafetyCheckOf(node->op()));
......
......@@ -63,6 +63,7 @@ class V8_EXPORT_PRIVATE BranchElimination final
Reduction ReduceBranch(Node* node);
Reduction ReduceDeoptimizeConditional(Node* node);
Reduction ReduceIf(Node* node, bool is_true_branch);
Reduction ReduceTrapConditional(Node* node);
Reduction ReduceLoop(Node* node);
Reduction ReduceMerge(Node* node);
Reduction ReduceStart(Node* node);
......
......@@ -74,6 +74,9 @@ Reduction CommonOperatorReducer::Reduce(Node* node) {
return ReduceSwitch(node);
case IrOpcode::kStaticAssert:
return ReduceStaticAssert(node);
case IrOpcode::kTrapIf:
case IrOpcode::kTrapUnless:
return ReduceTrapConditional(node);
default:
break;
}
......@@ -472,6 +475,30 @@ Reduction CommonOperatorReducer::ReduceStaticAssert(Node* node) {
}
}
Reduction CommonOperatorReducer::ReduceTrapConditional(Node* trap) {
DCHECK(trap->opcode() == IrOpcode::kTrapIf ||
trap->opcode() == IrOpcode::kTrapUnless);
bool trapping_condition = trap->opcode() == IrOpcode::kTrapIf;
Node* const cond = trap->InputAt(0);
Decision decision = DecideCondition(broker(), cond);
if (decision == Decision::kUnknown) {
return NoChange();
} else if ((decision == Decision::kTrue) == trapping_condition) {
// This will always trap. Mark its outputs as dead and connect it to
// graph()->end().
ReplaceWithValue(trap, dead(), dead(), dead());
Node* effect = NodeProperties::GetEffectInput(trap);
Node* control = graph()->NewNode(common()->Throw(), effect, trap);
NodeProperties::MergeControlToEnd(graph(), common(), control);
Revisit(graph()->end());
return Changed(trap);
} else {
// This will not trap, remove it.
return Replace(NodeProperties::GetControlInput(trap));
}
}
Reduction CommonOperatorReducer::Change(Node* node, Operator const* op,
Node* a) {
node->ReplaceInput(0, a);
......
......@@ -43,6 +43,7 @@ class V8_EXPORT_PRIVATE CommonOperatorReducer final
Reduction ReduceSelect(Node* node);
Reduction ReduceSwitch(Node* node);
Reduction ReduceStaticAssert(Node* node);
Reduction ReduceTrapConditional(Node* node);
Reduction Change(Node* node, Operator const* op, Node* a);
Reduction Change(Node* node, Operator const* op, Node* a, Node* b);
......
......@@ -1396,7 +1396,8 @@ Node* WasmGraphBuilder::Return(Vector<Node*> vals) {
Node* WasmGraphBuilder::Trap(wasm::TrapReason reason,
wasm::WasmCodePosition position) {
TrapIfFalse(reason, Int32Constant(0), position);
Return(Vector<Node*>{});
// Connect control to end via a Throw() node.
TerminateThrow(effect(), control());
return nullptr;
}
......
This diff is collapsed.
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