[vm/compiler] handle non-nullable null situation better

Rationale:
Previously, we avoided introducing redefinitions that
introduced the empty non-nullable null type. This situation
arises when we do a null check on an actual null value
(making all subsequent uses effectively dead code). This
is too simple, however, since it still allows hoisting the
uses before the check. This CL gives a better solution
by introducing redefinitions without a constraining type
(which are not removed and avoid the type). In the long
run perhaps the best solution would be to simply remove
all subsequent uses as dead.

#32167
#34473
#35335

Change-Id: Ib5dd072a9e546f6b91faa52ea08e8c0f6350d7e0
Reviewed-on: https://dart-review.googlesource.com/c/89922
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Aart Bik <ajcbik@google.com>

runtime/vm/compiler/backend/compile_type.h

@@ -80,7 +80,13 @@ class CompileType : public ZoneAllocated {
   // abstract type. The pair is assumed to be coherent.
   static CompileType Create(intptr_t cid, const AbstractType& type);
 
-  CompileType CopyNonNullable() const {
+  // Return the non-nullable version of this type.
+  CompileType CopyNonNullable() {
+    if (IsNull()) {
+      // Represent a non-nullable null type (typically arising for
+      // unreachable values) as None.
+      return None();
+    }
     return CompileType(kNonNullable, kIllegalCid, type_);
   }
 

runtime/vm/compiler/backend/flow_graph.cc

@@ -1559,7 +1559,13 @@ RedefinitionInstr* FlowGraph::EnsureRedefinition(Instruction* prev,
     }
   }
   RedefinitionInstr* redef = new RedefinitionInstr(new Value(original));
-  redef->set_constrained_type(new CompileType(compile_type));
+
+  // Don't set the constrained type when the type is None(), which denotes an
+  // unreachable value (e.g. using value null after an explicit null check).
+  if (!compile_type.IsNone()) {
+    redef->set_constrained_type(new CompileType(compile_type));
+  }
+
   InsertAfter(prev, redef, NULL, FlowGraph::kValue);
   RenameDominatedUses(original, redef, redef);
   return redef;

runtime/vm/compiler/backend/type_propagator.cc

@@ -283,7 +283,7 @@ void FlowGraphTypePropagator::VisitCheckClassId(CheckClassIdInstr* check) {
 void FlowGraphTypePropagator::VisitCheckNull(CheckNullInstr* check) {
   Definition* receiver = check->value()->definition();
   CompileType* type = TypeOf(receiver);
-  if (type->is_nullable() && !type->IsNull()) {
+  if (type->is_nullable()) {
     // Insert redefinition for the receiver to guard against invalid
     // code motion.
     EnsureMoreAccurateRedefinition(check, receiver, type->CopyNonNullable());
@@ -305,7 +305,7 @@ void FlowGraphTypePropagator::CheckNonNullSelector(
   if (target.IsNull()) {
     // If the selector is not defined on Null, we can propagate non-nullness.
     CompileType* type = TypeOf(receiver);
-    if (type->is_nullable() && !type->IsNull()) {
+    if (type->is_nullable()) {
       // Insert redefinition for the receiver to guard against invalid
       // code motion.
       EnsureMoreAccurateRedefinition(call, receiver, type->CopyNonNullable());