commit 1e5b80918a186997951c00e98acea906d089c7c4
author Stephen Adams <sra@google.com> Fri Sep 21 23:47:50 2018 +0000
committer commit-bot@chromium.org <commit-bot@chromium.org> Fri Sep 21 23:47:50 2018 +0000
tree 6d630f8b305f4e5466a7dce817fdfe6f807475c9
parent 654c4babc8355dbc53d5575327c60d95ed6ff8ed

[dart2js] hashCode stability to improve repeatability

The default Object.hashCode is different on every run, so avoid using it for MemberEntity.hashCode.

Using a consistent (if arbitrary) hashCode appears to fix issue 34527
and also makes the *.js.map files consistent.
We also sort instructions in SsaCodeMotion to make order less sensitive to unrelated changes.

We might still want to impose a canonical ordering for map files, e.g. minified map file entries might be sorted by name.

Change-Id: I94ad6c1e18ac808b92e62e76b48558a7e2fbd7b7
Reviewed-on: https://dart-review.googlesource.com/75982
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Stephen Adams <sra@google.com>

pkg/compiler/lib/src/elements/indexed.dart

diff --git a/pkg/compiler/lib/src/elements/indexed.dart b/pkg/compiler/lib/src/elements/indexed.dart
index 28545af..f20a4ffab 100644
--- a/pkg/compiler/lib/src/elements/indexed.dart
+++ b/pkg/compiler/lib/src/elements/indexed.dart
@@ -13,16 +13,19 @@
 abstract class IndexedLibrary extends _Indexed implements LibraryEntity {
   /// Library index used for fast lookup in [KernelToElementMapBase].
   int get libraryIndex => _index;
+  int get hashCode => 7 * _index + 2;
 }
 
 abstract class IndexedClass extends _Indexed implements ClassEntity {
   /// Class index used for fast lookup in [KernelToElementMapBase].
   int get classIndex => _index;
+  int get hashCode => 7 * _index + 1;
 }
 
 abstract class IndexedMember extends _Indexed implements MemberEntity {
   /// Member index used for fast lookup in [KernelToElementMapBase].
   int get memberIndex => _index;
+  int get hashCode => 7 * _index;
 }
 
 abstract class IndexedFunction extends _Indexed

pkg/compiler/lib/src/ssa/optimize.dart

diff --git a/pkg/compiler/lib/src/ssa/optimize.dart b/pkg/compiler/lib/src/ssa/optimize.dart
index 22a3003..104c919 100644
--- a/pkg/compiler/lib/src/ssa/optimize.dart
+++ b/pkg/compiler/lib/src/ssa/optimize.dart
@@ -2400,6 +2400,11 @@
 
       if (!instructions.isEmpty) {
         List<HInstruction> list = instructions.toList();
+        // Sort by instruction 'id' for more stable ordering under changes to
+        // unrelated source code. 'id' is a function of the operations of
+        // compiling the current method, whereas the ValueSet order is dependent
+        // hashCodes that are a function of the whole program.
+        list.sort((insn1, insn2) => insn1.id.compareTo(insn2.id));
         for (HInstruction instruction in list) {
           // Move the instruction to the current block.
           instruction.block.detach(instruction);
@@ -2416,6 +2421,10 @@
           }
         }
       }
+      // TODO(sra): There are some non-gvn-able instructions that we could move,
+      // e.g. allocations. We should probably not move instructions that can
+      // directly or indirectly throw since the reported location might be in
+      // the 'wrong' branch.
     }
 
     // Don't try to merge instructions to a dominator if we have
@@ -2445,6 +2454,8 @@
         if (input.block == block) {
           canBeMoved = false;
           break;
+          // TODO(sra): We could move trees of instructions provided we move the
+          // roots before the leaves.
         }
       }
       if (!canBeMoved) continue;