[turbofan] Extend LoadElimination to introduce TypeGuards.

If the type of a tracked field or element value is less precise than the
advertised type of the field or element load, then we replace the load
operation with a TypeGuard that guards the advertised type.

R=jarin@chromium.org
BUG=v8:5267

Review-Url: https://codereview.chromium.org/2295643002
Cr-Commit-Position: refs/heads/master@{#39032}

src/compiler/load-elimination.cc

@@ -4,6 +4,7 @@
 
 #include "src/compiler/load-elimination.h"
 
+#include "src/compiler/common-operator.h"
 #include "src/compiler/js-graph.h"
 #include "src/compiler/node-properties.h"
 #include "src/compiler/simplified-operator.h"
@@ -521,16 +522,21 @@ Reduction LoadElimination::ReduceLoadField(Node* node) {
   FieldAccess const& access = FieldAccessOf(node->op());
   Node* const object = NodeProperties::GetValueInput(node, 0);
   Node* const effect = NodeProperties::GetEffectInput(node);
+  Node* const control = NodeProperties::GetControlInput(node);
   AbstractState const* state = node_states_.Get(effect);
   if (state == nullptr) return NoChange();
   int field_index = FieldIndexOf(access);
   if (field_index >= 0) {
-    if (Node* const replacement = state->LookupField(object, field_index)) {
-      // Make sure the {replacement} has at least as good type
-      // as the original {node}.
-      if (!replacement->IsDead() &&
-          NodeProperties::GetType(replacement)
-              ->Is(NodeProperties::GetType(node))) {
+    if (Node* replacement = state->LookupField(object, field_index)) {
+      // Make sure we don't resurrect dead {replacement} nodes.
+      if (!replacement->IsDead()) {
+        // We might need to guard the {replacement} if the type of the
+        // {node} is more precise than the type of the {replacement}.
+        Type* const node_type = NodeProperties::GetType(node);
+        if (!NodeProperties::GetType(replacement)->Is(node_type)) {
+          replacement = graph()->NewNode(common()->TypeGuard(node_type),
+                                         replacement, control);
+        }
         ReplaceWithValue(node, replacement, effect);
         return Replace(replacement);
       }
@@ -568,14 +574,19 @@ Reduction LoadElimination::ReduceLoadElement(Node* node) {
   Node* const object = NodeProperties::GetValueInput(node, 0);
   Node* const index = NodeProperties::GetValueInput(node, 1);
   Node* const effect = NodeProperties::GetEffectInput(node);
+  Node* const control = NodeProperties::GetControlInput(node);
   AbstractState const* state = node_states_.Get(effect);
   if (state == nullptr) return NoChange();
-  if (Node* const replacement = state->LookupElement(object, index)) {
-    // Make sure the {replacement} has at least as good type
-    // as the original {node}.
-    if (!replacement->IsDead() &&
-        NodeProperties::GetType(replacement)
-            ->Is(NodeProperties::GetType(node))) {
+  if (Node* replacement = state->LookupElement(object, index)) {
+    // Make sure we don't resurrect dead {replacement} nodes.
+    if (!replacement->IsDead()) {
+      // We might need to guard the {replacement} if the type of the
+      // {node} is more precise than the type of the {replacement}.
+      Type* const node_type = NodeProperties::GetType(node);
+      if (!NodeProperties::GetType(replacement)->Is(node_type)) {
+        replacement = graph()->NewNode(common()->TypeGuard(node_type),
+                                       replacement, control);
+      }
       ReplaceWithValue(node, replacement, effect);
       return Replace(replacement);
     }
@@ -805,6 +816,12 @@ int LoadElimination::FieldIndexOf(FieldAccess const& access) {
   return field_index;
 }
 
+CommonOperatorBuilder* LoadElimination::common() const {
+  return jsgraph()->common();
+}
+
+Graph* LoadElimination::graph() const { return jsgraph()->graph(); }
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

src/compiler/load-elimination.h

@@ -12,7 +12,9 @@ namespace internal {
 namespace compiler {
 
 // Foward declarations.
+class CommonOperatorBuilder;
 struct FieldAccess;
+class Graph;
 class JSGraph;
 
 class LoadElimination final : public AdvancedReducer {
@@ -206,7 +208,9 @@ class LoadElimination final : public AdvancedReducer {
 
   static int FieldIndexOf(FieldAccess const& access);
 
+  CommonOperatorBuilder* common() const;
   AbstractState const* empty_state() const { return &empty_state_; }
+  Graph* graph() const;
   JSGraph* jsgraph() const { return jsgraph_; }
   Zone* zone() const { return node_states_.zone(); }
 

src/compiler/simplified-lowering.cc

@@ -878,11 +878,13 @@ class RepresentationSelector {
       bool is_word64 = GetInfo(node->InputAt(0))->representation() ==
                        MachineRepresentation::kWord64;
 #ifdef DEBUG
-      // Check that all the inputs agree on being Word64.
-      DCHECK_EQ(IrOpcode::kPhi, node->opcode());  // This only works for phis.
-      for (int i = 1; i < node->op()->ValueInputCount(); i++) {
-        DCHECK_EQ(is_word64, GetInfo(node->InputAt(i))->representation() ==
-                                 MachineRepresentation::kWord64);
+      if (node->opcode() != IrOpcode::kTypeGuard) {
+        // Check that all the inputs agree on being Word64.
+        DCHECK_EQ(IrOpcode::kPhi, node->opcode());  // This only works for phis.
+        for (int i = 1; i < node->op()->ValueInputCount(); i++) {
+          DCHECK_EQ(is_word64, GetInfo(node->InputAt(i))->representation() ==
+                                   MachineRepresentation::kWord64);
+        }
       }
 #endif
       return is_word64 ? MachineRepresentation::kWord64

test/unittests/compiler/load-elimination-unittest.cc

@@ -359,6 +359,64 @@ TEST_F(LoadEliminationTest, LoadFieldOnTrueBranchOfDiamond) {
   EXPECT_EQ(load, r.replacement());
 }
 
+TEST_F(LoadEliminationTest, LoadFieldWithTypeMismatch) {
+  Node* object = Parameter(Type::Any(), 0);
+  Node* value = Parameter(Type::Signed32(), 1);
+  Node* effect = graph()->start();
+  Node* control = graph()->start();
+  FieldAccess const access = {kTaggedBase,
+                              kPointerSize,
+                              MaybeHandle<Name>(),
+                              Type::Unsigned31(),
+                              MachineType::AnyTagged(),
+                              kNoWriteBarrier};
+
+  StrictMock<MockAdvancedReducerEditor> editor;
+  LoadElimination load_elimination(&editor, jsgraph(), zone());
+
+  load_elimination.Reduce(graph()->start());
+
+  Node* store = effect = graph()->NewNode(simplified()->StoreField(access),
+                                          object, value, effect, control);
+  load_elimination.Reduce(effect);
+
+  Node* load = graph()->NewNode(simplified()->LoadField(access), object, effect,
+                                control);
+  EXPECT_CALL(editor,
+              ReplaceWithValue(load, IsTypeGuard(value, control), store, _));
+  Reduction r = load_elimination.Reduce(load);
+  ASSERT_TRUE(r.Changed());
+  EXPECT_THAT(r.replacement(), IsTypeGuard(value, control));
+}
+
+TEST_F(LoadEliminationTest, LoadElementWithTypeMismatch) {
+  Node* object = Parameter(Type::Any(), 0);
+  Node* index = Parameter(Type::UnsignedSmall(), 1);
+  Node* value = Parameter(Type::Signed32(), 2);
+  Node* effect = graph()->start();
+  Node* control = graph()->start();
+  ElementAccess const access = {kTaggedBase, kPointerSize, Type::Unsigned31(),
+                                MachineType::AnyTagged(), kNoWriteBarrier};
+
+  StrictMock<MockAdvancedReducerEditor> editor;
+  LoadElimination load_elimination(&editor, jsgraph(), zone());
+
+  load_elimination.Reduce(graph()->start());
+
+  Node* store = effect =
+      graph()->NewNode(simplified()->StoreElement(access), object, index, value,
+                       effect, control);
+  load_elimination.Reduce(effect);
+
+  Node* load = graph()->NewNode(simplified()->LoadElement(access), object,
+                                index, effect, control);
+  EXPECT_CALL(editor,
+              ReplaceWithValue(load, IsTypeGuard(value, control), store, _));
+  Reduction r = load_elimination.Reduce(load);
+  ASSERT_TRUE(r.Changed());
+  EXPECT_THAT(r.replacement(), IsTypeGuard(value, control));
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8

test/unittests/compiler/node-test-utils.cc

@@ -406,6 +406,35 @@ class IsTerminateMatcher final : public NodeMatcher {
   const Matcher<Node*> control_matcher_;
 };
 
+class IsTypeGuardMatcher final : public NodeMatcher {
+ public:
+  IsTypeGuardMatcher(const Matcher<Node*>& value_matcher,
+                     const Matcher<Node*>& control_matcher)
+      : NodeMatcher(IrOpcode::kTypeGuard),
+        value_matcher_(value_matcher),
+        control_matcher_(control_matcher) {}
+
+  void DescribeTo(std::ostream* os) const final {
+    NodeMatcher::DescribeTo(os);
+    *os << " whose value (";
+    value_matcher_.DescribeTo(os);
+    *os << ") and control (";
+    control_matcher_.DescribeTo(os);
+    *os << ")";
+  }
+
+  bool MatchAndExplain(Node* node, MatchResultListener* listener) const final {
+    return (NodeMatcher::MatchAndExplain(node, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetValueInput(node, 0),
+                                 "value", value_matcher_, listener) &&
+            PrintMatchAndExplain(NodeProperties::GetControlInput(node),
+                                 "control", control_matcher_, listener));
+  }
+
+ private:
+  const Matcher<Node*> value_matcher_;
+  const Matcher<Node*> control_matcher_;
+};
 
 template <typename T>
 class IsConstantMatcher final : public NodeMatcher {
@@ -1714,6 +1743,10 @@ Matcher<Node*> IsTerminate(const Matcher<Node*>& effect_matcher,
   return MakeMatcher(new IsTerminateMatcher(effect_matcher, control_matcher));
 }
 
+Matcher<Node*> IsTypeGuard(const Matcher<Node*>& value_matcher,
+                           const Matcher<Node*>& control_matcher) {
+  return MakeMatcher(new IsTypeGuardMatcher(value_matcher, control_matcher));
+}
 
 Matcher<Node*> IsExternalConstant(
     const Matcher<ExternalReference>& value_matcher) {

test/unittests/compiler/node-test-utils.h

@@ -83,6 +83,8 @@ Matcher<Node*> IsReturn2(const Matcher<Node*>& value_matcher,
                          const Matcher<Node*>& control_matcher);
 Matcher<Node*> IsTerminate(const Matcher<Node*>& effect_matcher,
                            const Matcher<Node*>& control_matcher);
+Matcher<Node*> IsTypeGuard(const Matcher<Node*>& value_matcher,
+                           const Matcher<Node*>& control_matcher);
 Matcher<Node*> IsExternalConstant(
     const Matcher<ExternalReference>& value_matcher);
 Matcher<Node*> IsHeapConstant(Handle<HeapObject> value);