Commit ef4de566 authored by Manos Koukoutos's avatar Manos Koukoutos Committed by V8 LUCI CQ

[turbofan] Optimize BranchConditions in BranchElimination

When BranchElimination has to find the common prefix of a set of
BranchConditions in a Merge, it has to traverse a number of linked lists
of individual conditions, which is inefficient.
This CL improves its performance by grouping conditions between an
IfTrue/IfFalse and a Merge in a single entry of BranchConditions.
Additional change: Improve documentation of FunctionalList.

Change-Id: I93a58886151f6831cafb483aafb48e8e6c2433e5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2917600
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: 's avatarGeorg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74793}
parent f370d607
......@@ -170,7 +170,6 @@ Reduction BranchElimination::ReduceTrapConditional(Node* node) {
return NoChange();
}
ControlPathConditions from_input = node_conditions_.Get(control_input);
Node* branch;
bool condition_value;
......@@ -190,7 +189,7 @@ Reduction BranchElimination::ReduceTrapConditional(Node* node) {
}
}
return UpdateConditions(node, from_input, condition, node,
!trapping_condition);
!trapping_condition, false);
}
Reduction BranchElimination::ReduceDeoptimizeConditional(Node* node) {
......@@ -229,7 +228,8 @@ Reduction BranchElimination::ReduceDeoptimizeConditional(Node* node) {
}
return Replace(dead());
}
return UpdateConditions(node, conditions, condition, node, condition_is_true);
return UpdateConditions(node, conditions, condition, node, condition_is_true,
false);
}
Reduction BranchElimination::ReduceIf(Node* node, bool is_true_branch) {
......@@ -243,7 +243,8 @@ Reduction BranchElimination::ReduceIf(Node* node, bool is_true_branch) {
return NoChange();
}
Node* condition = branch->InputAt(0);
return UpdateConditions(node, from_branch, condition, branch, is_true_branch);
return UpdateConditions(node, from_branch, condition, branch, is_true_branch,
true);
}
Reduction BranchElimination::ReduceLoop(Node* node) {
......@@ -273,9 +274,9 @@ Reduction BranchElimination::ReduceMerge(Node* node) {
// inputs.
auto input_end = inputs.end();
for (; input_it != input_end; ++input_it) {
// Change the current condition list to a longest common tail
// of this condition list and the other list. (The common tail
// should correspond to the list from the common dominator.)
// Change the current condition block list to a longest common tail of this
// condition list and the other list. (The common tail should correspond to
// the list from the common dominator.)
conditions.ResetToCommonAncestor(node_conditions_.Get(*input_it));
}
return UpdateConditions(node, conditions);
......@@ -310,13 +311,18 @@ Reduction BranchElimination::UpdateConditions(
Reduction BranchElimination::UpdateConditions(
Node* node, ControlPathConditions prev_conditions, Node* current_condition,
Node* current_branch, bool is_true_branch) {
ControlPathConditions original = node_conditions_.Get(node);
Node* current_branch, bool is_true_branch, bool in_new_block) {
// 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
// a hint to avoid allocations.
prev_conditions.AddCondition(zone_, current_condition, current_branch,
is_true_branch, original);
if (in_new_block || prev_conditions.Size() == 0) {
prev_conditions.AddConditionInNewBlock(zone_, current_condition,
current_branch, is_true_branch);
} else {
ControlPathConditions original = node_conditions_.Get(node);
prev_conditions.AddCondition(zone_, current_condition, current_branch,
is_true_branch, original);
}
return UpdateConditions(node, prev_conditions);
}
......@@ -324,25 +330,45 @@ void BranchElimination::ControlPathConditions::AddCondition(
Zone* zone, Node* condition, Node* branch, bool is_true,
ControlPathConditions hint) {
if (!LookupCondition(condition)) {
PushFront({condition, branch, is_true}, zone, hint);
FunctionalList<BranchCondition> prev_front = Front();
if (hint.Size() > 0) {
prev_front.PushFront({condition, branch, is_true}, zone, hint.Front());
} else {
prev_front.PushFront({condition, branch, is_true}, zone);
}
DropFront();
PushFront(prev_front, zone);
}
}
void BranchElimination::ControlPathConditions::AddConditionInNewBlock(
Zone* zone, Node* condition, Node* branch, bool is_true) {
FunctionalList<BranchCondition> new_block;
if (!LookupCondition(condition)) {
new_block.PushFront({condition, branch, is_true}, zone);
}
PushFront(new_block, zone);
}
bool BranchElimination::ControlPathConditions::LookupCondition(
Node* condition) const {
for (BranchCondition element : *this) {
if (element.condition == condition) return true;
for (auto block : *this) {
for (BranchCondition element : block) {
if (element.condition == condition) return true;
}
}
return false;
}
bool BranchElimination::ControlPathConditions::LookupCondition(
Node* condition, Node** branch, bool* is_true) const {
for (BranchCondition element : *this) {
if (element.condition == condition) {
*is_true = element.is_true;
*branch = element.branch;
return true;
for (auto block : *this) {
for (BranchCondition element : block) {
if (element.condition == condition) {
*is_true = element.is_true;
*branch = element.branch;
return true;
}
}
}
return false;
......
......@@ -35,6 +35,8 @@ class V8_EXPORT_PRIVATE BranchElimination final
Reduction Reduce(Node* node) final;
private:
// Represents a condition along with its value in the current control path.
// Also stores the node that branched on this condition.
struct BranchCondition {
Node* condition;
Node* branch;
......@@ -47,18 +49,28 @@ class V8_EXPORT_PRIVATE BranchElimination final
bool operator!=(BranchCondition other) const { return !(*this == other); }
};
// Class for tracking information about branch conditions.
// At the moment it is a linked list of conditions and their values
// (true or false).
class ControlPathConditions : public FunctionalList<BranchCondition> {
// Class for tracking information about branch conditions. It is represented
// as a linked list of condition blocks, each of which corresponds to a block
// of code bewteen an IfTrue/IfFalse and a Merge. Each block is in turn
// represented as a linked list of {BranchCondition}s.
class ControlPathConditions
: public FunctionalList<FunctionalList<BranchCondition>> {
public:
// Checks if {condition} is present in this {ControlPathConditions}.
bool LookupCondition(Node* condition) const;
// Checks if {condition} is present in this {ControlPathConditions} and
// copies its {branch} and {is_true} fields.
bool LookupCondition(Node* condition, Node** branch, bool* is_true) const;
// Adds a condition in the current code block, or a new block if the block
// list is empty.
void AddCondition(Zone* zone, Node* condition, Node* branch, bool is_true,
ControlPathConditions hint);
// Adds a condition in a new block.
void AddConditionInNewBlock(Zone* zone, Node* condition, Node* branch,
bool is_true);
private:
using FunctionalList<BranchCondition>::PushFront;
using FunctionalList<FunctionalList<BranchCondition>>::PushFront;
};
Reduction ReduceBranch(Node* node);
......@@ -75,7 +87,7 @@ class V8_EXPORT_PRIVATE BranchElimination final
Reduction UpdateConditions(Node* node, ControlPathConditions conditions);
Reduction UpdateConditions(Node* node, ControlPathConditions prev_conditions,
Node* current_condition, Node* current_branch,
bool is_true_branch);
bool is_true_branch, bool in_new_block);
void MarkAsSafetyCheckIfNeeded(Node* branch, Node* node);
Node* dead() const { return dead_; }
......
......@@ -12,10 +12,13 @@ namespace v8 {
namespace internal {
namespace compiler {
// A generic stack implemented as a purely functional singly-linked list, which
// results in an O(1) copy operation. It is the equivalent of functional lists
// in ML-like languages, with the only difference that it also caches the length
// of the list in each node.
// A generic stack implemented with a singly-linked list, which results in an
// O(1) copy operation. It can be used to model immutable lists like those in
// functional languages. Compared to typical functional lists, this also caches
// the length of the list in each node.
// Note: The underlying implementation is mutable, so if you want to use this as
// an immutable list, make sure to create a copy by passing it by value and
// operate on the copy.
// TODO(turbofan): Use this implementation also for RedundancyElimination.
template <class A>
class FunctionalList {
......
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