[turbofan] Make GraphAssembler branching respect typing

GeorgNeis · Commit Bot · commit 349e4ee3fcbb · 2020-05-15T12:50:11.000Z
GraphAssembler creates Phi nodes and creates additional inputs to them depending on how many jumps go there. If the typer decorator is active, it will type the Phi node at creation time. GraphAssembler was not aware of types (until recently it was not used while the graph is typed) and did not update the Phi type with each new input. This CL fixes that. Bug: chromium:1082704 Change-Id: Id94bcda752c7b3dc836eb2b6c6b55b1690185a09 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2202978 Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Reviewed-by: Nico Hartmann <nicohartmann@chromium.org> Commit-Queue: Georg Neis <neis@chromium.org> Cr-Commit-Position: refs/heads/master@{#67823}
diff --git a/src/compiler/graph-assembler.h b/src/compiler/graph-assembler.h
@@ -582,6 +582,7 @@ void GraphAssembler::MergeState(GraphAssemblerLabel<sizeof...(Vars)>* label,
       label->effect_->ReplaceInput(1, effect());
       for (size_t i = 0; i < kVarCount; i++) {
         label->bindings_[i]->ReplaceInput(1, var_array[i]);
+        CHECK(!NodeProperties::IsTyped(var_array[i]));  // Unsupported.
       }
     }
   } else {
@@ -625,6 +626,13 @@ void GraphAssembler::MergeState(GraphAssemblerLabel<sizeof...(Vars)>* label,
         NodeProperties::ChangeOp(
             label->bindings_[i],
             common()->Phi(label->representations_[i], merged_count + 1));
+        if (NodeProperties::IsTyped(label->bindings_[i])) {
+          CHECK(NodeProperties::IsTyped(var_array[i]));
+          Type old_type = NodeProperties::GetType(label->bindings_[i]);
+          Type new_type = Type::Union(
+              old_type, NodeProperties::GetType(var_array[i]), graph()->zone());
+          NodeProperties::SetType(label->bindings_[i], new_type);
+        }
       }
     }
   }
diff --git a/test/mjsunit/compiler/regress-1082704.js b/test/mjsunit/compiler/regress-1082704.js
@@ -0,0 +1,16 @@
+// Copyright 2020 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.
+
+// Flags: --allow-natives-syntax
+
+var array = [[]];
+function foo() {
+  const x = array[0];
+  const y = [][0];
+  return x == y;
+}
+%PrepareFunctionForOptimization(foo);
+assertFalse(foo());
+%OptimizeFunctionOnNextCall(foo);
+assertFalse(foo());

Original file line number	Diff line number	Diff line change
`@@ -582,6 +582,7 @@ void GraphAssembler::MergeState(GraphAssemblerLabel<sizeof...(Vars)>* label,`
`582`	`582`	`label->effect_->ReplaceInput(1, effect());`
`583`	`583`	`for (size_t i = 0; i < kVarCount; i++) {`
`584`	`584`	`label->bindings_[i]->ReplaceInput(1, var_array[i]);`
	`585`	`+ CHECK(!NodeProperties::IsTyped(var_array[i])); // Unsupported.`
`585`	`586`	`}`
`586`	`587`	`}`
`587`	`588`	`} else {`
`@@ -625,6 +626,13 @@ void GraphAssembler::MergeState(GraphAssemblerLabel<sizeof...(Vars)>* label,`
`625`	`626`	`NodeProperties::ChangeOp(`
`626`	`627`	`label->bindings_[i],`
`627`	`628`	`common()->Phi(label->representations_[i], merged_count + 1));`
	`629`	`+ if (NodeProperties::IsTyped(label->bindings_[i])) {`
	`630`	`+ CHECK(NodeProperties::IsTyped(var_array[i]));`
	`631`	`+ Type old_type = NodeProperties::GetType(label->bindings_[i]);`
	`632`	`+ Type new_type = Type::Union(`
	`633`	`+ old_type, NodeProperties::GetType(var_array[i]), graph()->zone());`
	`634`	`+ NodeProperties::SetType(label->bindings_[i], new_type);`
	`635`	`+ }`
`628`	`636`	`}`
`629`	`637`	`}`
`630`	`638`	`}`