[vm/compiler] Reland "Fix receiver redefinition type for poly inlining."

In cases where the same target function is used for multiple
CID ranges, we only perform the inlining once. However, if the
CID range used for the initial inlining is a single CID, we will
set the type of the redefinition to that CID.

Detect this case and clear the concrete type associated with
the redefinition if it was given one.

Change-Id: I9f9bdd7c21e0dc1ac537f8facece0010630bd9aa
Cq-Include-Trybots: luci.dart.try:vm-kernel-win-debug-x64-try,vm-kernel-win-release-ia32-try,vm-kernel-win-product-x64-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105221
Commit-Queue: Teagan Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>

runtime/vm/compiler/backend/inliner.cc

@@ -1731,6 +1731,11 @@ bool PolymorphicInliner::CheckInlinedDuplicate(const Function& target) {
           BlockEntryInstr* block = old_target->dominated_blocks()[j];
           new_join->AddDominatedBlock(block);
         }
+        // Since we are reusing the same inlined body across multiple cids,
+        // reset the type information on the redefinition of the receiver
+        // in case it was originally given a concrete type.
+        ASSERT(new_join->next()->IsRedefinition());
+        new_join->next()->AsRedefinition()->UpdateType(CompileType::Dynamic());
         // Create a new target with the join as unconditional successor.
         TargetEntryInstr* new_target = new TargetEntryInstr(
             AllocateBlockId(), old_target->try_index(), DeoptId::kNone);

runtime/vm/compiler/backend/inliner_test.cc

@@ -0,0 +1,137 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+#include "vm/compiler/backend/inliner.h"
+
+#include "vm/compiler/backend/il.h"
+#include "vm/compiler/backend/il_printer.h"
+#include "vm/compiler/backend/il_test_helper.h"
+#include "vm/compiler/compiler_pass.h"
+#include "vm/object.h"
+#include "vm/unit_test.h"
+
+namespace dart {
+
+// Test that the redefinition for an inlined polymorphic function used with
+// multiple receiver cids does not have a concrete type.
+ISOLATE_UNIT_TEST_CASE(Inliner_PolyInliningRedefinition) {
+  const char* kScript = R"(
+    abstract class A {
+      String toInline() { return "A"; }
+    }
+
+    class B extends A {}
+    class C extends A {
+      @override
+      String toInline() { return "C";}
+    }
+    class D extends A {}
+
+    testInlining(A arg) {
+      arg.toInline();
+    }
+
+    main() {
+      for (var i = 0; i < 10; i++) {
+        testInlining(B());
+        testInlining(C());
+        testInlining(D());
+      }
+    }
+  )";
+
+  const auto& root_library = Library::Handle(LoadTestScript(kScript));
+  const auto& function =
+      Function::Handle(GetFunction(root_library, "testInlining"));
+
+  Invoke(root_library, "main");
+
+  TestPipeline pipeline(function, CompilerPass::kJIT);
+  FlowGraph* flow_graph = pipeline.RunPasses({
+      CompilerPass::kComputeSSA,
+      CompilerPass::kApplyICData,
+      CompilerPass::kTryOptimizePatterns,
+      CompilerPass::kSetOuterInliningId,
+      CompilerPass::kTypePropagation,
+      CompilerPass::kApplyClassIds,
+      CompilerPass::kInlining,
+  });
+
+  auto entry = flow_graph->graph_entry()->normal_entry();
+  EXPECT(entry != nullptr);
+
+  EXPECT(entry->initial_definitions()->length() == 1);
+  EXPECT(entry->initial_definitions()->At(0)->IsParameter());
+  ParameterInstr* param = entry->initial_definitions()->At(0)->AsParameter();
+
+  // First we find the start of the prelude for the inlined instruction,
+  // and also keep a reference to the LoadClassId instruction for later.
+  LoadClassIdInstr* lcid = nullptr;
+  BranchInstr* prelude = nullptr;
+
+  ILMatcher cursor(flow_graph, entry);
+  RELEASE_ASSERT(cursor.TryMatch(
+      {
+          {kMatchLoadClassId, &lcid},
+          {kMatchBranch, &prelude},
+      },
+      /*insert_before=*/kMoveGlob));
+
+  const Class& cls = Class::Handle(
+      root_library.LookupLocalClass(String::Handle(Symbols::New(thread, "B"))));
+
+  Definition* cid_B = flow_graph->GetConstant(Smi::Handle(Smi::New(cls.id())));
+  Instruction* current = prelude;
+
+  // We walk false branches until we either reach a branch instruction that uses
+  // B's cid for comparison to the value returned from the LCID instruction
+  // above, or a default case if there was no branch instruction for B's cid.
+  while (true) {
+    EXPECT(current->IsBranch());
+    const ComparisonInstr* check = current->AsBranch()->comparison();
+    EXPECT(check->left()->definition() == lcid);
+    if (check->right()->definition() == cid_B) break;
+    current = current->SuccessorAt(1);
+    // By following false paths, we should be walking a series of blocks that
+    // looks like:
+    // B#[target]:#
+    //   Branch if <check on class ID>
+    // If we end up not finding a branch, then we're in a default case
+    // that contains a class check.
+    current = current->next();
+    if (!current->IsBranch()) {
+      break;
+    }
+  }
+  // If we found a branch that checks against the class ID, we follow the true
+  // branch to a block that contains only a goto to the desired join block.
+  if (current->IsBranch()) {
+    current = current->SuccessorAt(0);
+  } else {
+    // We're in the default case, which will check the class ID to make sure
+    // it's the one expected for the fallthrough. That check will be followed
+    // by a goto to the desired join block.
+    EXPECT(current->IsRedefinition());
+    const auto redef = current->AsRedefinition();
+    EXPECT(redef->value()->definition() == lcid);
+    current = current->next();
+    EXPECT(current->IsCheckClassId());
+    EXPECT(current->AsCheckClassId()->value()->definition() == redef);
+  }
+  current = current->next();
+  EXPECT(current->IsGoto());
+  current = current->AsGoto()->successor();
+  // Now we should be at a block that starts like:
+  // BY[join]:# pred(...)
+  //    vW <- Redefinition(vV)
+  //
+  // where vV is a reference to the function parameter (the receiver of
+  // the inlined function).
+  current = current->next();
+  EXPECT(current->IsRedefinition());
+  EXPECT(current->AsRedefinition()->value()->definition() == param);
+  EXPECT(current->AsRedefinition()->Type()->ToCid() == kDynamicCid);
+}
+
+}  // namespace dart

runtime/vm/compiler/compiler_sources.gni

@@ -168,6 +168,7 @@ compiler_sources_tests = [
   "backend/il_test.cc",
   "backend/il_test_helper.h",
   "backend/il_test_helper.cc",
+  "backend/inliner_test.cc",
   "backend/locations_helpers_test.cc",
   "backend/loops_test.cc",
   "backend/range_analysis_test.cc",