[vm] Walk Functions as well as Code objects in DedupInstructions.

Previously, we used the owner field to find the function whose entry
points to fix up after deduplicating instructions/code objects. However,
this means retrieving and checking the owning Function for each Code
object that has one, even if that Code object isn't the current code
object for the owning function. Instead, now we extend the instruction
deduplication visitors to Functions, where visiting a Function means
visiting its active Code object before fixing up the entry point cache.

Since the ProgramWalker also visits Code objects, this means visiting
some Code objects twice (once in the ProgramWalker and once when
visiting the Function for which it is the current code). However, since
the deduplication process is idempotent, the additional visits will not
cause the entry points for the Code object to change further (which
would otherwise invalidate the caches of already visited Functions).
Thus, the order of the visits does not affect the results.

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-product-x64-try
Change-Id: Idd12fc94c5fb98f1fc9eb669d09b47822b0eac1e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/144700
Reviewed-by: Daco Harkes <[email protected]>
Commit-Queue: Tess Strickland <[email protected]>

Author: sstrickl

runtime/vm/program_visitor.cc

@@ -1164,7 +1164,7 @@ void ProgramVisitor::DedupInstructions(Zone* zone, Isolate* isolate) {
    public:
     explicit DedupInstructionsVisitor(Zone* zone)
         : Dedupper(zone),
-          function_(Function::Handle(zone)),
+          code_(Code::Handle(zone)),
           instructions_(Instructions::Handle(zone)) {
       if (Snapshot::IncludesCode(Dart::vm_snapshot_kind())) {
         // Prefer existing objects in the VM isolate.
@@ -1178,6 +1178,16 @@ void ProgramVisitor::DedupInstructions(Zone* zone, Isolate* isolate) {
       AddCanonical(instructions_);
     }
 
+    void VisitFunction(const Function& function) {
+      if (!function.HasCode()) return;
+      code_ = function.CurrentCode();
+      // This causes the code to be visited once here and once directly in the
+      // ProgramWalker, but as long as the deduplication process is idempotent,
+      // the cached entry points won't change during the second visit.
+      VisitCode(code_);
+      function.SetInstructions(code_);  // Update cached entry point.
+    }
+
     void VisitCode(const Code& code) {
       instructions_ = code.instructions();
       instructions_ = Dedup(instructions_);
@@ -1188,37 +1198,32 @@ void ProgramVisitor::DedupInstructions(Zone* zone, Isolate* isolate) {
       }
       code.SetActiveInstructions(instructions_,
                                  code.UncheckedEntryPointOffset());
-      if (!code.IsFunctionCode()) return;
-      function_ = code.function();
-      if (function_.IsNull()) return;
-      // Make sure that this is the code object currently installed for the
-      // function before we update the entry points.
-      if (!function_.HasCode()) return;
-      if (function_.CurrentCode() != code.raw()) return;
-      function_.SetInstructions(code);  // Update cached entry point.
     }
 
    private:
-    Function& function_;
+    Code& code_;
     Instructions& instructions_;
   };
 
 #if defined(DART_PRECOMPILER)
   class DedupInstructionsWithSameMetadataVisitor
       : public CodeVisitor,
-        public Dedupper<Code, CodeKeyValueTrait>,
-        public ObjectVisitor {
+        public Dedupper<Code, CodeKeyValueTrait> {
    public:
     explicit DedupInstructionsWithSameMetadataVisitor(Zone* zone)
         : Dedupper(zone),
           canonical_(Code::Handle(zone)),
-          function_(Function::Handle(zone)),
+          code_(Code::Handle(zone)),
           instructions_(Instructions::Handle(zone)) {}
 
-    void VisitObject(RawObject* obj) {
-      if (!obj->IsCode()) return;
-      canonical_ = Code::RawCast(obj);
-      AddCanonical(canonical_);
+    void VisitFunction(const Function& function) {
+      if (!function.HasCode()) return;
+      code_ = function.CurrentCode();
+      // This causes the code to be visited once here and once directly in the
+      // ProgramWalker, but as long as the deduplication process is idempotent,
+      // the cached entry points won't change during the second visit.
+      VisitCode(code_);
+      function.SetInstructions(code_);  // Update cached entry point.
     }
 
     void VisitCode(const Code& code) {
@@ -1228,17 +1233,13 @@ void ProgramVisitor::DedupInstructions(Zone* zone, Isolate* isolate) {
       code.SetActiveInstructions(instructions_,
                                  code.UncheckedEntryPointOffset());
       code.set_instructions(instructions_);
-      if (!code.IsFunctionCode()) return;
-      function_ = code.function();
-      if (function_.IsNull()) return;
-      function_.SetInstructions(code);  // Update cached entry point.
     }
 
    private:
     bool CanCanonicalize(const Code& code) const { return !code.IsDisabled(); }
 
     Code& canonical_;
-    Function& function_;
+    Code& code_;
     Instructions& instructions_;
   };