[dart2js] new-rti: fix TypeEval GVN

rakudrama · commit-bot@chromium.org · commit 9197c1b021b6 · 2019-07-22T23:32:00.000Z
Type expression GVN needs to structural equality on TypeRecipe and TypeEnvironmentStructure. Change-Id: Iaa4985ec99fff6db29e8bfd63fd0dbacdc10dde6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/109942 Reviewed-by: Mayank Patke <fishythefish@google.com> Commit-Queue: Stephen Adams <sra@google.com>
diff --git a/pkg/compiler/lib/src/js_model/type_recipe.dart b/pkg/compiler/lib/src/js_model/type_recipe.dart
@@ -12,7 +12,30 @@ import '../elements/types.dart';
 /// A type environment maps type parameter variables to type values. The type
 /// variables are mostly elided in the runtime representation, replaced by
 /// indexes into the reified environment.
-abstract class TypeEnvironmentStructure {}
+abstract class TypeEnvironmentStructure {
+  /// Structural equality on [TypeEnvironmentStructure].
+  static bool same(TypeEnvironmentStructure a, TypeEnvironmentStructure b) {
+    if (a is SingletonTypeEnvironmentStructure) {
+      if (b is SingletonTypeEnvironmentStructure) {
+        return a.variable == b.variable;
+      }
+      return false;
+    }
+    return _sameFullStructure(a, b);
+  }
+
+  static bool _sameFullStructure(
+      FullTypeEnvironmentStructure a, FullTypeEnvironmentStructure b) {
+    if (a.classType != b.classType) return false;
+    List<TypeVariableType> aBindings = a.bindings;
+    List<TypeVariableType> bBindings = b.bindings;
+    if (aBindings.length != bBindings.length) return false;
+    for (int i = 0; i < aBindings.length; i++) {
+      if (aBindings[i] != bBindings[i]) return false;
+    }
+    return true;
+  }
+}
 
 /// A singleton type environment maps a binds a single value.
 class SingletonTypeEnvironmentStructure extends TypeEnvironmentStructure {
@@ -38,14 +61,37 @@ class FullTypeEnvironmentStructure extends TypeEnvironmentStructure {
 
 /// A TypeRecipe is evaluated against a type environment to produce either a
 /// type, or another type environment.
-abstract class TypeRecipe {}
+abstract class TypeRecipe {
+  /// Returns `true` is [recipeB] evaluated in an environment described by
+  /// [structureB] gives the same type as [recipeA] evaluated in environment
+  /// described by [structureA].
+  static bool yieldsSameType(
+      TypeRecipe recipeA,
+      TypeEnvironmentStructure structureA,
+      TypeRecipe recipeB,
+      TypeEnvironmentStructure structureB) {
+    if (recipeA == recipeB &&
+        TypeEnvironmentStructure.same(structureA, structureB)) {
+      return true;
+    }
+
+    // TODO(sra): Type recipes that are different but equal modulo naming also
+    // yield the same type, e.g. `List<X> @X` and `List<Y> @Y`.
+    return false;
+  }
+}
 
 /// A recipe that yields a reified type.
 class TypeExpressionRecipe extends TypeRecipe {
   final DartType type;
 
   TypeExpressionRecipe(this.type);
 
+  @override
+  bool operator ==(other) {
+    return other is TypeExpressionRecipe && type == other.type;
+  }
+
   @override
   String toString() => 'TypeExpressionRecipe($type)';
 }
@@ -60,6 +106,11 @@ class SingletonTypeEnvironmentRecipe extends TypeEnvironmentRecipe {
 
   SingletonTypeEnvironmentRecipe(this.type);
 
+  @override
+  bool operator ==(other) {
+    return other is SingletonTypeEnvironmentRecipe && type == other.type;
+  }
+
   @override
   String toString() => 'SingletonTypeEnvironmentRecipe($type)';
 }
@@ -80,6 +131,22 @@ class FullTypeEnvironmentRecipe extends TypeEnvironmentRecipe {
 
   FullTypeEnvironmentRecipe({this.classType, this.types = const []});
 
+  @override
+  bool operator ==(other) {
+    return other is FullTypeEnvironmentRecipe && _equal(this, other);
+  }
+
+  static bool _equal(FullTypeEnvironmentRecipe a, FullTypeEnvironmentRecipe b) {
+    if (a.classType != b.classType) return false;
+    List<DartType> aTypes = a.types;
+    List<DartType> bTypes = b.types;
+    if (aTypes.length != bTypes.length) return false;
+    for (int i = 0; i < aTypes.length; i++) {
+      if (aTypes[i] != bTypes[i]) return false;
+    }
+    return true;
+  }
+
   @override
   String toString() => 'FullTypeEnvironmentRecipe($classType, $types)';
 }
diff --git a/pkg/compiler/lib/src/ssa/nodes.dart b/pkg/compiler/lib/src/ssa/nodes.dart
@@ -4521,7 +4521,8 @@ class HTypeEval extends HRtiInstruction {
 
   @override
   bool dataEquals(HTypeEval other) {
-    return typeExpression == other.typeExpression;
+    return TypeRecipe.yieldsSameType(
+        typeExpression, envStructure, other.typeExpression, other.envStructure);
   }
 
   @override
diff --git a/pkg/compiler/lib/src/ssa/ssa_tracer.dart b/pkg/compiler/lib/src/ssa/ssa_tracer.dart
@@ -708,7 +708,7 @@ class HInstructionStringifier implements HVisitor<String> {
   @override
   String visitTypeEval(HTypeEval node) {
     var inputs = node.inputs.map(temporaryId).join(', ');
-    return "TypeEval: ${node.typeExpression}  $inputs";
+    return "TypeEval: ${node.typeExpression}  ${node.envStructure}  $inputs";
   }
 
   @override

Original file line number	Diff line number	Diff line change
`@@ -4521,7 +4521,8 @@ class HTypeEval extends HRtiInstruction {`
`4521`	`4521`
`4522`	`4522`	`@override`
`4523`	`4523`	`bool dataEquals(HTypeEval other) {`
`4524`		`- return typeExpression == other.typeExpression;`
	`4524`	`+ return TypeRecipe.yieldsSameType(`
	`4525`	`+ typeExpression, envStructure, other.typeExpression, other.envStructure);`
`4525`	`4526`	`}`
`4526`	`4527`
`4527`	`4528`	`@override`
Original file line number	Diff line number	Diff line change
`@@ -708,7 +708,7 @@ class HInstructionStringifier implements HVisitor<String> {`
`708`	`708`	`@override`
`709`	`709`	`String visitTypeEval(HTypeEval node) {`
`710`	`710`	`var inputs = node.inputs.map(temporaryId).join(', ');`
`711`		`- return "TypeEval: ${node.typeExpression} $inputs";`
	`711`	`+ return "TypeEval: ${node.typeExpression} ${node.envStructure} $inputs";`
`712`	`712`	`}`
`713`	`713`
`714`	`714`	`@override`