Commit 0719ace6 authored by Darius M's avatar Darius M Committed by V8 LUCI CQ

Reland^2 [compiler] Simplify "==0" branches in MachineOperatorReducer

This is a reland of 6b690a6b.

The previous version of this CL was a bit too aggressive in the
duplication of branch conditions. This caused an increase in
register pressure in some cases, thus reducing performance.

In fact, duplicating branch conditions that require an "== 0" to be
added provides no benefits. We are thus now a bit less aggressive, and
only duplicate comparisons.

Original change's description:
> Reland [compiler] Simplify "==0" branches in MachineOperatorReducer
>
> This is a reland of 48b443f6.
>
> While fixing the initial CL, we stumbled upon a few bugs that
> we had to fix:
>
>  - CommonOperatorReducer and SimplifiedOperatorReducer were applied
>    before and after SimplifiedLowering, but always assumed that it
>    was before SimplifiedLowering, and thus had the wrong semantics
>    for branches in some cases. They now have an added parameter to
>    know which semantics of branch they should use.
>
>  - The lowering of StaticAssert was wrong and could leave kHeapConstant
>    in the assert (instead of machine Booleans).
>
> Original change's description:
> > [compiler] Simplify "==0" branches in MachineOperatorReducer
> >
> > Bug: v8:12484
> > Change-Id: I0667c7464c0dd71338bc199a24a69248a7a0a525
> > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3497303
> > Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
> > Owners-Override: Tobias Tebbi <tebbi@chromium.org>
> > Commit-Queue: Darius Mercadier <dmercadier@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#79379}
>
> Bug: v8:12484
> Change-Id: Ibbf5df96fce5ccb04868dc517539479bf69f5703
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3516869
> Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
> Commit-Queue: Darius Mercadier <dmercadier@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#79528}

