Fix bad generate-at-use-site.

sdk/lib/_internal/compiler/implementation/ssa/codegen_helpers.dart

Index: sdk/lib/_internal/compiler/implementation/ssa/codegen_helpers.dart
diff --git a/sdk/lib/_internal/compiler/implementation/ssa/codegen_helpers.dart b/sdk/lib/_internal/compiler/implementation/ssa/codegen_helpers.dart
index 1e83906f09e213186317df5c505165962b732c96..20c51d4f85d999af4c8ef77e639cd91490979d09 100644
--- a/sdk/lib/_internal/compiler/implementation/ssa/codegen_helpers.dart
+++ b/sdk/lib/_internal/compiler/implementation/ssa/codegen_helpers.dart
@@ -285,15 +285,60 @@ class SsaInstructionMerger extends HBaseVisitor {
         markAsGenerateAtUseSite(instruction);
         continue;
       }
-      if (instruction.isJsStatement()) {

[kasperl, 2014/03/15 07:14:48]

Can you enumerate the things we use isJsStatement for now? It is still a
confusing concept so getting rid of that (in another CL) might be beneficial.

[floitsch, 2014/03/17 10:26:14]

It informs `generateAtUseSite` that the node must not be used in an expression
context.

I will look (in a separate CL) why store-nodes are marked as `isJsStatement`.

-        expectedInputs.clear();
-      }
       if (instruction.isPure()) {
         if (pureInputs.contains(instruction)) {
           tryGenerateAtUseSite(instruction);
         } else {
           // If the input is not in the [pureInputs] set, it has not
-          // been visited.
+          // been visited or should not be generated at use-site. The most
+          // likely reason for the latter, is that the instruction is used
+          // in more than one location.
+          // We must either clear the expectedInputs, or move the pure
+          // instruction's inputs in front of the existing ones.
+          // Example:
+          //   t1 = foo();  // side-effect.
+          //   t2 = bar();  // side-effect.
+          //   t3 = pure(t2);    // used more than once.
+          //   f(t1, t3);   // expected inputs of 'f': t1.
+          //   use(t3);
+          //
+          // If we don't clear the expected inputs we end up in a situation
+          // where pure pushes "t2" on top of "t1" leading to:
+          //
+          //  t3 = pure(bar());
+          //  f(foo(), t3);
+          //  use(t3);
+          //
+          // If we clear the expected-inputs list we have the correct
+          // output:
+          //   t1 = foo();
+          //   t3 = pure(bar());
+          //   f(t1, t3);
+          //   use(t3);
+          //
+          // Clearing is, however, not optimal.
+          // Example:
+          //   t1 = foo();  // t1 is now used by `pure`.
+          //   t2 = bar();  // t2 is now used by `f`.
+          //   t3 = pure(t1);
+          //   f(t2, t3);
+          //   use(t3);
+          //
+          // If we clear the expected-inputs we can't generate-at-use any of
+          // the instructions.
+          //
+          // The optimal solution is to store move the inputs of 'pure' in

[kasperl, 2014/03/15 07:10:56]

store move -> move

[floitsch, 2014/03/17 10:26:14]

Done.

+          // front of the expectedInputs list. This makes sense, since we
+          // push expected-inputs from left-to right, and the `pure` function
+          // invocation is "more left" (i.e. before) the first argument of `f`.
+          // With that approach we would end up with:
+          //   t3 = pure(foo();
+          //   f(bar(), t3);
+          //   use(t3);
+          //
+          // TODO(floitsch): should we keep existing inputs and move new inputs

[kasperl, 2014/03/15 07:10:56]

File a bug for this?

[floitsch, 2014/03/17 10:26:14]

Just implemented the optimization. -> removed the TODO.

+          // to the front of the list?
+          expectedInputs.clear();
           instruction.accept(this);
         }
       } else {