[dart2js] merge generator body into empty entry function

Change-Id: I99a5f16f9a22d3c3cbb4989151a8d3c7bfda16ef
Reviewed-on: https://dart-review.googlesource.com/56037
Commit-Queue: Stephen Adams <sra@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>

pkg/compiler/lib/src/js_backend/backend.dart

@@ -964,6 +964,7 @@ class JavaScriptBackend {
       CodegenRegistry registry,
       FunctionEntity element,
       jsAst.Expression code,
+      DartType asyncTypeParameter,
       SourceInformation bodySourceInformation,
       SourceInformation exitSourceInformation) {
     if (element.asyncMarker == AsyncMarker.SYNC) return code;
@@ -974,8 +975,8 @@ class JavaScriptBackend {
 
     switch (element.asyncMarker) {
       case AsyncMarker.ASYNC:
-        rewriter = _makeAsyncRewriter(
-            commonElements, elementEnvironment, registry, element, code, name);
+        rewriter = _makeAsyncRewriter(commonElements, elementEnvironment,
+            registry, element, code, asyncTypeParameter, name);
         break;
       case AsyncMarker.SYNC_STAR:
         rewriter = new SyncStarRewriter(reporter, element,
@@ -983,8 +984,7 @@ class JavaScriptBackend {
                 emitter.staticFunctionAccess(commonElements.endOfIteration),
             iterableFactory: emitter
                 .staticFunctionAccess(commonElements.syncStarIterableFactory),
-            iterableFactoryTypeArguments:
-                _fetchItemType(element, elementEnvironment),
+            iterableFactoryTypeArguments: _fetchItemType(asyncTypeParameter),
             yieldStarExpression:
                 emitter.staticFunctionAccess(commonElements.yieldStar),
             uncaughtErrorExpression: emitter
@@ -1006,8 +1006,7 @@ class JavaScriptBackend {
             wrapBody: emitter.staticFunctionAccess(commonElements.wrapBody),
             newController: emitter.staticFunctionAccess(
                 commonElements.asyncStarStreamControllerFactory),
-            newControllerTypeArguments:
-                _fetchItemType(element, elementEnvironment),
+            newControllerTypeArguments: _fetchItemType(asyncTypeParameter),
             safeVariableName: namer.safeVariablePrefixForAsyncRewrite,
             yieldExpression:
                 emitter.staticFunctionAccess(commonElements.yieldSingle),
@@ -1024,22 +1023,15 @@ class JavaScriptBackend {
     return rewriter.rewrite(code, bodySourceInformation, exitSourceInformation);
   }
 
-  /// Returns an optional expression that evaluates the type argument to the
-  /// Future/Stream/Iterable.
-  /// Returns an empty list if the type is not needed.
-  /// Returns `null` if the type expression is determined by
-  /// the outside context and should be added as a function parameter.
-  List<jsAst.Expression> _fetchItemType(
-      FunctionEntity element, ElementEnvironment elementEnvironment) {
-    //DartType type =
-    //  elementEnvironment.getFunctionAsyncOrSyncStarElementType(element);
-
-    //if (!type.containsFreeTypeVariables) {
-    //  var ast = rtiEncoder.getTypeRepresentation(emitter.emitter, type, null);
-    //  return <jsAst.Expression>[ast];
-    //}
-
-    return null;
+  /// Returns an optional expression that evaluates [type].  Returns `null` if
+  /// the type expression is determined by the outside context and should be
+  /// added as a function parameter to the rewritten code.
+  // TODO(sra): We could also return an empty list if the generator takes no
+  // type (e.g. due to rtiNeed optimization).
+  List<jsAst.Expression> _fetchItemType(DartType type) {
+    if (type == null) return null;
+    var ast = rtiEncoder.getTypeRepresentation(emitter.emitter, type, null);
+    return <jsAst.Expression>[ast];
   }
 
   AsyncRewriter _makeAsyncRewriter(
@@ -1048,6 +1040,7 @@ class JavaScriptBackend {
       CodegenRegistry registry,
       FunctionEntity element,
       jsAst.Expression code,
+      DartType elementType,
       jsAst.Name name) {
     bool startAsyncSynchronously = compiler.options.startAsyncSynchronously;
 
@@ -1058,8 +1051,7 @@ class JavaScriptBackend {
         ? commonElements.asyncAwaitCompleterFactory
         : commonElements.syncCompleterFactory;
 
-    List<jsAst.Expression> itemTypeExpression =
-        _fetchItemType(element, elementEnvironment);
+    List<jsAst.Expression> itemTypeExpression = _fetchItemType(elementType);
 
     var rewriter = new AsyncRewriter(reporter, element,
         asyncStart: emitter.staticFunctionAccess(startFunction),

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

@@ -143,8 +143,9 @@ class JsElementCreatorMixin {
     return new JConstructorBody(constructor);
   }
 
-  JGeneratorBody createGeneratorBody(FunctionEntity function) {
-    return new JGeneratorBody(function);
+  JGeneratorBody createGeneratorBody(
+      FunctionEntity function, DartType elementType) {
+    return new JGeneratorBody(function, elementType);
   }
 
   IndexedFunction createGetter(LibraryEntity library,
@@ -504,9 +505,10 @@ class JMethod extends JFunction {
 
 class JGeneratorBody extends JFunction {
   final FunctionEntity function;
+  final DartType elementType;
   final int hashCode;
 
-  JGeneratorBody(this.function)
+  JGeneratorBody(this.function, this.elementType)
       : hashCode = function.hashCode + 1, // Hack stabilize sort order.
         super(function.library, function.enclosingClass, function.memberName,
             function.parameterStructure, function.asyncMarker,

pkg/compiler/lib/src/kernel/element_map_impl.dart

@@ -2920,12 +2920,13 @@ class JsKernelToElementMap extends KernelToElementMapBase
   }
 
   JGeneratorBody getGeneratorBody(covariant IndexedFunction function) {
-    FunctionData functionData = _members.getData(function);
-    ir.TreeNode node = functionData.definition.node;
-    // TODO(sra): Maybe store this in the FunctionData.
     JGeneratorBody generatorBody = _generatorBodies[function];
     if (generatorBody == null) {
-      generatorBody = createGeneratorBody(function);
+      FunctionData functionData = _members.getData(function);
+      ir.TreeNode node = functionData.definition.node;
+      DartType elementType =
+          _elementEnvironment.getFunctionAsyncOrSyncStarElementType(function);
+      generatorBody = createGeneratorBody(function, elementType);
       _members.register<IndexedFunction, FunctionData>(
           generatorBody,
           new GeneratorBodyFunctionData(
@@ -2942,7 +2943,8 @@ class JsKernelToElementMap extends KernelToElementMapBase
     return generatorBody;
   }
 
-  JGeneratorBody createGeneratorBody(FunctionEntity function);
+  JGeneratorBody createGeneratorBody(
+      FunctionEntity function, DartType elementType);
 }
 
 class KernelClassQueries extends ClassQueries {

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

@@ -1013,37 +1013,19 @@ class KernelSsaGraphBuilder extends ir.Visitor
         isStatement: true));
   }
 
-  /// Builds a SSA graph for a sync*/async/async* generator.
+  /// Builds a SSA graph for a sync*/async/async* generator.  We generate a
+  /// entry function which tail-calls a body function. The entry contains
+  /// per-invocation checks and the body, which is later transformed, contains
+  /// the re-entrant 'state machine' code.
   void buildGenerator(FunctionEntity function, ir.FunctionNode functionNode) {
-    // TODO(sra): Optimize by generating a merged entry + body when (1) there
-    // are no checks in the entry and (2) the element type is simple.
-    if (true == true) {
-      buildGeneratorEntry(function, functionNode);
-    } else {
-      openFunction(function, functionNode);
-      functionNode.body.accept(this);
-      closeFunction();
-    }
-  }
-
-  /// Builds a SSA graph for a sync*/async/async* generator body.
-  void buildGeneratorEntry(
-      FunctionEntity function, ir.FunctionNode functionNode) {
-    graph.isGeneratorEntry = true;
-
-    // TODO(sra): Omit entry checks.
     openFunction(function, functionNode);
 
-    // Generate type argument for generator class.
-
-    // Tail-call body.
-    JGeneratorBody body = _elementMap.getGeneratorBody(function);
-    backend.outputUnitData.registerColocatedMembers(function, body);
+    // Prepare to tail-call the body.
 
     // Is 'buildAsyncBody' the best location for the entry?
     var sourceInformation = _sourceInformationBuilder.buildAsyncBody();
 
-    // Forward all the parameters.
+    // Forward all the parameters to the body.
     List<HInstruction> inputs = <HInstruction>[];
     if (graph.thisInstruction != null) {
       inputs.add(graph.thisInstruction);
@@ -1061,8 +1043,29 @@ class KernelSsaGraphBuilder extends ir.Visitor
     // Add the type parameter for the generator's element type.
     DartType elementType = _elementMap.elementEnvironment
         .getAsyncOrSyncStarElementType(function.asyncMarker, _returnType);
-    inputs.add(typeBuilder.analyzeTypeArgument(elementType, function));
 
+    if (elementType.containsFreeTypeVariables) {
+      // Type must be computed in the entry function, where the type variables
+      // are in scope, and passed to the body function.
+      inputs.add(typeBuilder.analyzeTypeArgument(elementType, function));
+    } else {
+      // Types with no type variables can be emitted as part of the generator,
+      // avoiding an extra argument.
+      if (_generatedEntryIsEmpty()) {
+        // If the entry function is empty (e.g. no argument checks) and the type
+        // can be generated in body, 'inline' the body by generating it in
+        // place. This works because the subsequent transformation of the code
+        // is 'correct' for the empty entry function code.
+        graph.needsAsyncRewrite = true;
+        graph.asyncElementType = elementType;
+        functionNode.body.accept(this);
+        closeFunction();
+        return;
+      }
+    }
+
+    JGeneratorBody body = _elementMap.getGeneratorBody(function);
+    backend.outputUnitData.registerColocatedMembers(function, body);
     push(new HInvokeGeneratorBody(
         body,
         inputs,
@@ -1078,13 +1081,28 @@ class KernelSsaGraphBuilder extends ir.Visitor
   /// Builds a SSA graph for a sync*/async/async* generator body.
   void buildGeneratorBody(
       JGeneratorBody function, ir.FunctionNode functionNode) {
-    // TODO(sra): Omit entry checks.
     FunctionEntity entry = function.function;
     openFunction(entry, functionNode);
+    graph.needsAsyncRewrite = true;
+    if (!function.elementType.containsFreeTypeVariables) {
+      // We can generate the element type in place
+      graph.asyncElementType = function.elementType;
+    }
     functionNode.body.accept(this);
     closeFunction();
   }
 
+  bool _generatedEntryIsEmpty() {
+    HBasicBlock block = current;
+    // If `block.id` is not 1 then we generated some control flow.
+    if (block.id != 1) return false;
+    for (HInstruction node = block.first; node != null; node = node.next) {
+      if (node is HGoto) continue;
+      return false;
+    }
+    return true;
+  }
+
   void _potentiallyAddFunctionParameterTypeChecks(ir.FunctionNode function) {
     // Put the type checks in the first successor of the entry,
     // because that is where the type guards will also be inserted.
@@ -1107,6 +1125,7 @@ class KernelSsaGraphBuilder extends ir.Visitor
         return;
       }
       HInstruction newParameter = localsHandler.directLocals[local];
+
       newParameter = typeBuilder.potentiallyCheckOrTrustTypeOfParameter(
           newParameter, _getDartTypeIfValid(variable.type));
       localsHandler.directLocals[local] = newParameter;
@@ -1225,20 +1244,20 @@ class KernelSsaGraphBuilder extends ir.Visitor
     }
   }
 
-  void openFunction(MemberEntity member, [ir.FunctionNode function]) {
+  void openFunction(MemberEntity member, [ir.FunctionNode functionNode]) {
     Map<Local, TypeMask> parameterMap = <Local, TypeMask>{};
-    if (function != null) {
+    if (functionNode != null) {
       void handleParameter(ir.VariableDeclaration node) {
         Local local = localsMap.getLocalVariable(node);
         parameterMap[local] =
             _typeInferenceMap.getInferredTypeOfParameter(local);
       }
 
-      function.positionalParameters.forEach(handleParameter);
-      function.namedParameters.toList()
+      functionNode.positionalParameters.forEach(handleParameter);
+      functionNode.namedParameters.toList()
         ..sort(namedOrdering)
         ..forEach(handleParameter);
-      _returnType = _elementMap.getDartType(function.returnType);
+      _returnType = _elementMap.getDartType(functionNode.returnType);
     }
 
     HBasicBlock block = graph.addNewBlock();
@@ -1258,8 +1277,8 @@ class KernelSsaGraphBuilder extends ir.Visitor
     _addClassTypeVariablesIfNeeded(member);
     _addFunctionTypeVariablesIfNeeded(member);
 
-    if (function != null) {
-      _potentiallyAddFunctionParameterTypeChecks(function);
+    if (functionNode != null) {
+      _potentiallyAddFunctionParameterTypeChecks(functionNode);
     }
     _insertTraceCall(member);
     _insertCoverageCall(member);

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

@@ -47,7 +47,7 @@ class SsaCodeGeneratorTask extends CompilerTask {
 
   String get name => 'SSA code generator';
 
-  js.Fun buildJavaScriptFunction(bool isGeneratorEntry, FunctionEntity element,
+  js.Fun buildJavaScriptFunction(bool needsAsyncRewrite, FunctionEntity element,
       List<js.Parameter> parameters, js.Block body) {
     js.Fun finish(js.AsyncModifier asyncModifier) {
       return new js.Fun(parameters, body, asyncModifier: asyncModifier)
@@ -56,15 +56,17 @@ class SsaCodeGeneratorTask extends CompilerTask {
               .buildDeclaration(element));
     }
 
-    if (isGeneratorEntry) return finish(const js.AsyncModifier.sync());
-
-    return finish(element.asyncMarker.isAsync
-        ? (element.asyncMarker.isYielding
-            ? const js.AsyncModifier.asyncStar()
-            : const js.AsyncModifier.async())
-        : (element.asyncMarker.isYielding
-            ? const js.AsyncModifier.syncStar()
-            : const js.AsyncModifier.sync()));
+    if (needsAsyncRewrite) {
+      return finish(element.asyncMarker.isAsync
+          ? (element.asyncMarker.isYielding
+              ? const js.AsyncModifier.asyncStar()
+              : const js.AsyncModifier.async())
+          : (element.asyncMarker.isYielding
+              ? const js.AsyncModifier.syncStar()
+              : const js.AsyncModifier.sync()));
+    } else {
+      return finish(const js.AsyncModifier.sync());
+    }
   }
 
   js.Expression generateCode(
@@ -122,7 +124,7 @@ class SsaCodeGeneratorTask extends CompilerTask {
           work);
       codegen.visitGraph(graph);
       backend.tracer.traceGraph("codegen", graph);
-      return buildJavaScriptFunction(graph.isGeneratorEntry, work.element,
+      return buildJavaScriptFunction(graph.needsAsyncRewrite, work.element,
           codegen.parameters, codegen.body);
     });
   }

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

@@ -208,9 +208,13 @@ class HGraph {
   HBasicBlock exit;
   HThis thisInstruction;
 
-  /// `true` if a sync*/async/async* method is split into an entry and a body
-  /// and this graph is for the entry, which should not be rewritten.
-  bool isGeneratorEntry = false;
+  /// `true` if this graph should be transformed by a sync*/async/async*
+  /// rewrite.
+  bool needsAsyncRewrite = false;
+
+  /// If this function requires an async rewrite, this is the element type of
+  /// the generator.
+  DartType asyncElementType;
 
   /// Receiver parameter, set for methods using interceptor calling convention.
   HParameterValue explicitReceiverParameter;

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

@@ -7,8 +7,7 @@ library ssa;
 import '../common/codegen.dart' show CodegenWorkItem, CodegenRegistry;
 import '../common/tasks.dart' show CompilerTask, Measurer;
 import '../constants/values.dart';
-import '../elements/entities.dart'
-    show FieldEntity, FunctionEntity, MemberEntity;
+import '../elements/entities.dart' show FieldEntity, MemberEntity;
 import '../io/source_information.dart';
 import '../js/js.dart' as js;
 import '../js_backend/backend.dart' show JavaScriptBackend, FunctionCompiler;
@@ -45,7 +44,7 @@ class SsaFunctionCompiler implements FunctionCompiler {
     optimizer.optimize(work, graph, closedWorld);
     MemberEntity element = work.element;
     js.Expression result = generator.generateCode(work, graph, closedWorld);
-    if (element is FunctionEntity && !graph.isGeneratorEntry) {
+    if (graph.needsAsyncRewrite) {
       SourceInformationBuilder sourceInformationBuilder =
           backend.sourceInformationStrategy.createBuilderForContext(element);
       result = backend.rewriteAsync(
@@ -54,6 +53,7 @@ class SsaFunctionCompiler implements FunctionCompiler {
           work.registry,
           element,
           result,
+          graph.asyncElementType,
           sourceInformationBuilder.buildAsyncBody(),
           sourceInformationBuilder.buildAsyncExit());
     }

tests/compiler/dart2js/sourcemaps/stacktrace/sync_throw_in_async.dart

@@ -10,6 +10,6 @@ main() {
 }
 
 @NoInline()
-test() async /*2:test*/ /*kernel.3:test*/ {
+test() async /*2:test*/ {
   /*4:test*/ throw '>ExceptionMarker<';
 }

tests/compiler/dart2js/sourcemaps/stacktrace/sync_throw_in_awaited_async.dart

@@ -10,12 +10,12 @@ main() {
 }
 
 @NoInline()
-test1() async /*3:test1*/ /*kernel.4:test1*/ {
+test1() async /*3:test1*/ {
   // This call is on the stack when the error is thrown.
   await /*5:test1*/ test2();
 }
 
 @NoInline()
-test2() async /*7:test2*/ /*kernel.8:test2*/ {
+test2() async /*7:test2*/ {
   /*9:test2*/ throw '>ExceptionMarker<';
 }

tests/compiler/dart2js/sourcemaps/stacktrace/sync_throw_in_constructor_from_async.dart

@@ -12,7 +12,7 @@ test() async {
   // TODO(johnniwinther): Investigate why kernel doesn't point to the body
   // start brace.
   // ignore: UNUSED_LOCAL_VARIABLE
-  var /*2:test*/ /*3:test*/ c = new /*4:test*/ Class();
+  var /*2:test*/ c = new /*4:test*/ Class();
 }
 
 class Class {

tests/compiler/dart2js/sourcemaps/stacktrace/sync_throw_in_top_level_method_from_async.dart

@@ -9,7 +9,7 @@ main() {
 }
 
 @NoInline()
-test1() async /*2:test1*/ /*kernel.3:test1*/ {
+test1() async /*2:test1*/ {
   /*9:test1*/ test2();
 }