Bug: v8:12484
Change-Id: I31f575a59811a83c7c1acb4c14bf5ded63a8f536
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3540102Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
Commit-Queue: Darius Mercadier <dmercadier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79560}
parent 7ecaee95
...@@ -2610,6 +2610,8 @@ filegroup( ...@@ -2610,6 +2610,8 @@ filegroup(
"src/compiler/backend/unwinding-info-writer.h", "src/compiler/backend/unwinding-info-writer.h",
"src/compiler/basic-block-instrumentor.cc", "src/compiler/basic-block-instrumentor.cc",
"src/compiler/basic-block-instrumentor.h", "src/compiler/basic-block-instrumentor.h",
"src/compiler/branch-condition-duplicator.cc",
"src/compiler/branch-condition-duplicator.h",
"src/compiler/branch-elimination.cc", "src/compiler/branch-elimination.cc",
"src/compiler/branch-elimination.h", "src/compiler/branch-elimination.h",
"src/compiler/bytecode-analysis.cc", "src/compiler/bytecode-analysis.cc",
......
...@@ -2778,6 +2778,7 @@ v8_header_set("v8_internal_headers") { ...@@ -2778,6 +2778,7 @@ v8_header_set("v8_internal_headers") {
"src/compiler/backend/spill-placer.h", "src/compiler/backend/spill-placer.h",
"src/compiler/backend/unwinding-info-writer.h", "src/compiler/backend/unwinding-info-writer.h",
"src/compiler/basic-block-instrumentor.h", "src/compiler/basic-block-instrumentor.h",
"src/compiler/branch-condition-duplicator.h",
"src/compiler/branch-elimination.h", "src/compiler/branch-elimination.h",
"src/compiler/bytecode-analysis.h", "src/compiler/bytecode-analysis.h",
"src/compiler/bytecode-graph-builder.h", "src/compiler/bytecode-graph-builder.h",
...@@ -3890,6 +3891,7 @@ v8_compiler_sources = [ ...@@ -3890,6 +3891,7 @@ v8_compiler_sources = [
"src/compiler/backend/register-allocator.cc", "src/compiler/backend/register-allocator.cc",
"src/compiler/backend/spill-placer.cc", "src/compiler/backend/spill-placer.cc",
"src/compiler/basic-block-instrumentor.cc", "src/compiler/basic-block-instrumentor.cc",
"src/compiler/branch-condition-duplicator.cc",
"src/compiler/branch-elimination.cc", "src/compiler/branch-elimination.cc",
"src/compiler/bytecode-analysis.cc", "src/compiler/bytecode-analysis.cc",
"src/compiler/bytecode-graph-builder.cc", "src/compiler/bytecode-graph-builder.cc",
......
// Copyright 2022 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "src/compiler/branch-condition-duplicator.h"
#include "src/compiler/backend/instruction-codes.h"
#include "src/compiler/graph.h"
#include "src/compiler/node-properties.h"
#include "src/compiler/opcodes.h"
namespace v8 {
namespace internal {
namespace compiler {
namespace {
bool IsBranch(Node* node) { return node->opcode() == IrOpcode::kBranch; }
bool CanDuplicate(Node* node) {
// We only allow duplication of comparisons. Duplicating other nodes makes
// little sense, because a "== 0" need to be inserted in branches anyways
// (except for some binops, but duplicating binops might increase the live
// range of the operands, thus increasing register pressure).
switch (node->opcode()) {
#define BRANCH_CASE(op) \
case IrOpcode::k##op: \
break;
MACHINE_COMPARE_BINOP_LIST(BRANCH_CASE)
default:
return false;
}
// We do not duplicate nodes if all their inputs are used a single time,
// because this would keep those inputs alive, thus increasing register
// pressure.
int all_inputs_have_only_a_single_use = true;
for (Node* input : node->inputs()) {
if (input->UseCount() > 1) {
all_inputs_have_only_a_single_use = false;
}
}
if (all_inputs_have_only_a_single_use) {
return false;
}
return true;
}
} // namespace
Node* BranchConditionDuplicator::DuplicateNode(Node* node) {
return graph_->CloneNode(node);
}
void BranchConditionDuplicator::DuplicateConditionIfNeeded(Node* node) {
if (!IsBranch(node)) return;
Node* condNode = node->InputAt(0);
if (condNode->UseCount() > 1 && CanDuplicate(condNode)) {
node->ReplaceInput(0, DuplicateNode(condNode));
}
}
void BranchConditionDuplicator::Enqueue(Node* node) {
if (seen_.Get(node)) return;
seen_.Set(node, true);
to_visit_.push(node);
}
void BranchConditionDuplicator::VisitNode(Node* node) {
DuplicateConditionIfNeeded(node);
for (int i = 0; i < node->op()->ControlInputCount(); i++) {
Enqueue(NodeProperties::GetControlInput(node, i));
}
}
void BranchConditionDuplicator::ProcessGraph() {
Enqueue(graph_->end());
while (!to_visit_.empty()) {
Node* node = to_visit_.front();
to_visit_.pop();
VisitNode(node);
}
}
BranchConditionDuplicator::BranchConditionDuplicator(Zone* zone, Graph* graph)
: graph_(graph), to_visit_(zone), seen_(graph, 2) {}
void BranchConditionDuplicator::Reduce() { ProcessGraph(); }
} // namespace compiler
} // namespace internal
} // namespace v8
// Copyright 2022 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef V8_COMPILER_BRANCH_CONDITION_DUPLICATOR_H_
#define V8_COMPILER_BRANCH_CONDITION_DUPLICATOR_H_
#include "src/base/macros.h"
#include "src/compiler/node-marker.h"
#include "src/compiler/node.h"
#include "src/zone/zone.h"
namespace v8 {
namespace internal {
namespace compiler {
// Forward declare.
class Graph;
// BranchConditionDuplicator makes sure that the condition nodes of branches are
// used only once. When it finds a branch node whose condition has multiples
// uses, this condition is duplicated.
//
// Doing this enables the InstructionSelector to generate more efficient code
// for branches. For instance, consider this code:
//
// if (a + b == 0) { /* some code */ }
// if (a + b == 0) { /* more code */ }
//
// Then the generated code will be something like (using registers "ra" for "a"
// and "rb" for "b", and "rt" a temporary register):
//
// add ra, rb ; a + b
// cmp ra, 0 ; (a + b) == 0
// sete rt ; rt = (a + b) == 0
// cmp rt, 0 ; rt == 0
// jz
// ...
// cmp rt, 0 ; rt == 0
// jz
//
// As you can see, TurboFan materialized the == bit into a temporary register.
// However, since the "add" instruction sets the ZF flag (on x64), it can be
// used to determine wether the jump should be taken or not. The code we'd like
// to generate instead if thus:
//
// add ra, rb
// jnz
// ...
// add ra, rb
// jnz
//
// However, this requires to generate twice the instruction "add ra, rb". Due to
// how virtual registers are assigned in TurboFan (there is a map from node ID
// to virtual registers), both "add" instructions will use the same virtual
// register as output, which will break SSA.
//
// In order to overcome this issue, BranchConditionDuplicator duplicates branch
// conditions that are used more than once, so that they can be generated right
// before each branch without worrying about breaking SSA.
class V8_EXPORT_PRIVATE BranchConditionDuplicator final {
public:
BranchConditionDuplicator(Zone* zone, Graph* graph);
~BranchConditionDuplicator() = default;
void Reduce();
Node* DuplicateNode(Node* node);
void DuplicateConditionIfNeeded(Node* node);
void Enqueue(Node* node);
void VisitNode(Node* node);
void ProcessGraph();
private:
Graph* const graph_;
ZoneQueue<Node*> to_visit_;
NodeMarker<bool> seen_;
};
} // namespace compiler
} // namespace internal
} // namespace v8
#endif // V8_COMPILER_BRANCH_CONDITION_DUPLICATOR_H_
...@@ -13,45 +13,26 @@ ...@@ -13,45 +13,26 @@
#include "src/compiler/node-matchers.h" #include "src/compiler/node-matchers.h"
#include "src/compiler/node-properties.h" #include "src/compiler/node-properties.h"
#include "src/compiler/node.h" #include "src/compiler/node.h"
#include "src/compiler/opcodes.h"
namespace v8 { namespace v8 {
namespace internal { namespace internal {
namespace compiler { namespace compiler {
namespace {
Decision DecideCondition(JSHeapBroker* broker, Node* const cond) {
Node* unwrapped = SkipValueIdentities(cond);
switch (unwrapped->opcode()) {
case IrOpcode::kInt32Constant: {
Int32Matcher m(unwrapped);
return m.ResolvedValue() ? Decision::kTrue : Decision::kFalse;
}
case IrOpcode::kHeapConstant: {
HeapObjectMatcher m(unwrapped);
base::Optional<bool> maybe_result = m.Ref(broker).TryGetBooleanValue();
if (!maybe_result.has_value()) return Decision::kUnknown;
return *maybe_result ? Decision::kTrue : Decision::kFalse;
}
default:
return Decision::kUnknown;
}
}
} // namespace
CommonOperatorReducer::CommonOperatorReducer(Editor* editor, Graph* graph, CommonOperatorReducer::CommonOperatorReducer(Editor* editor, Graph* graph,
JSHeapBroker* broker, JSHeapBroker* broker,
CommonOperatorBuilder* common, CommonOperatorBuilder* common,
MachineOperatorBuilder* machine, MachineOperatorBuilder* machine,
Zone* temp_zone) Zone* temp_zone,
BranchSemantics branch_semantics)
: AdvancedReducer(editor), : AdvancedReducer(editor),
graph_(graph), graph_(graph),
broker_(broker), broker_(broker),
common_(common), common_(common),
machine_(machine), machine_(machine),
dead_(graph->NewNode(common->Dead())), dead_(graph->NewNode(common->Dead())),
zone_(temp_zone) { zone_(temp_zone),
branch_semantics_(branch_semantics) {
NodeProperties::SetType(dead_, Type::None()); NodeProperties::SetType(dead_, Type::None());
} }
...@@ -86,6 +67,27 @@ Reduction CommonOperatorReducer::Reduce(Node* node) { ...@@ -86,6 +67,27 @@ Reduction CommonOperatorReducer::Reduce(Node* node) {
return NoChange(); return NoChange();
} }
Decision CommonOperatorReducer::DecideCondition(Node* const cond) {
Node* unwrapped = SkipValueIdentities(cond);
switch (unwrapped->opcode()) {
case IrOpcode::kInt32Constant: {
DCHECK_EQ(branch_semantics_, BranchSemantics::kMachine);
Int32Matcher m(unwrapped);
return m.ResolvedValue() ? Decision::kTrue : Decision::kFalse;
}
case IrOpcode::kHeapConstant: {
if (branch_semantics_ == BranchSemantics::kMachine) {
return Decision::kTrue;
}
HeapObjectMatcher m(unwrapped);
base::Optional<bool> maybe_result = m.Ref(broker_).TryGetBooleanValue();
if (!maybe_result.has_value()) return Decision::kUnknown;
return *maybe_result ? Decision::kTrue : Decision::kFalse;
}
default:
return Decision::kUnknown;
}
}
Reduction CommonOperatorReducer::ReduceBranch(Node* node) { Reduction CommonOperatorReducer::ReduceBranch(Node* node) {
DCHECK_EQ(IrOpcode::kBranch, node->opcode()); DCHECK_EQ(IrOpcode::kBranch, node->opcode());
...@@ -97,8 +99,8 @@ Reduction CommonOperatorReducer::ReduceBranch(Node* node) { ...@@ -97,8 +99,8 @@ Reduction CommonOperatorReducer::ReduceBranch(Node* node) {
// not (i.e. true being returned in the false case and vice versa). // not (i.e. true being returned in the false case and vice versa).
if (cond->opcode() == IrOpcode::kBooleanNot || if (cond->opcode() == IrOpcode::kBooleanNot ||
(cond->opcode() == IrOpcode::kSelect && (cond->opcode() == IrOpcode::kSelect &&
DecideCondition(broker(), cond->InputAt(1)) == Decision::kFalse && DecideCondition(cond->InputAt(1)) == Decision::kFalse &&
DecideCondition(broker(), cond->InputAt(2)) == Decision::kTrue)) { DecideCondition(cond->InputAt(2)) == Decision::kTrue)) {
for (Node* const use : node->uses()) { for (Node* const use : node->uses()) {
switch (use->opcode()) { switch (use->opcode()) {
case IrOpcode::kIfTrue: case IrOpcode::kIfTrue:
...@@ -120,7 +122,7 @@ Reduction CommonOperatorReducer::ReduceBranch(Node* node) { ...@@ -120,7 +122,7 @@ Reduction CommonOperatorReducer::ReduceBranch(Node* node) {
node, common()->Branch(NegateBranchHint(BranchHintOf(node->op())))); node, common()->Branch(NegateBranchHint(BranchHintOf(node->op()))));
return Changed(node); return Changed(node);
} }
Decision const decision = DecideCondition(broker(), cond); Decision const decision = DecideCondition(cond);
if (decision == Decision::kUnknown) return NoChange(); if (decision == Decision::kUnknown) return NoChange();
Node* const control = node->InputAt(1); Node* const control = node->InputAt(1);
for (Node* const use : node->uses()) { for (Node* const use : node->uses()) {
...@@ -160,7 +162,7 @@ Reduction CommonOperatorReducer::ReduceDeoptimizeConditional(Node* node) { ...@@ -160,7 +162,7 @@ Reduction CommonOperatorReducer::ReduceDeoptimizeConditional(Node* node) {
: common()->DeoptimizeUnless(p.kind(), p.reason(), p.feedback())); : common()->DeoptimizeUnless(p.kind(), p.reason(), p.feedback()));
return Changed(node); return Changed(node);
} }
Decision const decision = DecideCondition(broker(), condition); Decision const decision = DecideCondition(condition);
if (decision == Decision::kUnknown) return NoChange(); if (decision == Decision::kUnknown) return NoChange();
if (condition_is_true == (decision == Decision::kTrue)) { if (condition_is_true == (decision == Decision::kTrue)) {
ReplaceWithValue(node, dead(), effect, control); ReplaceWithValue(node, dead(), effect, control);
...@@ -392,7 +394,7 @@ Reduction CommonOperatorReducer::ReduceSelect(Node* node) { ...@@ -392,7 +394,7 @@ Reduction CommonOperatorReducer::ReduceSelect(Node* node) {
Node* const vtrue = node->InputAt(1); Node* const vtrue = node->InputAt(1);
Node* const vfalse = node->InputAt(2); Node* const vfalse = node->InputAt(2);
if (vtrue == vfalse) return Replace(vtrue); if (vtrue == vfalse) return Replace(vtrue);
switch (DecideCondition(broker(), cond)) { switch (DecideCondition(cond)) {
case Decision::kTrue: case Decision::kTrue:
return Replace(vtrue); return Replace(vtrue);
case Decision::kFalse: case Decision::kFalse:
...@@ -469,7 +471,7 @@ Reduction CommonOperatorReducer::ReduceSwitch(Node* node) { ...@@ -469,7 +471,7 @@ Reduction CommonOperatorReducer::ReduceSwitch(Node* node) {
Reduction CommonOperatorReducer::ReduceStaticAssert(Node* node) { Reduction CommonOperatorReducer::ReduceStaticAssert(Node* node) {
DCHECK_EQ(IrOpcode::kStaticAssert, node->opcode()); DCHECK_EQ(IrOpcode::kStaticAssert, node->opcode());
Node* const cond = node->InputAt(0); Node* const cond = node->InputAt(0);
Decision decision = DecideCondition(broker(), cond); Decision decision = DecideCondition(cond);
if (decision == Decision::kTrue) { if (decision == Decision::kTrue) {
RelaxEffectsAndControls(node); RelaxEffectsAndControls(node);
return Changed(node); return Changed(node);
...@@ -483,7 +485,7 @@ Reduction CommonOperatorReducer::ReduceTrapConditional(Node* trap) { ...@@ -483,7 +485,7 @@ Reduction CommonOperatorReducer::ReduceTrapConditional(Node* trap) {
trap->opcode() == IrOpcode::kTrapUnless); trap->opcode() == IrOpcode::kTrapUnless);
bool trapping_condition = trap->opcode() == IrOpcode::kTrapIf; bool trapping_condition = trap->opcode() == IrOpcode::kTrapIf;
Node* const cond = trap->InputAt(0); Node* const cond = trap->InputAt(0);
Decision decision = DecideCondition(broker(), cond); Decision decision = DecideCondition(cond);
if (decision == Decision::kUnknown) { if (decision == Decision::kUnknown) {
return NoChange(); return NoChange();
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "src/base/compiler-specific.h" #include "src/base/compiler-specific.h"
#include "src/common/globals.h" #include "src/common/globals.h"
#include "src/compiler/common-operator.h"
#include "src/compiler/graph-reducer.h" #include "src/compiler/graph-reducer.h"
namespace v8 { namespace v8 {
...@@ -26,7 +27,8 @@ class V8_EXPORT_PRIVATE CommonOperatorReducer final ...@@ -26,7 +27,8 @@ class V8_EXPORT_PRIVATE CommonOperatorReducer final
public: public:
CommonOperatorReducer(Editor* editor, Graph* graph, JSHeapBroker* broker, CommonOperatorReducer(Editor* editor, Graph* graph, JSHeapBroker* broker,
CommonOperatorBuilder* common, CommonOperatorBuilder* common,
MachineOperatorBuilder* machine, Zone* temp_zone); MachineOperatorBuilder* machine, Zone* temp_zone,
BranchSemantics branch_semantics);
~CommonOperatorReducer() final = default; ~CommonOperatorReducer() final = default;
const char* reducer_name() const override { return "CommonOperatorReducer"; } const char* reducer_name() const override { return "CommonOperatorReducer"; }
...@@ -48,6 +50,9 @@ class V8_EXPORT_PRIVATE CommonOperatorReducer final ...@@ -48,6 +50,9 @@ class V8_EXPORT_PRIVATE CommonOperatorReducer final
Reduction Change(Node* node, Operator const* op, Node* a); Reduction Change(Node* node, Operator const* op, Node* a);
Reduction Change(Node* node, Operator const* op, Node* a, Node* b); Reduction Change(Node* node, Operator const* op, Node* a, Node* b);
// Helper to determine if conditions are true or false.
Decision DecideCondition(Node* const cond);
Graph* graph() const { return graph_; } Graph* graph() const { return graph_; }
JSHeapBroker* broker() const { return broker_; } JSHeapBroker* broker() const { return broker_; }
CommonOperatorBuilder* common() const { return common_; } CommonOperatorBuilder* common() const { return common_; }
...@@ -60,6 +65,7 @@ class V8_EXPORT_PRIVATE CommonOperatorReducer final ...@@ -60,6 +65,7 @@ class V8_EXPORT_PRIVATE CommonOperatorReducer final
MachineOperatorBuilder* const machine_; MachineOperatorBuilder* const machine_;
Node* const dead_; Node* const dead_;
Zone* zone_; Zone* zone_;
BranchSemantics branch_semantics_;
}; };
} // namespace compiler } // namespace compiler
......
...@@ -32,6 +32,13 @@ class Operator; ...@@ -32,6 +32,13 @@ class Operator;
class Type; class Type;
class Node; class Node;
// The semantics of IrOpcode::kBranch changes throughout the pipeline, and in
// particular is not the same before SimplifiedLowering (JS semantics) and after
// (machine branch semantics). Some passes are applied both before and after
// SimplifiedLowering, and use the BranchSemantics enum to know how branches
// should be treated.
enum class BranchSemantics { kJS, kMachine };
// Prediction hint for branches. // Prediction hint for branches.
enum class BranchHint : uint8_t { kNone, kTrue, kFalse }; enum class BranchHint : uint8_t { kNone, kTrue, kFalse };
......
...@@ -3,15 +3,19 @@ ...@@ -3,15 +3,19 @@
// found in the LICENSE file. // found in the LICENSE file.
#include "src/compiler/machine-operator-reducer.h" #include "src/compiler/machine-operator-reducer.h"
#include <cmath> #include <cmath>
#include <limits> #include <limits>
#include "src/base/bits.h" #include "src/base/bits.h"
#include "src/base/division-by-constant.h" #include "src/base/division-by-constant.h"
#include "src/base/ieee754.h" #include "src/base/ieee754.h"
#include "src/base/logging.h"
#include "src/base/overflowing-math.h" #include "src/base/overflowing-math.h"
#include "src/codegen/tnode.h"
#include "src/compiler/diamond.h" #include "src/compiler/diamond.h"
#include "src/compiler/graph.h" #include "src/compiler/graph.h"
#include "src/compiler/js-operator.h"
#include "src/compiler/machine-graph.h" #include "src/compiler/machine-graph.h"
#include "src/compiler/node-matchers.h" #include "src/compiler/node-matchers.h"
#include "src/compiler/node-properties.h" #include "src/compiler/node-properties.h"
...@@ -2187,6 +2191,124 @@ Reduction MachineOperatorReducer::ReduceFloat64RoundDown(Node* node) { ...@@ -2187,6 +2191,124 @@ Reduction MachineOperatorReducer::ReduceFloat64RoundDown(Node* node) {
return NoChange(); return NoChange();
} }
namespace {
// Returns true if |node| is a constant whose value is 0.
bool IsZero(Node* node) {
switch (node->opcode()) {
#define CASE_IS_ZERO(opcode, matcher) \
case IrOpcode::opcode: { \
matcher m(node); \
return m.Is(0); \
}
CASE_IS_ZERO(kInt32Constant, Int32Matcher)
CASE_IS_ZERO(kInt64Constant, Int64Matcher)
#undef CASE_IS_ZERO
default:
break;
}
return false;
}
// If |node| is of the form "x == 0", then return "x" (in order to remove the
// "== 0" part).
base::Optional<Node*> TryGetInvertedCondition(Node* cond) {
if (cond->opcode() == IrOpcode::kWord32Equal) {
Int32BinopMatcher m(cond);
if (IsZero(m.right().node())) {
return m.left().node();
}
}
return base::nullopt;
}
struct SimplifiedCondition {
Node* condition;
bool is_inverted;
};
// Tries to simplifies |cond| by removing all top-level "== 0". Everytime such a
// construction is removed, the meaning of the comparison is inverted. This is
// recorded by the variable |is_inverted| throughout this function, and returned
// at the end. If |is_inverted| is true at the end, the caller should invert the
// if/else branches following the comparison.
base::Optional<SimplifiedCondition> TrySimplifyCompareZero(Node* cond) {
bool is_inverted = false;
bool changed = false;
base::Optional<Node*> new_cond;
while ((new_cond = TryGetInvertedCondition(cond)).has_value()) {
cond = *new_cond;
is_inverted = !is_inverted;
changed = true;
}
if (changed) {
return SimplifiedCondition{cond, is_inverted};
} else {
return {};
}
}
} // namespace
void MachineOperatorReducer::SwapBranches(Node* node) {
DCHECK_EQ(node->opcode(), IrOpcode::kBranch);
for (Node* const use : node->uses()) {
switch (use->opcode()) {
case IrOpcode::kIfTrue:
NodeProperties::ChangeOp(use, common()->IfFalse());
break;
case IrOpcode::kIfFalse:
NodeProperties::ChangeOp(use, common()->IfTrue());
break;
default:
UNREACHABLE();
}
}
NodeProperties::ChangeOp(
node, common()->Branch(NegateBranchHint(BranchHintOf(node->op()))));
}
// If |node| is a branch, removes all top-level 32-bit "== 0" from |node|.
Reduction MachineOperatorReducer::SimplifyBranch(Node* node) {
Node* cond = node->InputAt(0);
if (auto simplified = TrySimplifyCompareZero(cond)) {
node->ReplaceInput(0, simplified->condition);
if (simplified->is_inverted) {
switch (node->opcode()) {
case IrOpcode::kBranch:
SwapBranches(node);
break;
case IrOpcode::kTrapIf:
NodeProperties::ChangeOp(node,
common()->TrapUnless(TrapIdOf(node->op())));
break;
case IrOpcode::kTrapUnless:
NodeProperties::ChangeOp(node,
common()->TrapIf(TrapIdOf(node->op())));
break;
case IrOpcode::kDeoptimizeIf: {
DeoptimizeParameters p = DeoptimizeParametersOf(node->op());
NodeProperties::ChangeOp(
node,
common()->DeoptimizeUnless(p.kind(), p.reason(), p.feedback()));
break;
}
case IrOpcode::kDeoptimizeUnless: {
DeoptimizeParameters p = DeoptimizeParametersOf(node->op());
NodeProperties::ChangeOp(
node, common()->DeoptimizeIf(p.kind(), p.reason(), p.feedback()));
break;
}
default:
UNREACHABLE();
}
}
return Changed(node);
}
return NoChange();
}
Reduction MachineOperatorReducer::ReduceConditional(Node* node) { Reduction MachineOperatorReducer::ReduceConditional(Node* node) {
DCHECK(node->opcode() == IrOpcode::kBranch || DCHECK(node->opcode() == IrOpcode::kBranch ||
node->opcode() == IrOpcode::kDeoptimizeIf || node->opcode() == IrOpcode::kDeoptimizeIf ||
...@@ -2197,17 +2319,18 @@ Reduction MachineOperatorReducer::ReduceConditional(Node* node) { ...@@ -2197,17 +2319,18 @@ Reduction MachineOperatorReducer::ReduceConditional(Node* node) {
// Reductions involving control flow happen elsewhere. Non-zero inputs are // Reductions involving control flow happen elsewhere. Non-zero inputs are
// considered true in all conditional ops. // considered true in all conditional ops.
NodeMatcher condition(NodeProperties::GetValueInput(node, 0)); NodeMatcher condition(NodeProperties::GetValueInput(node, 0));
Reduction reduction = NoChange();
if (condition.IsTruncateInt64ToInt32()) { if (condition.IsTruncateInt64ToInt32()) {
if (auto replacement = if (auto replacement =
ReduceConditionalN<Word64Adapter>(condition.node())) { ReduceConditionalN<Word64Adapter>(condition.node())) {
NodeProperties::ReplaceValueInput(node, *replacement, 0); NodeProperties::ReplaceValueInput(node, *replacement, 0);
return Changed(node); reduction = Changed(node);
} }
} else if (auto replacement = ReduceConditionalN<Word32Adapter>(node)) { } else if (auto replacement = ReduceConditionalN<Word32Adapter>(node)) {
NodeProperties::ReplaceValueInput(node, *replacement, 0); NodeProperties::ReplaceValueInput(node, *replacement, 0);
return Changed(node); reduction = Changed(node);
} }
return NoChange(); return reduction.FollowedBy(SimplifyBranch(node));
} }
template <typename WordNAdapter> template <typename WordNAdapter>
......
...@@ -131,6 +131,12 @@ class V8_EXPORT_PRIVATE MachineOperatorReducer final ...@@ -131,6 +131,12 @@ class V8_EXPORT_PRIVATE MachineOperatorReducer final
template <typename WordNAdapter> template <typename WordNAdapter>
Reduction ReduceWordNXor(Node* node); Reduction ReduceWordNXor(Node* node);
// Tries to simplify "if(x == 0)" by removing the "== 0" and inverting
// branches.
Reduction SimplifyBranch(Node* node);
// Helper for SimplifyBranch; swaps the if/else of a branch.
void SwapBranches(Node* node);
// Helper for ReduceConditional. Does not perform the actual reduction; just // Helper for ReduceConditional. Does not perform the actual reduction; just
// returns a new Node that could be used as the input to the condition. // returns a new Node that could be used as the input to the condition.
template <typename WordNAdapter> template <typename WordNAdapter>
......
This diff is collapsed.
...@@ -4025,7 +4025,8 @@ class RepresentationSelector { ...@@ -4025,7 +4025,8 @@ class RepresentationSelector {
ProcessInput<T>(node, 0, UseInfo::Any()); ProcessInput<T>(node, 0, UseInfo::Any());
return SetOutput<T>(node, MachineRepresentation::kNone); return SetOutput<T>(node, MachineRepresentation::kNone);
case IrOpcode::kStaticAssert: case IrOpcode::kStaticAssert:
return VisitUnop<T>(node, UseInfo::Any(), DCHECK(TypeOf(node->InputAt(0)).Is(Type::Boolean()));
return VisitUnop<T>(node, UseInfo::Bool(),
MachineRepresentation::kTagged); MachineRepresentation::kTagged);
case IrOpcode::kAssertType: case IrOpcode::kAssertType:
return VisitUnop<T>(node, UseInfo::AnyTagged(), return VisitUnop<T>(node, UseInfo::AnyTagged(),
......
...@@ -4,10 +4,12 @@ ...@@ -4,10 +4,12 @@
#include "src/compiler/simplified-operator-reducer.h" #include "src/compiler/simplified-operator-reducer.h"
#include "src/compiler/common-operator.h"
#include "src/compiler/js-graph.h" #include "src/compiler/js-graph.h"
#include "src/compiler/js-heap-broker.h" #include "src/compiler/js-heap-broker.h"
#include "src/compiler/machine-operator.h" #include "src/compiler/machine-operator.h"
#include "src/compiler/node-matchers.h" #include "src/compiler/node-matchers.h"
#include "src/compiler/opcodes.h"
#include "src/compiler/operator-properties.h" #include "src/compiler/operator-properties.h"
#include "src/compiler/simplified-operator.h" #include "src/compiler/simplified-operator.h"
#include "src/compiler/type-cache.h" #include "src/compiler/type-cache.h"
...@@ -33,10 +35,13 @@ Decision DecideObjectIsSmi(Node* const input) { ...@@ -33,10 +35,13 @@ Decision DecideObjectIsSmi(Node* const input) {
} // namespace } // namespace
SimplifiedOperatorReducer::SimplifiedOperatorReducer(Editor* editor, SimplifiedOperatorReducer::SimplifiedOperatorReducer(
JSGraph* jsgraph, Editor* editor, JSGraph* jsgraph, JSHeapBroker* broker,
JSHeapBroker* broker) BranchSemantics branch_semantics)
: AdvancedReducer(editor), jsgraph_(jsgraph), broker_(broker) {} : AdvancedReducer(editor),
jsgraph_(jsgraph),
broker_(broker),
branch_semantics_(branch_semantics) {}
SimplifiedOperatorReducer::~SimplifiedOperatorReducer() = default; SimplifiedOperatorReducer::~SimplifiedOperatorReducer() = default;
...@@ -277,7 +282,11 @@ Reduction SimplifiedOperatorReducer::Change(Node* node, const Operator* op, ...@@ -277,7 +282,11 @@ Reduction SimplifiedOperatorReducer::Change(Node* node, const Operator* op,
} }
Reduction SimplifiedOperatorReducer::ReplaceBoolean(bool value) { Reduction SimplifiedOperatorReducer::ReplaceBoolean(bool value) {
return Replace(jsgraph()->BooleanConstant(value)); if (branch_semantics_ == BranchSemantics::kJS) {
return Replace(jsgraph()->BooleanConstant(value));
} else {
return ReplaceInt32(value);
}
} }
Reduction SimplifiedOperatorReducer::ReplaceFloat64(double value) { Reduction SimplifiedOperatorReducer::ReplaceFloat64(double value) {
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "src/base/compiler-specific.h" #include "src/base/compiler-specific.h"
#include "src/common/globals.h" #include "src/common/globals.h"
#include "src/compiler/common-operator.h"
#include "src/compiler/graph-reducer.h" #include "src/compiler/graph-reducer.h"
namespace v8 { namespace v8 {
...@@ -27,7 +28,8 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorReducer final ...@@ -27,7 +28,8 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorReducer final
: public NON_EXPORTED_BASE(AdvancedReducer) { : public NON_EXPORTED_BASE(AdvancedReducer) {
public: public:
SimplifiedOperatorReducer(Editor* editor, JSGraph* jsgraph, SimplifiedOperatorReducer(Editor* editor, JSGraph* jsgraph,
JSHeapBroker* broker); JSHeapBroker* broker,
BranchSemantics branch_semantics);
~SimplifiedOperatorReducer() final; ~SimplifiedOperatorReducer() final;
SimplifiedOperatorReducer(const SimplifiedOperatorReducer&) = delete; SimplifiedOperatorReducer(const SimplifiedOperatorReducer&) = delete;
SimplifiedOperatorReducer& operator=(const SimplifiedOperatorReducer&) = SimplifiedOperatorReducer& operator=(const SimplifiedOperatorReducer&) =
...@@ -60,6 +62,7 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorReducer final ...@@ -60,6 +62,7 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorReducer final
JSGraph* const jsgraph_; JSGraph* const jsgraph_;
JSHeapBroker* const broker_; JSHeapBroker* const broker_;
BranchSemantics branch_semantics_;
}; };
} // namespace compiler } // namespace compiler
......
...@@ -322,6 +322,7 @@ class RuntimeCallTimer final { ...@@ -322,6 +322,7 @@ class RuntimeCallTimer final {
ADD_THREAD_SPECIFIC_COUNTER(V, Optimize, AllocateGeneralRegisters) \ ADD_THREAD_SPECIFIC_COUNTER(V, Optimize, AllocateGeneralRegisters) \
ADD_THREAD_SPECIFIC_COUNTER(V, Optimize, AssembleCode) \ ADD_THREAD_SPECIFIC_COUNTER(V, Optimize, AssembleCode) \
ADD_THREAD_SPECIFIC_COUNTER(V, Optimize, AssignSpillSlots) \ ADD_THREAD_SPECIFIC_COUNTER(V, Optimize, AssignSpillSlots) \
ADD_THREAD_SPECIFIC_COUNTER(V, Optimize, BranchConditionDuplication) \
ADD_THREAD_SPECIFIC_COUNTER(V, Optimize, BuildLiveRangeBundles) \ ADD_THREAD_SPECIFIC_COUNTER(V, Optimize, BuildLiveRangeBundles) \
ADD_THREAD_SPECIFIC_COUNTER(V, Optimize, BuildLiveRanges) \ ADD_THREAD_SPECIFIC_COUNTER(V, Optimize, BuildLiveRanges) \
ADD_THREAD_SPECIFIC_COUNTER(V, Optimize, BytecodeGraphBuilder) \ ADD_THREAD_SPECIFIC_COUNTER(V, Optimize, BytecodeGraphBuilder) \
......
...@@ -35,7 +35,8 @@ class SimplifiedOperatorReducerTest : public GraphTest { ...@@ -35,7 +35,8 @@ class SimplifiedOperatorReducerTest : public GraphTest {
JSGraph jsgraph(isolate(), graph(), common(), &javascript, simplified(), JSGraph jsgraph(isolate(), graph(), common(), &javascript, simplified(),
&machine); &machine);
GraphReducer graph_reducer(zone(), graph(), tick_counter(), broker()); GraphReducer graph_reducer(zone(), graph(), tick_counter(), broker());
SimplifiedOperatorReducer reducer(&graph_reducer, &jsgraph, broker()); SimplifiedOperatorReducer reducer(&graph_reducer, &jsgraph, broker(),
BranchSemantics::kJS);
return reducer.Reduce(node); return reducer.Reduce(node);
} }
......
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