Reland r45243.

runtime/vm/constant_propagator.cc

 // Copyright (c) 2013, 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/constant_propagator.h"
 
 #include "vm/bit_vector.h"
 #include "vm/flow_graph_builder.h"
 #include "vm/flow_graph_compiler.h"
 #include "vm/flow_graph_range_analysis.h"
@@ skipping 300 matching lines @@
   }
   if (!SetValue(instr, value) &&
       marked_phis_->Contains(instr->ssa_temp_index())) {
     marked_phis_->Remove(instr->ssa_temp_index());
     definition_worklist_.Add(instr);
   }
 }
 
 
 void ConstantPropagator::VisitRedefinition(RedefinitionInstr* instr) {
-  SetValue(instr, instr->value()->definition()->constant_value());
+  // Ensure that we never remove redefinition of a constant unless we are also
+  // are guaranteed to fold away code paths that correspond to non-matching
+  // class ids. Otherwise LICM might potentially hoist incorrect code.
+  const Object& value = instr->value()->definition()->constant_value();
+  if (IsConstant(value) &&
+      CheckClassInstr::IsImmutableClassId(value.GetClassId())) {

[Florian Schneider, 2015/04/20 09:33:32]

Works for now. Could we use the knowledge that the cid of a string can only
change from StringCid -> ExternalStringCid? i.e. even though the cid is not
considered constant, it can only be one of two values.

+    SetValue(instr, value);
+  } else {
+    SetValue(instr, non_constant_);
+  }
 }
 
 
 void ConstantPropagator::VisitParameter(ParameterInstr* instr) {
   SetValue(instr, non_constant_);
 }
 
 
 void ConstantPropagator::VisitPushArgument(PushArgumentInstr* instr) {
   SetValue(instr, instr->value()->definition()->constant_value());
@@ skipping 449 matching lines @@
   SetValue(instr, non_constant_);
 }
 
 
 void ConstantPropagator::VisitLoadClassId(LoadClassIdInstr* instr) {
   intptr_t cid = instr->object()->Type()->ToCid();
   if (cid != kDynamicCid) {
     SetValue(instr, Smi::ZoneHandle(Z, Smi::New(cid)));
     return;
   }
+
   const Object& object = instr->object()->definition()->constant_value();
   if (IsConstant(object)) {
-    SetValue(instr, Smi::ZoneHandle(Z, Smi::New(object.GetClassId())));
-    return;
+    cid = object.GetClassId();
+    if (CheckClassInstr::IsImmutableClassId(cid)) {
+      SetValue(instr, Smi::ZoneHandle(Z, Smi::New(cid)));
+      return;
+    }
   }
   SetValue(instr, non_constant_);
 }
 
 
 void ConstantPropagator::VisitLoadField(LoadFieldInstr* instr) {
   Value* instance = instr->instance();
   if ((instr->recognized_kind() == MethodRecognizer::kObjectArrayLength) &&
       instance->definition()->OriginalDefinition()->IsCreateArray()) {
     Value* num_elements = instance->definition()->OriginalDefinition()
@@ skipping 858 matching lines @@
   graph_->MergeBlocks();
   GrowableArray<BitVector*> dominance_frontier;
   graph_->ComputeDominators(&dominance_frontier);
 
   if (FLAG_trace_constant_propagation) {
     FlowGraphPrinter::PrintGraph("After CP", graph_);
   }
 }
 
 }  // namespace dart