[turbofan] Keep sharing when merging into dead environment

Also make return and unconditional jumps kill the environment instead
of clearing it. This was still leftover from before we introduced
liveness and prevented sharing as well.

Bug: v8:7790
Change-Id: Ic79d64c9eaedf608d26e3265d4b27d21f7f3dfe1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1948710
Auto-Submit: Georg Neis <neis@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65345}

src/compiler/serializer-for-background-compilation.cc

@@ -909,14 +909,15 @@ void SerializerForBackgroundCompilation::Environment::Merge(Environment* other,
   SLOW_DCHECK(closure_hints_ == other->closure_hints_);
 
   if (IsDead()) {
-    // TODO(neis): Acquire existing hints rather than merge them into empty?
-    ephemeral_hints_.resize(other->ephemeral_hints_.size());
+    ephemeral_hints_ = other->ephemeral_hints_;
+  } else {
+    CHECK_EQ(ephemeral_hints_.size(), other->ephemeral_hints_.size());
+    for (size_t i = 0; i < ephemeral_hints_.size(); ++i) {
+      ephemeral_hints_[i].Merge(other->ephemeral_hints_[i], zone);
+    }
   }
 
-  CHECK_EQ(ephemeral_hints_.size(), other->ephemeral_hints_.size());
-  for (size_t i = 0; i < ephemeral_hints_.size(); ++i) {
-    ephemeral_hints_[i].Merge(other->ephemeral_hints_[i], zone);
-  }
+  // TODO(neis): Deal with context hints.
 
   CHECK(!IsDead());
 }
@@ -2666,7 +2667,7 @@ void SerializerForBackgroundCompilation::ProcessJump(
 void SerializerForBackgroundCompilation::VisitReturn(
     BytecodeArrayIterator* iterator) {
   return_value_hints().Add(environment()->accumulator_hints(), zone());
-  environment()->ClearEphemeralHints();
+  environment()->Kill();
 }
 
 void SerializerForBackgroundCompilation::VisitSwitchOnSmiNoFeedback(
@@ -3374,7 +3375,7 @@ CONDITIONAL_JUMPS_LIST(DEFINE_CONDITIONAL_JUMP)
   void SerializerForBackgroundCompilation::Visit##name( \
       BytecodeArrayIterator* iterator) {                \
     ProcessJump(iterator);                              \
-    environment()->ClearEphemeralHints();               \
+    environment()->Kill();                              \
   }
 UNCONDITIONAL_JUMPS_LIST(DEFINE_UNCONDITIONAL_JUMP)
 #undef DEFINE_UNCONDITIONAL_JUMP

test/mjsunit/compiler/serializer-dead-after-jump.js

+// Copyright 2019 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 --opt --no-always-opt
+
+function f(x) {
+  %TurbofanStaticAssert(x.foo === 42);
+  return %IsBeingInterpreted();
+}
+
+function main(b, ret) {
+  const x = new Object();
+  const y = x;
+  if (b) {
+    return ret;
+  } else {
+    x.foo = 42;
+    return f(y);
+  }
+}
+
+
+%PrepareFunctionForOptimization(f);
+%PrepareFunctionForOptimization(main);
+
+f({a: 1});
+f({b: 1});
+f({c: 1});
+f({d: 1});
+
+assertTrue(main(true, true));
+assertTrue(main(true, true));
+assertTrue(main(false, true));
+assertTrue(main(false, true));
+%OptimizeFunctionOnNextCall(main);
+assertFalse(main(false));

test/mjsunit/compiler/serializer-dead-after-return.js

+// Copyright 2019 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 --opt --no-always-opt
+
+function f(x) {
+  %TurbofanStaticAssert(x.foo === 42);
+  return %IsBeingInterpreted();
+}
+
+function main(b, ret) {
+  const x = new Object();
+  const y = x;
+  if (b) return ret;
+
+  x.foo = 42;
+  return f(y);
+}
+
+
+%PrepareFunctionForOptimization(f);
+%PrepareFunctionForOptimization(main);
+
+f({a: 1});
+f({b: 1});
+f({c: 1});
+f({d: 1});
+
+assertTrue(main(true, true));
+assertTrue(main(true, true));
+assertTrue(main(false, true));
+assertTrue(main(false, true));
+%OptimizeFunctionOnNextCall(main);
+assertFalse(main(false));

test/mjsunit/mjsunit.status

@@ -412,6 +412,8 @@
   'compiler/serializer-accessors': [SKIP],
   'compiler/serializer-apply': [SKIP],
   'compiler/serializer-call': [SKIP],
+  'compiler/serializer-dead-after-jump': [SKIP],
+  'compiler/serializer-dead-after-return': [SKIP],
   'compiler/serializer-transition-propagation': [SKIP],
 
   # These tests check that we can trace the compiler.
@@ -841,6 +843,8 @@
   'code-coverage-block-opt': [SKIP],
   'compiler/serializer-apply': [SKIP],
   'compiler/serializer-call': [SKIP],
+  'compiler/serializer-dead-after-jump': [SKIP],
+  'compiler/serializer-dead-after-return': [SKIP],
   'compiler/serializer-transition-propagation': [SKIP],
 
   # Bounds check triggers forced deopt for array constructors.
@@ -888,6 +892,8 @@
   'compiler/native-context-specialization-hole-check': [SKIP],
   'compiler/serializer-apply': [SKIP],
   'compiler/serializer-call': [SKIP],
+  'compiler/serializer-dead-after-jump': [SKIP],
+  'compiler/serializer-dead-after-return': [SKIP],
   'compiler/serializer-transition-propagation': [SKIP],
   'opt-elements-kind': [SKIP],
   'regress/regress-trap-allocation-memento': [SKIP],
@@ -1108,6 +1114,8 @@
   'compiler/serializer-accessors': [SKIP],
   'compiler/serializer-apply': [SKIP],
   'compiler/serializer-call': [SKIP],
+  'compiler/serializer-dead-after-jump': [SKIP],
+  'compiler/serializer-dead-after-return': [SKIP],
   'compiler/serializer-feedback-propagation-*': [SKIP],
   'compiler/serializer-transition-propagation': [SKIP],