[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

@@ -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 (b->opcode()) {
+    case 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;
       }
+      break;
     }
-  if (a->opcode() == IrOpcode::kAllocate) {
+    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;
-      case IrOpcode::kFinishRegion:
-        return QueryAlias(a, b->InputAt(0));
         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();

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<Node*, Node*> 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;

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<Name>(),
+                              Type::Signed32(),
+                              MachineType::AnyTagged(),
+                              kNoWriteBarrier};
+
+  StrictMock<MockAdvancedReducerEditor> 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