From 9f37e303c3fab8146826a01ae442ef27c3cec4aa Mon Sep 17 00:00:00 2001 From: bmeurer Date: Thu, 1 Sep 2016 03:50:08 -0700 Subject: [PATCH] [turbofan] Properly look through FinishRegion in alias analysis. For two FinishRegion nodes, the alias analysis returned "may alias" even without properly looking through them. Drive-by-fix: Add meaningful output for --trace-turbo-load-elimination. R=jarin@chromium.org BUG=v8:5267 Review-Url: https://codereview.chromium.org/2301903002 Cr-Commit-Position: refs/heads/master@{#39075} --- src/compiler/load-elimination.cc | 118 +++++++++++++++--- src/compiler/load-elimination.h | 8 ++ .../compiler/load-elimination-unittest.cc | 59 +++++++++ 3 files changed, 166 insertions(+), 19 deletions(-) diff --git a/src/compiler/load-elimination.cc b/src/compiler/load-elimination.cc index 82fc47af07..957859a94c 100644 --- a/src/compiler/load-elimination.cc +++ b/src/compiler/load-elimination.cc @@ -22,28 +22,38 @@ Aliasing QueryAlias(Node* a, Node* b) { if (!NodeProperties::GetType(a)->Maybe(NodeProperties::GetType(b))) { return kNoAlias; } - if (b->opcode() == IrOpcode::kAllocate) { - switch (a->opcode()) { - case IrOpcode::kAllocate: - case IrOpcode::kHeapConstant: - case IrOpcode::kParameter: - return kNoAlias; - case IrOpcode::kFinishRegion: - return QueryAlias(a->InputAt(0), b); - default: - break; + switch (b->opcode()) { + case IrOpcode::kAllocate: { + switch (a->opcode()) { + case IrOpcode::kAllocate: + case IrOpcode::kHeapConstant: + case IrOpcode::kParameter: + return kNoAlias; + default: + break; + } + break; } + case IrOpcode::kFinishRegion: + return QueryAlias(a, b->InputAt(0)); + default: + break; } - if (a->opcode() == IrOpcode::kAllocate) { - switch (b->opcode()) { - case IrOpcode::kHeapConstant: - case IrOpcode::kParameter: - return kNoAlias; - case IrOpcode::kFinishRegion: - return QueryAlias(a, b->InputAt(0)); - default: - break; + switch (a->opcode()) { + case IrOpcode::kAllocate: { + switch (b->opcode()) { + case IrOpcode::kHeapConstant: + case IrOpcode::kParameter: + return kNoAlias; + default: + break; + } + break; } + case IrOpcode::kFinishRegion: + return QueryAlias(a->InputAt(0), b); + default: + break; } return kMayAlias; } @@ -55,6 +65,32 @@ bool MustAlias(Node* a, Node* b) { return QueryAlias(a, b) == kMustAlias; } } // namespace Reduction LoadElimination::Reduce(Node* node) { + if (FLAG_trace_turbo_load_elimination) { + if (node->op()->EffectInputCount() > 0) { + PrintF(" visit #%d:%s", node->id(), node->op()->mnemonic()); + if (node->op()->ValueInputCount() > 0) { + PrintF("("); + for (int i = 0; i < node->op()->ValueInputCount(); ++i) { + if (i > 0) PrintF(", "); + Node* const value = NodeProperties::GetValueInput(node, i); + PrintF("#%d:%s", value->id(), value->op()->mnemonic()); + } + PrintF(")"); + } + PrintF("\n"); + for (int i = 0; i < node->op()->EffectInputCount(); ++i) { + Node* const effect = NodeProperties::GetEffectInput(node, i); + if (AbstractState const* const state = node_states_.Get(effect)) { + PrintF(" state[%i]: #%d:%s\n", i, effect->id(), + effect->op()->mnemonic()); + state->Print(); + } else { + PrintF(" no state[%i]: #%d:%s\n", i, effect->id(), + effect->op()->mnemonic()); + } + } + } + } switch (node->opcode()) { case IrOpcode::kArrayBufferWasNeutered: return ReduceArrayBufferWasNeutered(node); @@ -147,6 +183,14 @@ LoadElimination::AbstractChecks const* LoadElimination::AbstractChecks::Merge( return copy; } +void LoadElimination::AbstractChecks::Print() const { + for (Node* const node : nodes_) { + if (node != nullptr) { + PrintF(" #%d:%s\n", node->id(), node->op()->mnemonic()); + } + } +} + Node* LoadElimination::AbstractElements::Lookup(Node* object, Node* index) const { for (Element const element : elements_) { @@ -235,6 +279,17 @@ LoadElimination::AbstractElements::Merge(AbstractElements const* that, return copy; } +void LoadElimination::AbstractElements::Print() const { + for (Element const& element : elements_) { + if (element.object) { + PrintF(" #%d:%s @ #%d:%s -> #%d:%s\n", element.object->id(), + element.object->op()->mnemonic(), element.index->id(), + element.index->op()->mnemonic(), element.value->id(), + element.value->op()->mnemonic()); + } + } +} + Node* LoadElimination::AbstractField::Lookup(Node* object) const { for (auto pair : info_for_node_) { if (MustAlias(object, pair.first)) return pair.second; @@ -256,6 +311,14 @@ LoadElimination::AbstractField const* LoadElimination::AbstractField::Kill( return this; } +void LoadElimination::AbstractField::Print() const { + for (auto pair : info_for_node_) { + PrintF(" #%d:%s -> #%d:%s\n", pair.first->id(), + pair.first->op()->mnemonic(), pair.second->id(), + pair.second->op()->mnemonic()); + } +} + bool LoadElimination::AbstractState::Equals(AbstractState const* that) const { if (this->checks_) { if (!that->checks_ || !that->checks_->Equals(this->checks_)) { @@ -392,6 +455,23 @@ Node* LoadElimination::AbstractState::LookupField(Node* object, return nullptr; } +void LoadElimination::AbstractState::Print() const { + if (checks_) { + PrintF(" checks:\n"); + checks_->Print(); + } + if (elements_) { + PrintF(" elements:\n"); + elements_->Print(); + } + for (size_t i = 0; i < arraysize(fields_); ++i) { + if (AbstractField const* const field = fields_[i]) { + PrintF(" field %zu:\n", i); + field->Print(); + } + } +} + LoadElimination::AbstractState const* LoadElimination::AbstractStateForEffectNodes::Get(Node* node) const { size_t const id = node->id(); diff --git a/src/compiler/load-elimination.h b/src/compiler/load-elimination.h index 5aa62b48bd..8da3545b95 100644 --- a/src/compiler/load-elimination.h +++ b/src/compiler/load-elimination.h @@ -52,6 +52,8 @@ class LoadElimination final : public AdvancedReducer { bool Equals(AbstractChecks const* that) const; AbstractChecks const* Merge(AbstractChecks const* that, Zone* zone) const; + void Print() const; + private: Node* nodes_[kMaxTrackedChecks]; size_t next_index_ = 0; @@ -86,6 +88,8 @@ class LoadElimination final : public AdvancedReducer { AbstractElements const* Merge(AbstractElements const* that, Zone* zone) const; + void Print() const; + private: struct Element { Element() {} @@ -137,6 +141,8 @@ class LoadElimination final : public AdvancedReducer { return copy; } + void Print() const; + private: ZoneMap info_for_node_; }; @@ -169,6 +175,8 @@ class LoadElimination final : public AdvancedReducer { AbstractState const* AddCheck(Node* node, Zone* zone) const; Node* LookupCheck(Node* node) const; + void Print() const; + private: AbstractChecks const* checks_ = nullptr; AbstractElements const* elements_ = nullptr; diff --git a/test/unittests/compiler/load-elimination-unittest.cc b/test/unittests/compiler/load-elimination-unittest.cc index 2306f72b4c..81393941bb 100644 --- a/test/unittests/compiler/load-elimination-unittest.cc +++ b/test/unittests/compiler/load-elimination-unittest.cc @@ -417,6 +417,65 @@ TEST_F(LoadEliminationTest, LoadElementWithTypeMismatch) { EXPECT_THAT(r.replacement(), IsTypeGuard(value, control)); } +TEST_F(LoadEliminationTest, AliasAnalysisForFinishRegion) { + Node* value0 = Parameter(Type::Signed32(), 0); + Node* value1 = Parameter(Type::Signed32(), 1); + Node* effect = graph()->start(); + Node* control = graph()->start(); + FieldAccess const access = {kTaggedBase, + kPointerSize, + MaybeHandle(), + Type::Signed32(), + MachineType::AnyTagged(), + kNoWriteBarrier}; + + StrictMock editor; + LoadElimination load_elimination(&editor, jsgraph(), zone()); + + load_elimination.Reduce(effect); + + effect = graph()->NewNode( + common()->BeginRegion(RegionObservability::kNotObservable), effect); + load_elimination.Reduce(effect); + + Node* object0 = effect = + graph()->NewNode(simplified()->Allocate(NOT_TENURED), + jsgraph()->Constant(16), effect, control); + load_elimination.Reduce(effect); + + Node* region0 = effect = + graph()->NewNode(common()->FinishRegion(), object0, effect); + load_elimination.Reduce(effect); + + effect = graph()->NewNode( + common()->BeginRegion(RegionObservability::kNotObservable), effect); + load_elimination.Reduce(effect); + + Node* object1 = effect = + graph()->NewNode(simplified()->Allocate(NOT_TENURED), + jsgraph()->Constant(16), effect, control); + load_elimination.Reduce(effect); + + Node* region1 = effect = + graph()->NewNode(common()->FinishRegion(), object1, effect); + load_elimination.Reduce(effect); + + effect = graph()->NewNode(simplified()->StoreField(access), region0, value0, + effect, control); + load_elimination.Reduce(effect); + + effect = graph()->NewNode(simplified()->StoreField(access), region1, value1, + effect, control); + load_elimination.Reduce(effect); + + Node* load = graph()->NewNode(simplified()->LoadField(access), region0, + effect, control); + EXPECT_CALL(editor, ReplaceWithValue(load, value0, effect, _)); + Reduction r = load_elimination.Reduce(load); + ASSERT_TRUE(r.Changed()); + EXPECT_EQ(value0, r.replacement()); +} + } // namespace compiler } // namespace internal } // namespace v8