Fixup redefinitions before doing code motion

runtime/vm/compiler.cc

 // 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.
 
 #include "vm/compiler.h"
 
 #include "vm/assembler.h"
 
 #include "vm/ast_printer.h"
 #include "vm/block_scheduler.h"
@@ skipping 873 matching lines @@
           DEBUG_ASSERT(flow_graph->VerifyUseLists());
           // Canonicalization introduced more opportunities for constant
           // propagation.
           ConstantPropagator::Optimize(flow_graph);
           DEBUG_ASSERT(flow_graph->VerifyUseLists());
         }
 
         // Optimistically convert loop phis that have a single non-smi input
         // coming from the loop pre-header into smi-phis.
         if (FLAG_loop_invariant_code_motion) {
+          flow_graph->FixupRedefinitions();
+          DEBUG_ASSERT(flow_graph->VerifyRedefinitions());
           LICM licm(flow_graph);
           licm.OptimisticallySpecializeSmiPhis();

[Vyacheslav Egorov (Google), 2017/02/23 13:47:27]

I think we don't need to fixup redefinitions here - this pass only hoists
CheckSmi instructions.

[Florian Schneider, 2017/02/23 19:26:35]

Done.

           DEBUG_ASSERT(flow_graph->VerifyUseLists());
         }
 
         // Propagate types and eliminate even more type tests.
         // Recompute types after constant propagation to infer more precise
         // types for uses that were previously reached by now eliminated phis.
         FlowGraphTypePropagator::Propagate(flow_graph);
         DEBUG_ASSERT(flow_graph->VerifyUseLists());
 
         {
@@ skipping 12 matching lines @@
 
         {
           NOT_IN_PRODUCT(TimelineDurationScope tds2(
               thread(), compiler_timeline, "CommonSubexpressionElinination"));
           if (FLAG_common_subexpression_elimination ||
               FLAG_loop_invariant_code_motion) {
             flow_graph->ComputeBlockEffects();
           }
 
           if (FLAG_common_subexpression_elimination) {
             if (DominatorBasedCSE::Optimize(flow_graph)) {

[Vyacheslav Egorov (Google), 2017/02/23 13:47:26]

We do load forwarding here so we might introduce new uses of redefined values,
e.g. by replacing some LoadFieldInstr(...) with concrete values. 

So we need to have flow_graph->FixupRedefinitions() past this point.

[Florian Schneider, 2017/02/23 19:26:35]

Done.

               DEBUG_ASSERT(flow_graph->VerifyUseLists());
               flow_graph->Canonicalize();
               // Do another round of CSE to take secondary effects into account:
               // e.g. when eliminating dependent loads (a.x[0] + a.x[0])
               // TODO(fschneider): Change to a one-pass optimization pass.
               if (DominatorBasedCSE::Optimize(flow_graph)) {
                 flow_graph->Canonicalize();
               }
               DEBUG_ASSERT(flow_graph->VerifyUseLists());
             }
           }
 
           // Run loop-invariant code motion right after load elimination since
           // it depends on the numbering of loads from the previous
           // load-elimination.
           if (FLAG_loop_invariant_code_motion) {
             LICM licm(flow_graph);

[Vyacheslav Egorov (Google), 2017/02/23 13:47:27]

I think we should fixup redefinitions here.

[Florian Schneider, 2017/02/23 19:26:35]

Done.

             licm.Optimize();
             DEBUG_ASSERT(flow_graph->VerifyUseLists());
           }
           flow_graph->RemoveRedefinitions();
         }
 
         // Optimize (a << b) & c patterns, merge operations.
         // Run after CSE in order to have more opportunity to merge
         // instructions that have same inputs.
         flow_graph->TryOptimizePatterns();
@@ skipping 1309 matching lines @@
 
 
 bool BackgroundCompiler::IsDisabled() {
   UNREACHABLE();
   return true;
 }
 
 #endif  // DART_PRECOMPILED_RUNTIME
 
 }  // namespace dart