From f4900cf92b51d94744451ad5ea9d2ef629011918 Mon Sep 17 00:00:00 2001 From: Nico Hartmann Date: Thu, 26 Jan 2023 16:20:58 +0100 Subject: [PATCH] [turbofan] Add proper conversions in RedundancyElimination Change-Id: Ia832abb79894dfde290a8127534b161d6fcc8178 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4197350 Reviewed-by: Darius Mercadier Commit-Queue: Nico Hartmann Cr-Commit-Position: refs/heads/main@{#85504} --- src/compiler/pipeline.cc | 6 +- src/compiler/redundancy-elimination.cc | 84 ++++++++++++++++--- src/compiler/redundancy-elimination.h | 8 +- .../redundancy-elimination-unittest.cc | 14 +++- 4 files changed, 92 insertions(+), 20 deletions(-) diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc index 4683dbbdc5..d1c5e27779 100644 --- a/src/compiler/pipeline.cc +++ b/src/compiler/pipeline.cc @@ -1805,7 +1805,8 @@ struct EarlyOptimizationPhase { SimplifiedOperatorReducer simple_reducer(&graph_reducer, data->jsgraph(), data->broker(), BranchSemantics::kMachine); - RedundancyElimination redundancy_elimination(&graph_reducer, temp_zone); + RedundancyElimination redundancy_elimination(&graph_reducer, + data->jsgraph(), temp_zone); ValueNumberingReducer value_numbering(temp_zone, data->graph()->zone()); MachineOperatorReducer machine_reducer( &graph_reducer, data->jsgraph(), @@ -1920,7 +1921,8 @@ struct LoadEliminationPhase { &graph_reducer, data->jsgraph(), temp_zone, BranchElimination::kEARLY); DeadCodeElimination dead_code_elimination(&graph_reducer, data->graph(), data->common(), temp_zone); - RedundancyElimination redundancy_elimination(&graph_reducer, temp_zone); + RedundancyElimination redundancy_elimination(&graph_reducer, + data->jsgraph(), temp_zone); LoadElimination load_elimination(&graph_reducer, data->jsgraph(), temp_zone); CheckpointElimination checkpoint_elimination(&graph_reducer); diff --git a/src/compiler/redundancy-elimination.cc b/src/compiler/redundancy-elimination.cc index 9cfb03c18c..784665cb94 100644 --- a/src/compiler/redundancy-elimination.cc +++ b/src/compiler/redundancy-elimination.cc @@ -4,6 +4,7 @@ #include "src/compiler/redundancy-elimination.h" +#include "src/compiler/js-graph.h" #include "src/compiler/node-properties.h" #include "src/compiler/simplified-operator.h" @@ -11,8 +12,12 @@ namespace v8 { namespace internal { namespace compiler { -RedundancyElimination::RedundancyElimination(Editor* editor, Zone* zone) - : AdvancedReducer(editor), node_checks_(zone), zone_(zone) {} +RedundancyElimination::RedundancyElimination(Editor* editor, JSGraph* jsgraph, + Zone* zone) + : AdvancedReducer(editor), + node_checks_(zone), + jsgraph_(jsgraph), + zone_(zone) {} RedundancyElimination::~RedundancyElimination() = default; @@ -133,8 +138,43 @@ RedundancyElimination::EffectPathChecks::AddCheck(Zone* zone, namespace { +struct Subsumption { + enum class Kind { + kNone, + kImplicit, + kWithConversion, + }; + + static Subsumption None() { return Subsumption(Kind::kNone, nullptr); } + static Subsumption Implicit() { + return Subsumption(Kind::kImplicit, nullptr); + } + static Subsumption WithConversion(const Operator* conversion_op) { + return Subsumption(Kind::kWithConversion, conversion_op); + } + + bool IsNone() const { return kind_ == Kind::kNone; } + bool IsImplicit() const { return kind_ == Kind::kImplicit; } + bool IsWithConversion() const { return kind_ == Kind::kWithConversion; } + const Operator* conversion_operator() const { + DCHECK(IsWithConversion()); + return conversion_op_; + } + + private: + Subsumption(Kind kind, const Operator* conversion_op) + : kind_(kind), conversion_op_(conversion_op) { + DCHECK_EQ(kind_ == Kind::kWithConversion, conversion_op_ != nullptr); + } + + Kind kind_; + const Operator* conversion_op_; +}; + // Does check {a} subsume check {b}? -bool CheckSubsumes(Node const* a, Node const* b) { +Subsumption CheckSubsumes(Node const* a, Node const* b, + MachineOperatorBuilder* machine) { + Subsumption subsumption = Subsumption::Implicit(); if (a->op() != b->op()) { if (a->opcode() == IrOpcode::kCheckInternalizedString && b->opcode() == IrOpcode::kCheckString) { @@ -149,14 +189,24 @@ bool CheckSubsumes(Node const* a, Node const* b) { b->opcode() == IrOpcode::kCheckedTaggedToArrayIndex) { // CheckedTaggedSignedToInt32(node) implies // CheckedTaggedToArrayIndex(node) + if (machine->Is64()) { + // On 64 bit architectures, ArrayIndex is 64 bit. + subsumption = + Subsumption::WithConversion(machine->ChangeInt32ToInt64()); + } } else if (a->opcode() == IrOpcode::kCheckedTaggedToInt32 && b->opcode() == IrOpcode::kCheckedTaggedToArrayIndex) { // CheckedTaggedToInt32(node) implies CheckedTaggedToArrayIndex(node) + if (machine->Is64()) { + // On 64 bit architectures, ArrayIndex is 64 bit. + subsumption = + Subsumption::WithConversion(machine->ChangeInt32ToInt64()); + } } else if (a->opcode() == IrOpcode::kCheckReceiver && b->opcode() == IrOpcode::kCheckReceiverOrNullOrUndefined) { // CheckReceiver(node) implies CheckReceiverOrNullOrUndefined(node) } else if (a->opcode() != b->opcode()) { - return false; + return Subsumption::None(); } else { switch (a->opcode()) { case IrOpcode::kCheckBounds: @@ -189,7 +239,7 @@ bool CheckSubsumes(Node const* a, Node const* b) { const CheckMinusZeroParameters& bp = CheckMinusZeroParametersOf(b->op()); if (ap.mode() != bp.mode()) { - return false; + return Subsumption::None(); } break; } @@ -203,20 +253,20 @@ bool CheckSubsumes(Node const* a, Node const* b) { // for Number, in which case {b} will be subsumed no matter what. if (ap.mode() != bp.mode() && ap.mode() != CheckTaggedInputMode::kNumber) { - return false; + return Subsumption::None(); } break; } default: DCHECK(!IsCheckedWithFeedback(a->op())); - return false; + return Subsumption::None(); } } } for (int i = a->op()->ValueInputCount(); --i >= 0;) { - if (a->InputAt(i) != b->InputAt(i)) return false; + if (a->InputAt(i) != b->InputAt(i)) return Subsumption::None(); } - return true; + return subsumption; } bool TypeSubsumes(Node* node, Node* replacement) { @@ -232,11 +282,19 @@ bool TypeSubsumes(Node* node, Node* replacement) { } // namespace -Node* RedundancyElimination::EffectPathChecks::LookupCheck(Node* node) const { +Node* RedundancyElimination::EffectPathChecks::LookupCheck( + Node* node, JSGraph* jsgraph) const { for (Check const* check = head_; check != nullptr; check = check->next) { - if (CheckSubsumes(check->node, node) && TypeSubsumes(node, check->node)) { + Subsumption subsumption = + CheckSubsumes(check->node, node, jsgraph->machine()); + if (!subsumption.IsNone() && TypeSubsumes(node, check->node)) { DCHECK(!check->node->IsDead()); - return check->node; + Node* result = check->node; + if (subsumption.IsWithConversion()) { + result = jsgraph->graph()->NewNode(subsumption.conversion_operator(), + result); + } + return result; } } return nullptr; @@ -276,7 +334,7 @@ Reduction RedundancyElimination::ReduceCheckNode(Node* node) { // because we will have to recompute anyway once we compute the predecessor. if (checks == nullptr) return NoChange(); // See if we have another check that dominates us. - if (Node* check = checks->LookupCheck(node)) { + if (Node* check = checks->LookupCheck(node, jsgraph_)) { ReplaceWithValue(node, check); return Replace(check); } diff --git a/src/compiler/redundancy-elimination.h b/src/compiler/redundancy-elimination.h index cabdb1b41c..ffa02df7a2 100644 --- a/src/compiler/redundancy-elimination.h +++ b/src/compiler/redundancy-elimination.h @@ -6,14 +6,17 @@ #define V8_COMPILER_REDUNDANCY_ELIMINATION_H_ #include "src/compiler/graph-reducer.h" +#include "src/compiler/machine-operator.h" namespace v8 { namespace internal { namespace compiler { +class JSGraph; + class V8_EXPORT_PRIVATE RedundancyElimination final : public AdvancedReducer { public: - RedundancyElimination(Editor* editor, Zone* zone); + RedundancyElimination(Editor* editor, JSGraph* jsgraph, Zone* zone); ~RedundancyElimination() final; RedundancyElimination(const RedundancyElimination&) = delete; RedundancyElimination& operator=(const RedundancyElimination&) = delete; @@ -37,7 +40,7 @@ class V8_EXPORT_PRIVATE RedundancyElimination final : public AdvancedReducer { void Merge(EffectPathChecks const* that); EffectPathChecks const* AddCheck(Zone* zone, Node* node) const; - Node* LookupCheck(Node* node) const; + Node* LookupCheck(Node* node, JSGraph* jsgraph) const; Node* LookupBoundsCheckFor(Node* node) const; private: @@ -74,6 +77,7 @@ class V8_EXPORT_PRIVATE RedundancyElimination final : public AdvancedReducer { Zone* zone() const { return zone_; } PathChecksForEffectNodes node_checks_; + JSGraph* jsgraph_; Zone* const zone_; }; diff --git a/test/unittests/compiler/redundancy-elimination-unittest.cc b/test/unittests/compiler/redundancy-elimination-unittest.cc index 5625a138a2..b7113563d3 100644 --- a/test/unittests/compiler/redundancy-elimination-unittest.cc +++ b/test/unittests/compiler/redundancy-elimination-unittest.cc @@ -6,6 +6,7 @@ #include "src/codegen/tick-counter.h" #include "src/compiler/feedback-source.h" +#include "src/compiler/js-graph.h" #include "test/unittests/compiler/graph-reducer-unittest.h" #include "test/unittests/compiler/graph-unittest.h" #include "test/unittests/compiler/node-test-utils.h" @@ -22,8 +23,12 @@ class RedundancyEliminationTest : public GraphTest { public: explicit RedundancyEliminationTest(int num_parameters = 4) : GraphTest(num_parameters), - reducer_(&editor_, zone()), - simplified_(zone()) { + javascript_(zone()), + simplified_(zone()), + machine_(zone()), + jsgraph_(isolate(), graph(), common(), &javascript_, &simplified_, + &machine_), + reducer_(&editor_, &jsgraph_, zone()) { // Initialize the {reducer_} state for the Start node. reducer_.Reduce(graph()->start()); @@ -51,8 +56,11 @@ class RedundancyEliminationTest : public GraphTest { NiceMock editor_; std::vector vector_slot_pairs_; FeedbackSource feedback2_; - RedundancyElimination reducer_; + JSOperatorBuilder javascript_; SimplifiedOperatorBuilder simplified_; + MachineOperatorBuilder machine_; + JSGraph jsgraph_; + RedundancyElimination reducer_; }; namespace {