dart2js: eliminate redundant stores

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

 // Copyright (c) 2012, 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.
 
 import '../common/codegen.dart' show CodegenRegistry, CodegenWorkItem;
 import '../common/names.dart' show Selectors;
 import '../common/tasks.dart' show CompilerTask;
 import '../compiler.dart' show Compiler;
 import '../constants/constant_system.dart';
 import '../constants/values.dart';
@@ skipping 1167 matching lines @@
       // All intercepted classes extend `Interceptor`, so if the receiver can't
       // be a class extending `Interceptor` then it can be called directly.
       if (new TypeMask.nonNullSubclass(
               _helpers.jsInterceptorClass, _closedWorld)
           .isDisjoint(input.instructionType, _closedWorld)) {
         var inputs = <HInstruction>[input, input]; // [interceptor, receiver].
         HInstruction result = new HInvokeDynamicMethod(
             selector,
             input.instructionType, // receiver mask.
             inputs,
-            toStringType)..sourceInformation = node.sourceInformation;
+            toStringType)
+          ..sourceInformation = node.sourceInformation;
         return result;
       }
       return null;
     }
 
     return tryConstant() ?? tryToString() ?? node;
   }
 
   HInstruction visitOneShotInterceptor(HOneShotInterceptor node) {
     return handleInterceptedCall(node);
@@ skipping 1088 matching lines @@
       } else if (user is HBoolify) {
         // We collect targets for strictly boolean operations so HBoolify cannot
         // change the result.
         collectTargets(user, trueTargets, falseTargets);
       }
     }
   }
 }
 
 /**
- * Optimization phase that tries to eliminate memory loads (for
- * example [HFieldGet]), when it knows the value stored in that memory
- * location.
+ * Optimization phase that tries to eliminate memory loads (for example
+ * [HFieldGet]), when it knows the value stored in that memory location, and
+ * stores that overwrite with the same value.
  */
 class SsaLoadElimination extends HBaseVisitor implements OptimizationPhase {
   final BackendHelpers _helpers;
   final Compiler compiler;
   final ClosedWorld closedWorld;
   final String name = "SsaLoadElimination";
   MemorySet memorySet;
   List<MemorySet> memories;
   bool newGvnCandidates = false;
 
@@ skipping 76 matching lines @@
     if (existing != null) {
       checkNewGvnCandidates(instruction, existing);
       instruction.block.rewriteWithBetterUser(instruction, existing);
       instruction.block.remove(instruction);
     } else {
       memorySet.registerFieldValue(element, receiver, instruction);
     }
   }
 
   void visitFieldSet(HFieldSet instruction) {
+    FieldEntity element = instruction.element;
     HInstruction receiver = instruction.getDartReceiver(closedWorld).nonCheck();
-    memorySet.registerFieldValueUpdate(
-        instruction.element, receiver, instruction.inputs.last);
+    if (memorySet.registerFieldValueUpdate(
+        element, receiver, instruction.value)) {
+      instruction.block.remove(instruction);
+    }
   }
 
   void visitCreate(HCreate instruction) {
     memorySet.registerAllocation(instruction);
     if (shouldTrackInitialValues(instruction)) {
       int argumentIndex = 0;
       compiler.codegenWorldBuilder.forEachInstanceField(instruction.element,
           (_, FieldEntity member) {
         if (compiler.elementHasCompileTimeError(
             // ignore: UNNECESSARY_CAST
@@ skipping 66 matching lines @@
     } else {
       memorySet.registerFieldValue(element, null, instruction);
     }
   }
 
   void visitStatic(HStatic instruction) {
     handleStaticLoad(instruction.element, instruction);
   }
 
   void visitStaticStore(HStaticStore instruction) {
-    memorySet.registerFieldValueUpdate(
-        instruction.element, null, instruction.inputs.last);
+    if (memorySet.registerFieldValueUpdate(
+        instruction.element, null, instruction.inputs.last)) {
+      instruction.block.remove(instruction);
+    }
   }
 
   void visitLiteralList(HLiteralList instruction) {
     memorySet.registerAllocation(instruction);
     memorySet.killAffectedBy(instruction);
     // TODO(sra): Set initial keyed values.
     // TODO(sra): Set initial length.
   }
 
   void visitIndex(HIndex instruction) {
@@ skipping 109 matching lines @@
     return !nonEscapingReceivers.contains(receiver);
   }
 
   void registerAllocation(HInstruction instruction) {
     assert(instruction == instruction.nonCheck());
     nonEscapingReceivers.add(instruction);
   }
 
   /**
    * Sets `receiver.element` to contain [value]. Kills all potential places that
-   * may be affected by this update.
+   * may be affected by this update. Returns `true` if the update is redundant.
    */
-  void registerFieldValueUpdate(
+  bool registerFieldValueUpdate(
       MemberEntity element, HInstruction receiver, HInstruction value) {
     assert(receiver == null || receiver == receiver.nonCheck());
     if (closedWorld.nativeData.isNativeMember(element)) {
-      return; // TODO(14955): Remove this restriction?
+      return false; // TODO(14955): Remove this restriction?
     }
-    // [value] is being set in some place in memory, we remove it from
-    // the non-escaping set.
+    // [value] is being set in some place in memory, we remove it from the
+    // non-escaping set.
     nonEscapingReceivers.remove(value.nonCheck());

[Siggi Cherem (dart-lang), 2017/04/28 22:22:38]

if the assignments is redundant, would the kill here be already done as well?
Could we skip it?

[sra1, 2017/04/28 23:25:24]

I would expect so, but it is not worth skipping.
I want to factor out the nonEscapingReceivers into a separate aliasing analysis
that runs before load elimination.

     Map<HInstruction, HInstruction> map =
         fieldValues.putIfAbsent(element, () => <HInstruction, HInstruction>{});
+    bool isRedundant = map[receiver] == value;
     map.forEach((key, value) {
       if (mayAlias(receiver, key)) map[key] = null;
     });
     map[receiver] = value;
+    return isRedundant;
   }
 
   /**
    * Registers that `receiver.element` is now [value].
    */
   void registerFieldValue(
       MemberEntity element, HInstruction receiver, HInstruction value) {
     assert(receiver == null || receiver == receiver.nonCheck());
     if (closedWorld.nativeData.isNativeMember(element)) {
       return; // TODO(14955): Remove this restriction?
@@ skipping 214 matching lines @@
 
     keyedValues.forEach((receiver, values) {
       result.keyedValues[receiver] =
           new Map<HInstruction, HInstruction>.from(values);
     });
 
     result.nonEscapingReceivers.addAll(nonEscapingReceivers);
     return result;
   }
 }