dart2js cps: Replace GetLazyStatic with GetStatic.

pkg/compiler/lib/src/cps_ir/eagerly_load_statics.dart

+// Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+library dart2js.cps_ir.eagerly_load_statics;
+
+import 'cps_ir_nodes.dart';
+import 'optimizers.dart' show Pass;
+import '../elements/elements.dart';
+
+/// Replaces [GetLazyStatic] with [GetStatic] when the static field is known
+/// to have been initialized.
+///
+/// Apart from [GetStatic] generating better code, this improves the side-effect
+/// analysis in the [GVN] pass, since [GetStatic] is known to have no effects.

[sra1, 2015/12/11 17:13:09]

"is known to have" -> "has"

[asgerf, 2015/12/14 17:05:39]

Done.

+class EagerlyLoadStatics extends TrampolineRecursiveVisitor implements Pass {
+  String get passName => 'Eagerly load statics';
+
+  Set<FieldElement> loadedStatics = new Set<FieldElement>();
+  final Map<Continuation, Set<FieldElement>> loadedStaticsAt = {};

[sra1, 2015/12/11 17:13:08]

Put types on the map literal - that is what is checked.
(It is easy to accidentally add a Reference)

Perhaps use a HashMap instead of LinkedHashMap.

[asgerf, 2015/12/14 17:05:39]

The pass has changed quite a bit now, but there are types on the maps and there
is a clone method so we can switch to non-linked in one place if we want.

+
+  void rewrite(FunctionDefinition node) {
+    visit(node);
+  }
+
+  void visitBranch(Branch node) {
+    Continuation trueCont = node.trueContinuation.definition;
+    Continuation falseCont = node.falseContinuation.definition;
+    loadedStaticsAt[trueCont] = loadedStatics;
+    loadedStaticsAt[falseCont] = new Set<FieldElement>.from(loadedStatics);
+  }
+
+  Expression traverseContinuation(Continuation cont) {
+    loadedStatics = loadedStaticsAt[cont] ?? new Set<FieldElement>();

[sra1, 2015/12/11 17:13:08]

Why is it null?

I would add two helpers - newSet and cloneSet and put them side-by side so it is
easy to change the set type (e.g. HashSet instead of (LinkedHash)Set)

[asgerf, 2015/12/14 17:05:39]

Null was for the case of unreachable continuations, but the pass uses scoping
now, so it's not needed anymore.

+    return cont.body;
+  }
+
+  Expression traverseLetHandler(LetHandler node) {
+    loadedStaticsAt[node.handler] = new Set<FieldElement>.from(loadedStatics);
+    push(node.handler);
+    return node.body;
+  }
+
+  void visitInvokeContinuation(InvokeContinuation node) {
+    if (node.isRecursive) return;
+    Continuation cont = node.continuation.definition;
+    if (cont.isReturnContinuation) return;
+    Set<FieldElement> targetSet = loadedStaticsAt[cont];
+    if (targetSet == null) {
+      loadedStaticsAt[cont] = loadedStatics;
+    } else {
+      targetSet.retainAll(loadedStatics);
+    }
+  }
+
+  void visitGetLazyStatic(GetLazyStatic node) {
+    // Mark the field as loaded, and replace it with a GetStatic if it was
+    // already loaded.
+    if (!loadedStatics.add(node.element)) {
+      node.replaceWith(new GetStatic(node.element, node.sourceInformation)
+          ..type = node.type);
+    }
+  }
+
+  void visitSetStatic(SetStatic node) {
+    loadedStatics.add(node.element);
+  }

[sra1, 2015/12/11 17:13:08]

Could there already be a GetStatic?
Is the pass idempotent without adding it?

[asgerf, 2015/12/14 17:05:39]

There can't be any GetStatics unless the pass is run twice, and in that case the
dominating GetLazyStatic/SetStatic should still be there.

GetStatic doesn't actually load the field, it just returns undefined if
uninitialized, so I didn't want to add it.

Since GetStatic has a witness reference now, I should be clear that the witness
is going to be in scope anyway, so it makes no difference.

+}