[vm, compiler] Remove --fields_may_be_reset.

This is true everywhere by default. It can only be false when isolate groups are disabled, and the ability to disable isolate groups will be removed.

TEST=ci
Change-Id: I8ce12883a6128a6c6c1883605b5e3889952ce76c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/217153
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>

runtime/bin/gen_snapshot.cc

@@ -879,7 +879,6 @@ int main(int argc, char** argv) {
   if (IsSnapshottingForPrecompilation()) {
     vm_options.AddArgument("--precompilation");
   } else if ((snapshot_kind == kCoreJIT) || (snapshot_kind == kAppJIT)) {
-    vm_options.AddArgument("--fields_may_be_reset");
 #if !defined(TARGET_ARCH_IA32)
     vm_options.AddArgument("--link_natives_lazily");
 #endif

runtime/bin/main.cc

@@ -1213,9 +1213,6 @@ void main(int argc, char** argv) {
     try_load_snapshots_lambda();
   }
 
-  if (Options::gen_snapshot_kind() == kAppJIT) {
-    vm_options.AddArgument("--fields_may_be_reset");
-  }
 #if defined(DART_PRECOMPILED_RUNTIME)
   vm_options.AddArgument("--precompilation");
 #endif

runtime/tests/concurrency/stress_test_list.json

@@ -3230,7 +3230,6 @@
     "../../../tests/standalone/double_smi_comparison_test.dart",
     "../../../tests/standalone/double_temp_test.dart",
     "../../../tests/standalone/double_to_int_test.dart",
-    "../../../tests/standalone/fields_may_be_reset_test.dart",
     "../../../tests/standalone/float_array_test.dart",
     "../../../tests/standalone/fragmentation_typed_data_test.dart",
     "../../../tests/standalone/int_array_load_elimination_test.dart",
@@ -6560,7 +6559,6 @@
     "../../../tests/standalone_2/double_smi_comparison_test.dart",
     "../../../tests/standalone_2/double_temp_test.dart",
     "../../../tests/standalone_2/double_to_int_test.dart",
-    "../../../tests/standalone_2/fields_may_be_reset_test.dart",
     "../../../tests/standalone_2/float_array_test.dart",
     "../../../tests/standalone_2/int_array_load_elimination_test.dart",
     "../../../tests/standalone_2/int_array_test.dart",

runtime/vm/compiler/backend/constant_propagator.cc

@@ -845,17 +845,9 @@ void ConstantPropagator::VisitLoadIndexedUnsafe(LoadIndexedUnsafeInstr* instr) {
 }
 
 void ConstantPropagator::VisitLoadStaticField(LoadStaticFieldInstr* instr) {
-  if (!FLAG_fields_may_be_reset) {
-    const Field& field = instr->field();
-    ASSERT(field.is_static());
-    auto& obj = Object::Handle(Z);
-    if (field.is_final() && instr->IsFieldInitialized(&obj)) {
-      if (obj.IsSmi() || (obj.IsOld() && obj.IsCanonical())) {
-        SetValue(instr, obj);
-        return;
-      }
-    }
-  }
+  // Cannot treat an initialized field as constant because the same code will be
+  // used when the AppAOT or AppJIT starts over with everything uninitialized or
+  // another isolate in the isolate group starts with everything uninitialized.
   SetValue(instr, non_constant_);
 }
 

runtime/vm/compiler/backend/il.cc

@@ -1077,45 +1077,6 @@ bool LoadStaticFieldInstr::AttributesEqual(const Instruction& other) const {
   return field().ptr() == other.AsLoadStaticField()->field().ptr();
 }
 
-bool LoadStaticFieldInstr::IsFieldInitialized(Object* field_value) const {
-  if (FLAG_fields_may_be_reset) {
-    return false;
-  }
-
-  // Since new isolates will be spawned, the JITed code cannot depend on whether
-  // global field was initialized when running with --enable-isolate-groups.
-  if (FLAG_enable_isolate_groups) return false;
-
-  const Field& field = this->field();
-  Isolate* only_isolate = IsolateGroup::Current()->FirstIsolate();
-  if (only_isolate == nullptr) {
-    // This can happen if background compiler executes this code but the mutator
-    // is being shutdown and the isolate was already unregistered from the group
-    // (and is trying to stop this BG compiler).
-    if (field_value != nullptr) {
-      *field_value = Object::sentinel().ptr();
-    }
-    return false;
-  }
-  if (field_value == nullptr) {
-    field_value = &Object::Handle();
-  }
-  *field_value = only_isolate->field_table()->At(field.field_id());
-  return (field_value->ptr() != Object::sentinel().ptr()) &&
-         (field_value->ptr() != Object::transition_sentinel().ptr());
-}
-
-Definition* LoadStaticFieldInstr::Canonicalize(FlowGraph* flow_graph) {
-  // When precompiling, the fact that a field is currently initialized does not
-  // make it safe to omit code that checks if the field needs initialization
-  // because the field will be reset so it starts uninitialized in the process
-  // running the precompiled code. We must be prepared to reinitialize fields.
-  if (calls_initializer() && IsFieldInitialized()) {
-    set_calls_initializer(false);
-  }
-  return this;
-}
-
 ConstantInstr::ConstantInstr(const Object& value,
                              const InstructionSource& source)
     : TemplateDefinition(source), value_(value), token_pos_(source.token_pos) {

runtime/vm/compiler/backend/il.h

@@ -5577,7 +5577,6 @@ class LoadStaticFieldInstr : public TemplateDefinition<0, Throws> {
   virtual CompileType ComputeType() const;
 
   const Field& field() const { return field_; }
-  bool IsFieldInitialized(Object* field_value = nullptr) const;
 
   bool calls_initializer() const { return calls_initializer_; }
   void set_calls_initializer(bool value) { calls_initializer_ = value; }
@@ -5601,8 +5600,6 @@ class LoadStaticFieldInstr : public TemplateDefinition<0, Throws> {
   virtual bool CanTriggerGC() const { return calls_initializer(); }
   virtual bool MayThrow() const { return calls_initializer(); }
 
-  virtual Definition* Canonicalize(FlowGraph* flow_graph);
-
   virtual bool AttributesEqual(const Instruction& other) const;
 
   virtual TokenPosition token_pos() const { return token_pos_; }

runtime/vm/compiler/backend/redundancy_elimination.cc

@@ -432,8 +432,6 @@ class Place : public ValueObject {
     switch (kind()) {
       case kInstanceField:
         return instance_field().is_immutable();
-      case kStaticField:
-        return static_field().is_final() && !FLAG_fields_may_be_reset;
       default:
         return false;
     }
@@ -1542,7 +1540,7 @@ void LICM::Optimize() {
         // we should not move them around unless the field is initialized.
         // Otherwise we might move load past the initialization.
         if (LoadStaticFieldInstr* load = current->AsLoadStaticField()) {
-          if (load->AllowsCSE() && !load->IsFieldInitialized()) {
+          if (load->AllowsCSE()) {
             seen_visible_effect = true;
             continue;
           }

runtime/vm/compiler/backend/redundancy_elimination_test.cc

@@ -930,10 +930,6 @@ ISOLATE_UNIT_TEST_CASE(LoadOptimizer_RedundantStaticFieldInitialization) {
     }
   )";
 
-  // Make sure static field initialization is not removed because
-  // field is already initialized.
-  SetFlagScope<bool> sfs(&FLAG_fields_may_be_reset, true);
-
   const auto& root_library = Library::Handle(LoadTestScript(kScript));
   Invoke(root_library, "main");
   const auto& function = Function::Handle(GetFunction(root_library, "foo"));
@@ -979,10 +975,6 @@ ISOLATE_UNIT_TEST_CASE(LoadOptimizer_RedundantInitializerCallAfterIf) {
     }
   )";
 
-  // Make sure static field initialization is not removed because
-  // field is already initialized.
-  SetFlagScope<bool> sfs(&FLAG_fields_may_be_reset, true);
-
   const auto& root_library = Library::Handle(LoadTestScript(kScript));
   Invoke(root_library, "main");
   const auto& function = Function::Handle(GetFunction(root_library, "foo"));

runtime/vm/compiler/backend/type_propagator.cc

@@ -1541,16 +1541,6 @@ CompileType LoadStaticFieldInstr::ComputeType() const {
     is_nullable = false;
   }
 
-  auto& obj = Object::Handle();
-  const bool is_initialized = IsFieldInitialized(&obj);
-  if (field.is_final() && is_initialized) {
-    if (!obj.IsNull()) {
-      is_nullable = false;
-      cid = obj.GetClassId();
-      abstract_type = nullptr;  // Cid is known, calculate abstract type lazily.
-    }
-  }
-
   if ((field.guarded_cid() != kIllegalCid) &&
       (field.guarded_cid() != kDynamicCid)) {
     cid = field.guarded_cid();

runtime/vm/compiler/jit/compiler.cc

@@ -94,7 +94,6 @@ static void PrecompilationModeHandler(bool value) {
 
     FLAG_background_compilation = false;
     FLAG_enable_mirrors = false;
-    FLAG_fields_may_be_reset = true;
     FLAG_interpret_irregexp = true;
     FLAG_lazy_dispatchers = false;
     FLAG_link_natives_lazily = true;

runtime/vm/flag_list.h

@@ -124,8 +124,6 @@ constexpr bool FLAG_support_il_printer = false;
   P(enable_mirrors, bool, true,                                                \
     "Disable to make importing dart:mirrors an error.")                        \
   P(enable_ffi, bool, true, "Disable to make importing dart:ffi an error.")    \
-  P(fields_may_be_reset, bool, false,                                          \
-    "Don't optimize away static field initialization")                         \
   P(force_clone_compiler_objects, bool, false,                                 \
     "Force cloning of objects needed in compiler (ICData and Field).")         \
   P(getter_setter_ratio, int, 13,                                              \

runtime/vm/flags.cc

@@ -466,20 +466,6 @@ char* Flags::ProcessCommandLineFlags(int number_of_vm_flags,
     PrintFlags();
   }
 
-  // TODO(dartbug.com/36097): Support for isolate groups in JIT mode is
-  // in-development. We will start with very conservative settings. As we make
-  // more of our compiler, runtime as well as generated code re-entrant we'll
-  // graudally remove those restrictions.
-
-#if !defined(DART_PRCOMPILED_RUNTIME)
-  if (!FLAG_precompiled_mode && FLAG_enable_isolate_groups) {
-    // Our compiler should not make rely on a global field being initialized at
-    // compile-time, since that compiled code might be re-used in another
-    // isolate that has not yet initialized the global field.
-    FLAG_fields_may_be_reset = true;
-  }
-#endif  // !defined(DART_PRCOMPILED_RUNTIME)
-
   initialized_ = true;
   return NULL;
 }

tests/standalone/fields_may_be_reset_test.dart

@@ -1,9 +0,0 @@
-// Copyright (c) 2016, 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.
-
-// VMOptions=--fields_may_be_reset
-
-main() {
-  print("Okay");
-}

tests/standalone_2/fields_may_be_reset_test.dart

@@ -1,11 +0,0 @@
-// Copyright (c) 2016, 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.
-
-// @dart = 2.9
-
-// VMOptions=--fields_may_be_reset
-
-main() {
-  print("Okay");
-}