[dart2js] Try to preserve precision of bools in inferrer.

Use abstract type of constant for BoolConstantTypeInformation
as the AbstractValueDomain may have something more precise than 'bool'
for a true/false constant.

Evaluate short-circuit `&&` and `||` for constant inputs.
This can occur with named and -Dxxx= defined constants.

TODO: we should consider adding a node like a guarded-phi to
select an input, with possible boolean conversions on inputs.



Change-Id: I7f54503db87f6a6d2034ca59c16b4f1ff69ebbb6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/159323
Reviewed-by: Christian Altamirano <coam@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Stephen Adams <sra@google.com>

pkg/compiler/lib/src/inferrer/builder_kernel.dart

@@ -1432,11 +1432,12 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation> {
     return thisType;
   }
 
-  void handleCondition(ir.Node node) {
+  TypeInformation handleCondition(ir.Node node) {
     bool oldAccumulateIsChecks = _accumulateIsChecks;
     _accumulateIsChecks = true;
-    visit(node, conditionContext: true);
+    TypeInformation result = visit(node, conditionContext: true);
     _accumulateIsChecks = oldAccumulateIsChecks;
+    return result;
   }
 
   void _potentiallyAddIsCheck(ir.IsExpression node) {
@@ -1500,6 +1501,8 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation> {
     LocalState stateAfterOperandWhenFalse = _stateAfterWhenFalse;
     _setStateAfter(
         _state, stateAfterOperandWhenFalse, stateAfterOperandWhenTrue);
+    // TODO(sra): Improve precision on constant and bool-conversion-to-constant
+    // inputs.
     return _types.boolType;
   }
 
@@ -1508,11 +1511,11 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation> {
     if (node.operator == '&&') {
       LocalState stateBefore = _state;
       _state = new LocalState.childPath(stateBefore);
-      handleCondition(node.left);
+      TypeInformation leftInfo = handleCondition(node.left);
       LocalState stateAfterLeftWhenTrue = _stateAfterWhenTrue;
       LocalState stateAfterLeftWhenFalse = _stateAfterWhenFalse;
       _state = new LocalState.childPath(stateAfterLeftWhenTrue);
-      handleCondition(node.right);
+      TypeInformation rightInfo = handleCondition(node.right);
       LocalState stateAfterRightWhenTrue = _stateAfterWhenTrue;
       LocalState stateAfterRightWhenFalse = _stateAfterWhenFalse;
       LocalState stateAfterWhenTrue = stateAfterRightWhenTrue;
@@ -1522,15 +1525,22 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation> {
       LocalState after = stateBefore.mergeDiamondFlow(
           _inferrer, stateAfterWhenTrue, stateAfterWhenFalse);
       _setStateAfter(after, stateAfterWhenTrue, stateAfterWhenFalse);
+      // Constant-fold result.
+      if (_types.isLiteralFalse(leftInfo)) return leftInfo;
+      if (_types.isLiteralTrue(leftInfo)) {
+        if (_types.isLiteralFalse(rightInfo)) return rightInfo;
+        if (_types.isLiteralTrue(rightInfo)) return rightInfo;
+      }
+      // TODO(sra): Add a selector/mux node to improve precision.
       return _types.boolType;
     } else if (node.operator == '||') {
       LocalState stateBefore = _state;
       _state = new LocalState.childPath(stateBefore);
-      handleCondition(node.left);
+      TypeInformation leftInfo = handleCondition(node.left);
       LocalState stateAfterLeftWhenTrue = _stateAfterWhenTrue;
       LocalState stateAfterLeftWhenFalse = _stateAfterWhenFalse;
       _state = new LocalState.childPath(stateAfterLeftWhenFalse);
-      handleCondition(node.right);
+      TypeInformation rightInfo = handleCondition(node.right);
       LocalState stateAfterRightWhenTrue = _stateAfterWhenTrue;
       LocalState stateAfterRightWhenFalse = _stateAfterWhenFalse;
       LocalState stateAfterWhenTrue = new LocalState.childPath(stateBefore)
@@ -1540,6 +1550,13 @@ class KernelTypeGraphBuilder extends ir.Visitor<TypeInformation> {
       LocalState stateAfter = stateBefore.mergeDiamondFlow(
           _inferrer, stateAfterWhenTrue, stateAfterWhenFalse);
       _setStateAfter(stateAfter, stateAfterWhenTrue, stateAfterWhenFalse);
+      // Constant-fold result.
+      if (_types.isLiteralTrue(leftInfo)) return leftInfo;
+      if (_types.isLiteralFalse(leftInfo)) {
+        if (_types.isLiteralTrue(rightInfo)) return rightInfo;
+        if (_types.isLiteralFalse(rightInfo)) return rightInfo;
+      }
+      // TODO(sra): Add a selector/mux node to improve precision.
       return _types.boolType;
     }
     failedAt(CURRENT_ELEMENT_SPANNABLE,

pkg/compiler/lib/src/inferrer/inferrer_engine.dart

@@ -508,7 +508,9 @@ class InferrerEngine {
       // it is in the graph.
       types.withMember(member, () => analyze(member));
     });
+    metrics.elementsInGraph.add(_addedInGraph);
     _reporter.log('Added $_addedInGraph elements in inferencing graph.');
+    metrics.allTypesCount.add(types.allTypes.length);
   }
 
   /// Returns the body node for [member].
@@ -1201,6 +1203,8 @@ class _InferrerEngineMetrics extends MetricsBase {
   final analyze = DurationMetric('time.analyze');
   final refine1 = DurationMetric('time.refine1');
   final refine2 = DurationMetric('time.refine2');
+  final elementsInGraph = CountMetric('count.elementsInGraph');
+  final allTypesCount = CountMetric('count.allTypes');
   final exceededMaxChangeCount = CountMetric('count.exceededMaxChange');
   final overallRefineCount = CountMetric('count.overallRefines');
 
@@ -1210,6 +1214,8 @@ class _InferrerEngineMetrics extends MetricsBase {
       analyze,
       refine1,
       refine2,
+      elementsInGraph,
+      allTypesCount,
       exceededMaxChangeCount,
       overallRefineCount
     ];

pkg/compiler/lib/src/inferrer/type_system.dart

@@ -4,6 +4,7 @@
 
 import 'package:kernel/ast.dart' as ir;
 import '../common.dart';
+import '../constants/values.dart' show BoolConstantValue;
 import '../elements/entities.dart';
 import '../elements/types.dart';
 import '../world.dart';
@@ -81,6 +82,9 @@ class TypeSystem {
   final Map<AbstractValue, TypeInformation> concreteTypes =
       new Map<AbstractValue, TypeInformation>();
 
+  /// Cache of some primitive constant types.
+  final Map<Object, TypeInformation> primitiveConstantTypes = {};
+
   /// List of [TypeInformation]s for calls inside method bodies.
   final List<CallSiteTypeInformation> allocatedCalls =
       <CallSiteTypeInformation>[];
@@ -102,8 +106,9 @@ class TypeSystem {
         allocatedMaps.values,
         allocatedClosures,
         concreteTypes.values,
+        primitiveConstantTypes.values,
         allocatedCalls,
-        allocatedTypes
+        allocatedTypes,
       ].expand((x) => x);
 
   TypeSystem(this._closedWorld, this.strategy) {
@@ -283,8 +288,22 @@ class TypeSystem {
   }
 
   TypeInformation boolLiteralType(bool value) {
-    return new BoolLiteralTypeInformation(
-        _abstractValueDomain, value, _abstractValueDomain.boolType);
+    return primitiveConstantTypes[value] ??= _boolLiteralType(value);
+  }
+
+  TypeInformation _boolLiteralType(bool value) {
+    AbstractValue abstractValue = _abstractValueDomain
+        .computeAbstractValueForConstant(BoolConstantValue(value));
+    return BoolLiteralTypeInformation(
+        _abstractValueDomain, value, abstractValue);
+  }
+
+  bool isLiteralTrue(TypeInformation info) {
+    return info is BoolLiteralTypeInformation && info.value == true;
+  }
+
+  bool isLiteralFalse(TypeInformation info) {
+    return info is BoolLiteralTypeInformation && info.value == false;
   }
 
   /// Returns the least upper bound between [firstType] and

pkg/compiler/test/inference/data/logical.dart

@@ -154,14 +154,14 @@ returnLogicalAnd() {
 /// Return logical and of `true` && `true`.
 ////////////////////////////////////////////////////////////////////////////////
 
-/*member: returnLogicalAndTrueTrue:[exact=JSBool]*/
+/*member: returnLogicalAndTrueTrue:Value([exact=JSBool], value: true)*/
 returnLogicalAndTrueTrue() => true && true;
 
 ////////////////////////////////////////////////////////////////////////////////
 /// Return logical and of `false` && `true`.
 ////////////////////////////////////////////////////////////////////////////////
 
-/*member: returnLogicalAndFalseTrue:[exact=JSBool]*/
+/*member: returnLogicalAndFalseTrue:Value([exact=JSBool], value: false)*/
 /// ignore: dead_code
 returnLogicalAndFalseTrue() => false && true;
 
@@ -275,14 +275,14 @@ returnLogicalOr() {
 /// Return logical or of `false` || `true`.
 ////////////////////////////////////////////////////////////////////////////////
 
-/*member: returnLogicalOrFalseTrue:[exact=JSBool]*/
+/*member: returnLogicalOrFalseTrue:Value([exact=JSBool], value: true)*/
 returnLogicalOrFalseTrue() => false || true;
 
 ////////////////////////////////////////////////////////////////////////////////
 /// Return logical or of `false` || `false`.
 ////////////////////////////////////////////////////////////////////////////////
 
-/*member: returnLogicalOrFalseFalse:[exact=JSBool]*/
+/*member: returnLogicalOrFalseFalse:Value([exact=JSBool], value: false)*/
 returnLogicalOrFalseFalse() => false || false;
 
 ////////////////////////////////////////////////////////////////////////////////