[turboshaft] Use BranchHints instead of per-block deferred bit

Computing on-the-fly whether blocks are deferred is a bit tricky,
because some optimizations (BranchElimination and
MachineOperationReducer in particular) can remove branches, resulting
in deferred blocks with non-deferred predecessors and successors.
We are thus opting for the simpler solution: having branch hints on
branches, so that eliminating branches of cloning blocks doesn't have
an impact on deferredness.


Fixed: chromium:1393733
Bug: v8:12783
Change-Id: Ic886f48a33f25f1a3068b9bf684fae43e1f53b6d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4061266
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Darius Mercadier <dmercadier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84558}
This commit is contained in:
Darius M 2022-11-29 18:27:16 +01:00 committed by V8 LUCI CQ
parent 8d43b96711
commit 17065e3e7c
12 changed files with 94 additions and 75 deletions

View File

@ -16,6 +16,7 @@
#include "src/base/small-vector.h"
#include "src/base/template-utils.h"
#include "src/codegen/reloc-info.h"
#include "src/compiler/common-operator.h"
#include "src/compiler/turboshaft/graph.h"
#include "src/compiler/turboshaft/operation-matching.h"
#include "src/compiler/turboshaft/operations.h"
@ -139,14 +140,16 @@ class ReducerBase : public ReducerBaseForwarder<Next> {
return new_opindex;
}
OpIndex ReduceBranch(OpIndex condition, Block* if_true, Block* if_false) {
OpIndex ReduceBranch(OpIndex condition, Block* if_true, Block* if_false,
BranchHint hint) {
// There should never be a good reason to generate a Branch where both the
// {if_true} and {if_false} are the same Block. If we ever decide to lift
// this condition, then AddPredecessor and SplitEdge should be updated
// accordingly.
DCHECK_NE(if_true, if_false);
Block* saved_current_block = Asm().current_block();
OpIndex new_opindex = Base::ReduceBranch(condition, if_true, if_false);
OpIndex new_opindex =
Base::ReduceBranch(condition, if_true, if_false, hint);
Asm().AddPredecessor(saved_current_block, if_true, true);
Asm().AddPredecessor(saved_current_block, if_false, true);
return new_opindex;
@ -169,7 +172,7 @@ class ReducerBase : public ReducerBaseForwarder<Next> {
}
OpIndex ReduceSwitch(OpIndex input, base::Vector<const SwitchOp::Case> cases,
Block* default_case) {
Block* default_case, BranchHint default_hint) {
#ifdef DEBUG
// Making sure that all cases and {default_case} are different. If we ever
// decide to lift this condition, then AddPredecessor and SplitEdge should
@ -182,7 +185,8 @@ class ReducerBase : public ReducerBaseForwarder<Next> {
}
#endif
Block* saved_current_block = Asm().current_block();
OpIndex new_opindex = Base::ReduceSwitch(input, cases, default_case);
OpIndex new_opindex =
Base::ReduceSwitch(input, cases, default_case, default_hint);
for (SwitchOp::Case c : cases) {
Asm().AddPredecessor(saved_current_block, c.destination, true);
}
@ -846,8 +850,9 @@ class AssemblerOpInterface {
}
void Goto(Block* destination) { stack().ReduceGoto(destination); }
void Branch(OpIndex condition, Block* if_true, Block* if_false) {
stack().ReduceBranch(condition, if_true, if_false);
void Branch(OpIndex condition, Block* if_true, Block* if_false,
BranchHint hint = BranchHint::kNone) {
stack().ReduceBranch(condition, if_true, if_false, hint);
}
OpIndex Select(OpIndex cond, OpIndex vtrue, OpIndex vfalse,
RegisterRepresentation rep, BranchHint hint,
@ -855,8 +860,9 @@ class AssemblerOpInterface {
return stack().ReduceSelect(cond, vtrue, vfalse, rep, hint, implem);
}
void Switch(OpIndex input, base::Vector<const SwitchOp::Case> cases,
Block* default_case) {
stack().ReduceSwitch(input, cases, default_case);
Block* default_case,
BranchHint default_hint = BranchHint::kNone) {
stack().ReduceSwitch(input, cases, default_case, default_hint);
}
void Unreachable() { stack().ReduceUnreachable(); }
@ -951,15 +957,17 @@ class AssemblerOpInterface {
OpIndex LoadException() { return stack().ReduceLoadException(); }
// Return `true` if the control flow after the conditional jump is reachable.
V8_WARN_UNUSED_RESULT bool GotoIf(OpIndex condition, Block* if_true) {
V8_WARN_UNUSED_RESULT bool GotoIf(OpIndex condition, Block* if_true,
BranchHint hint = BranchHint::kNone) {
Block* if_false = stack().NewBlock();
stack().Branch(condition, if_true, if_false);
stack().Branch(condition, if_true, if_false, hint);
return stack().Bind(if_false);
}
// Return `true` if the control flow after the conditional jump is reachable.
V8_WARN_UNUSED_RESULT bool GotoIfNot(OpIndex condition, Block* if_false) {
V8_WARN_UNUSED_RESULT bool GotoIfNot(OpIndex condition, Block* if_false,
BranchHint hint = BranchHint::kNone) {
Block* if_true = stack().NewBlock();
stack().Branch(condition, if_true, if_false);
stack().Branch(condition, if_true, if_false, hint);
return stack().Bind(if_true);
}

View File

@ -174,9 +174,10 @@ class BranchEliminationReducer : public Next {
}
}
OpIndex ReduceBranch(OpIndex cond, Block* if_true, Block* if_false) {
OpIndex ReduceBranch(OpIndex cond, Block* if_true, Block* if_false,
BranchHint hint) {
LABEL_BLOCK(no_change) {
return Next::ReduceBranch(cond, if_true, if_false);
return Next::ReduceBranch(cond, if_true, if_false, hint);
}
if (ShouldSkipOptimizationStep()) goto no_change;

View File

@ -175,7 +175,6 @@ base::Optional<BailoutReason> GraphBuilder::Run() {
for (BasicBlock* block : *schedule.rpo_order()) {
Block* target_block = Map(block);
if (!assembler.Bind(target_block)) continue;
target_block->SetDeferred(block->deferred());
// Since we visit blocks in rpo-order, the new block predecessors are sorted
// in rpo order too. However, the input schedule does not order
@ -754,7 +753,7 @@ OpIndex GraphBuilder::Process(
case IrOpcode::kBranch:
DCHECK_EQ(block->SuccessorCount(), 2);
assembler.Branch(Map(node->InputAt(0)), Map(block->SuccessorAt(0)),
Map(block->SuccessorAt(1)));
Map(block->SuccessorAt(1)), BranchHintOf(node->op()));
return OpIndex::Invalid();
case IrOpcode::kSwitch: {
@ -765,11 +764,11 @@ OpIndex GraphBuilder::Process(
for (size_t i = 0; i < case_count; ++i) {
BasicBlock* branch = block->SuccessorAt(i);
const IfValueParameters& p = IfValueParametersOf(branch->front()->op());
cases.emplace_back(p.value(), Map(branch));
cases.emplace_back(p.value(), Map(branch), p.hint());
}
assembler.Switch(Map(node->InputAt(0)),
graph_zone->CloneVector(base::VectorOf(cases)),
Map(default_branch));
assembler.Switch(
Map(node->InputAt(0)), graph_zone->CloneVector(base::VectorOf(cases)),
Map(default_branch), BranchHintOf(default_branch->front()->op()));
return OpIndex::Invalid();
}

View File

@ -85,7 +85,6 @@ void JSONTurboshaftGraphWriter::PrintBlocks() {
first_block = false;
os_ << "{\"id\":" << block.index().id() << ",";
os_ << "\"type\":\"" << block.kind() << "\",";
os_ << "\"deferred\":" << std::boolalpha << block.IsDeferred() << ",";
os_ << "\"predecessors\":[";
bool first_predecessor = true;
for (const Block* pred : block.Predecessors()) {

View File

@ -64,7 +64,6 @@ void Block::PrintDominatorTree(std::vector<const char*> tree_symbols,
std::ostream& operator<<(std::ostream& os, PrintAsBlockHeader block_header) {
const Block& block = block_header.block;
os << block.kind() << " " << block_header.block_id;
if (block.IsDeferred()) os << " (deferred)";
if (!block.Predecessors().empty()) {
os << " <- ";
bool first = true;

View File

@ -281,9 +281,6 @@ class Block : public RandomAccessStackDominatorNode<Block> {
BlockIndex index() const { return index_; }
bool IsDeferred() const { return deferred_; }
void SetDeferred(bool deferred) { deferred_ = deferred; }
bool Contains(OpIndex op_idx) const {
return begin_ <= op_idx && op_idx < end_;
}
@ -403,7 +400,6 @@ class Block : public RandomAccessStackDominatorNode<Block> {
friend class Assembler;
Kind kind_;
bool deferred_ = false;
OpIndex begin_ = OpIndex::Invalid();
OpIndex end_ = OpIndex::Invalid();
BlockIndex index_ = BlockIndex::Invalid();
@ -526,17 +522,7 @@ class Graph {
V8_INLINE bool Add(Block* block) {
DCHECK_EQ(block->graph_generation_, generation_);
if (!bound_blocks_.empty() && !block->HasPredecessors()) return false;
if (!block->IsDeferred()) {
bool deferred = true;
for (Block* pred = block->last_predecessor_; pred != nullptr;
pred = pred->neighboring_predecessor_) {
if (!pred->IsDeferred()) {
deferred = false;
break;
}
}
block->SetDeferred(deferred);
}
DCHECK(!block->begin_.valid());
block->begin_ = next_operation_index();
DCHECK_EQ(block->index_, BlockIndex::Invalid());
@ -544,6 +530,7 @@ class Graph {
bound_blocks_.push_back(block);
uint32_t depth = block->ComputeDominator();
dominator_tree_depth_ = std::max<uint32_t>(dominator_tree_depth_, depth);
return true;
}

View File

@ -534,12 +534,11 @@ class MachineOptimizationReducer : public Next {
// lhs ** 0.5 ==> sqrt(lhs)
// (unless if lhs is -infinity)
Block* if_neg_infinity = Asm().NewBlock();
if_neg_infinity->SetDeferred(true);
Block* otherwise = Asm().NewBlock();
Block* merge = Asm().NewBlock();
Asm().Branch(Asm().FloatLessThanOrEqual(
lhs, Asm().FloatConstant(-V8_INFINITY, rep), rep),
if_neg_infinity, otherwise);
if_neg_infinity, otherwise, BranchHint::kFalse);
// TODO(dmercadier,tebbi): once the VariableAssembler has landed, and
// use only one AutoVariable both both {infty} and {sqrt} to avoid the
@ -1495,9 +1494,10 @@ class MachineOptimizationReducer : public Next {
return Next::ReduceShift(left, right, kind, rep);
}
OpIndex ReduceBranch(OpIndex condition, Block* if_true, Block* if_false) {
OpIndex ReduceBranch(OpIndex condition, Block* if_true, Block* if_false,
BranchHint hint) {
if (ShouldSkipOptimizationStep()) {
return Next::ReduceBranch(condition, if_true, if_false);
return Next::ReduceBranch(condition, if_true, if_false, hint);
}
if (base::Optional<bool> decision = DecideBranchCondition(condition)) {
Asm().Goto(*decision ? if_true : if_false);
@ -1506,10 +1506,14 @@ class MachineOptimizationReducer : public Next {
bool negated = false;
if (base::Optional<OpIndex> new_condition =
ReduceBranchCondition(condition, &negated)) {
if (negated) std::swap(if_true, if_false);
return Asm().ReduceBranch(new_condition.value(), if_true, if_false);
if (negated) {
std::swap(if_true, if_false);
hint = NegateBranchHint(hint);
}
return Asm().ReduceBranch(new_condition.value(), if_true, if_false, hint);
} else {
return Next::ReduceBranch(condition, if_true, if_false);
return Next::ReduceBranch(condition, if_true, if_false, hint);
}
}
@ -1575,9 +1579,9 @@ class MachineOptimizationReducer : public Next {
}
OpIndex ReduceSwitch(OpIndex input, base::Vector<const SwitchOp::Case> cases,
Block* default_case) {
Block* default_case, BranchHint default_hint) {
LABEL_BLOCK(no_change) {
return Next::ReduceSwitch(input, cases, default_case);
return Next::ReduceSwitch(input, cases, default_case, default_hint);
}
if (ShouldSkipOptimizationStep()) goto no_change;
if (int32_t value; Asm().MatchWord32Constant(input, &value)) {

View File

@ -165,7 +165,6 @@ class MemoryOptimizationReducer : public Next {
}
Block* call_runtime = Asm().NewBlock();
call_runtime->SetDeferred(true);
Block* done = Asm().NewBlock();
OpIndex limit_address = Asm().ExternalConstant(
@ -187,13 +186,13 @@ class MemoryOptimizationReducer : public Next {
reachable = Asm().GotoIfNot(
Asm().UintPtrLessThan(
size, Asm().IntPtrConstant(kMaxRegularHeapObjectSize)),
call_runtime);
call_runtime, BranchHint::kTrue);
}
if (reachable) {
Asm().Branch(
Asm().UintPtrLessThan(
Asm().PointerAdd(Asm().Get(top), reservation_size), limit),
done, call_runtime);
done, call_runtime, BranchHint::kTrue);
}
// Call the runtime if bump pointer area exhausted.

View File

@ -1970,41 +1970,49 @@ struct GotoOp : FixedArityOperationT<0, GotoOp> {
struct BranchOp : FixedArityOperationT<1, BranchOp> {
Block* if_true;
Block* if_false;
BranchHint hint;
static constexpr OpProperties properties = OpProperties::BlockTerminator();
base::Vector<const RegisterRepresentation> outputs_rep() const { return {}; }
OpIndex condition() const { return input(0); }
BranchOp(OpIndex condition, Block* if_true, Block* if_false)
: Base(condition), if_true(if_true), if_false(if_false) {}
auto options() const { return std::tuple{if_true, if_false}; }
BranchOp(OpIndex condition, Block* if_true, Block* if_false, BranchHint hint)
: Base(condition), if_true(if_true), if_false(if_false), hint(hint) {}
auto options() const { return std::tuple{if_true, if_false, hint}; }
};
struct SwitchOp : FixedArityOperationT<1, SwitchOp> {
struct Case {
int32_t value;
Block* destination;
BranchHint hint;
Case(int32_t value, Block* destination)
: value(value), destination(destination) {}
Case(int32_t value, Block* destination, BranchHint hint)
: value(value), destination(destination), hint(hint) {}
bool operator==(const Case& other) const {
return value == other.value && destination == other.destination;
return value == other.value && destination == other.destination &&
hint == other.hint;
}
};
base::Vector<const Case> cases;
Block* default_case;
BranchHint default_hint;
static constexpr OpProperties properties = OpProperties::BlockTerminator();
base::Vector<const RegisterRepresentation> outputs_rep() const { return {}; }
OpIndex input() const { return Base::input(0); }
SwitchOp(OpIndex input, base::Vector<const Case> cases, Block* default_case)
: Base(input), cases(cases), default_case(default_case) {}
SwitchOp(OpIndex input, base::Vector<const Case> cases, Block* default_case,
BranchHint default_hint)
: Base(input),
cases(cases),
default_case(default_case),
default_hint(default_hint) {}
void PrintOptions(std::ostream& os) const;
auto options() const { return std::tuple{cases, default_case}; }
auto options() const { return std::tuple{cases, default_case, default_hint}; }
};
template <>

View File

@ -279,7 +279,8 @@ class GraphVisitor {
assembler().output_graph().next_block_index()}
<< "\n";
}
if (!assembler().Bind(MapToNewGraph(input_block->index()), input_block)) {
Block* new_block = MapToNewGraph(input_block->index());
if (!assembler().Bind(new_block, input_block)) {
if constexpr (trace_reduction) TraceBlockUnreachable();
// If we eliminate a loop backedge, we need to turn the loop into a
// single-predecessor merge block.
@ -296,7 +297,6 @@ class GraphVisitor {
}
return;
}
assembler().current_block()->SetDeferred(input_block->IsDeferred());
for (OpIndex index : input_graph().OperationIndices(*input_block)) {
if (!VisitOp<trace_reduction>(index, input_block)) break;
}
@ -403,17 +403,18 @@ class GraphVisitor {
Block* if_true = MapToNewGraph(op.if_true->index());
Block* if_false = MapToNewGraph(op.if_false->index());
return assembler().ReduceBranch(MapToNewGraph(op.condition()), if_true,
if_false);
if_false, op.hint);
}
OpIndex VisitSwitch(const SwitchOp& op) {
base::SmallVector<SwitchOp::Case, 16> cases;
for (SwitchOp::Case c : op.cases) {
cases.emplace_back(c.value, MapToNewGraph(c.destination->index()));
cases.emplace_back(c.value, MapToNewGraph(c.destination->index()),
c.hint);
}
return assembler().ReduceSwitch(
MapToNewGraph(op.input()),
graph_zone()->CloneVector(base::VectorOf(cases)),
MapToNewGraph(op.default_case->index()));
MapToNewGraph(op.default_case->index()), op.default_hint);
}
OpIndex VisitPhi(const PhiOp& op) {
if (visiting_cloned_block_) {

View File

@ -135,7 +135,6 @@ RecreateScheduleResult ScheduleBuilder::Run() {
for (const Block& block : input_graph.blocks()) {
current_input_block = &block;
current_block = GetBlock(block);
current_block->set_deferred(current_input_block->IsDeferred());
for (OpIndex op : input_graph.OperationIndices(block)) {
DCHECK_NOT_NULL(current_block);
ProcessOperation(input_graph.Get(op));
@ -148,6 +147,10 @@ RecreateScheduleResult ScheduleBuilder::Run() {
DCHECK(schedule->rpo_order()->empty());
Scheduler::ComputeSpecialRPO(phase_zone, schedule);
// Note that Scheduler::GenerateDominatorTree also infers which blocks are
// deferred, so we only need to set branch targets as deferred based on the
// hints, and we let Scheduler::GenerateDominatorTree propagate this
// information to other blocks.
Scheduler::GenerateDominatorTree(schedule);
return {tf_graph, schedule};
}
@ -1289,13 +1292,22 @@ Node* ScheduleBuilder::ProcessOperation(const ReturnOp& op) {
return nullptr;
}
Node* ScheduleBuilder::ProcessOperation(const BranchOp& op) {
Node* branch =
MakeNode(common.Branch(BranchHint::kNone), {GetNode(op.condition())});
Node* branch = MakeNode(common.Branch(op.hint), {GetNode(op.condition())});
BasicBlock* true_block = GetBlock(*op.if_true);
BasicBlock* false_block = GetBlock(*op.if_false);
schedule->AddBranch(current_block, branch, true_block, false_block);
schedule->AddNode(true_block, MakeNode(common.IfTrue(), {branch}));
schedule->AddNode(false_block, MakeNode(common.IfFalse(), {branch}));
switch (op.hint) {
case BranchHint::kNone:
break;
case BranchHint::kTrue:
false_block->set_deferred(true);
break;
case BranchHint::kFalse:
true_block->set_deferred(false);
break;
}
current_block = nullptr;
return nullptr;
}
@ -1308,12 +1320,20 @@ Node* ScheduleBuilder::ProcessOperation(const SwitchOp& op) {
for (SwitchOp::Case c : op.cases) {
BasicBlock* case_block = GetBlock(*c.destination);
successors.push_back(case_block);
Node* case_node = MakeNode(common.IfValue(c.value), {switch_node});
Node* case_node =
MakeNode(common.IfValue(c.value, 0, c.hint), {switch_node});
schedule->AddNode(case_block, case_node);
if (c.hint == BranchHint::kFalse) {
case_block->set_deferred(true);
}
}
BasicBlock* default_block = GetBlock(*op.default_case);
successors.push_back(default_block);
schedule->AddNode(default_block, MakeNode(common.IfDefault(), {switch_node}));
schedule->AddNode(default_block,
MakeNode(common.IfDefault(op.default_hint), {switch_node}));
if (op.default_hint == BranchHint::kFalse) {
default_block->set_deferred(true);
}
schedule->AddSwitch(current_block, switch_node, successors.data(),
successors.size());

View File

@ -52,13 +52,7 @@ class SelectLoweringReducer : public Next {
Block* false_block = Asm().NewBlock();
Block* merge_block = Asm().NewBlock();
if (hint == BranchHint::kTrue) {
false_block->SetDeferred(true);
} else if (hint == BranchHint::kFalse) {
true_block->SetDeferred(true);
}
Asm().Branch(cond, true_block, false_block);
Asm().Branch(cond, true_block, false_block, hint);
// Note that it's possible that other reducers of the stack optimizes the
// Branch that we just introduced into a Goto (if its condition is already