From ef4de566959e25fd76d578fbff6a33e9dec36da0 Mon Sep 17 00:00:00 2001 From: Manos Koukoutos Date: Wed, 26 May 2021 16:13:26 +0000 Subject: [PATCH] [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 Reviewed-by: Georg Neis Cr-Commit-Position: refs/heads/master@{#74793} --- src/compiler/branch-elimination.cc | 64 +++++++++++++++++++++--------- src/compiler/branch-elimination.h | 24 ++++++++--- src/compiler/functional-list.h | 11 +++-- 3 files changed, 70 insertions(+), 29 deletions(-) diff --git a/src/compiler/branch-elimination.cc b/src/compiler/branch-elimination.cc index 9a16e6d148..8059faa176 100644 --- a/src/compiler/branch-elimination.cc +++ b/src/compiler/branch-elimination.cc @@ -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 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 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; diff --git a/src/compiler/branch-elimination.h b/src/compiler/branch-elimination.h index eb6f1f159d..6bc45a020d 100644 --- a/src/compiler/branch-elimination.h +++ b/src/compiler/branch-elimination.h @@ -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 { + // 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> { 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::PushFront; + using FunctionalList>::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_; } diff --git a/src/compiler/functional-list.h b/src/compiler/functional-list.h index 465bdf133b..b9968524e3 100644 --- a/src/compiler/functional-list.h +++ b/src/compiler/functional-list.h @@ -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 FunctionalList {