From 085e22dd72081a3ed5067f723cdffc64fb3fee83 Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Fri, 17 Apr 2020 18:32:28 +0000 Subject: [PATCH] [vm/nnbd] Change type of sentinel values to Never Special sentinel values which are used for lazy initialization can be stored in variables of any type. In NNBD world Null is no longer a bottom type as null cannot be assigned into variables of non-nullable types. The new universal bottom type is Never, so type of sentinel values is changed from Null to Never. This also allows to make late fields non-nullable in the type propagation and avoid extra updates to field guards. This change also adds special handling of comparisons with sentinel values into constant propagation as it can constant fold comparisons of values with different cids. Fixes https://github.com/dart-lang/sdk/issues/40796 Change-Id: I8266cfba0cc434b78ffa9570c30d434d4380d4af Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/143864 Reviewed-by: Liam Appelbe Reviewed-by: Ryan Macnak Commit-Queue: Alexander Markov --- runtime/vm/compiler/aot/precompiler.cc | 15 +++++++++++---- .../vm/compiler/backend/constant_propagator.cc | 4 ++++ runtime/vm/compiler/backend/slot.cc | 5 ----- runtime/vm/compiler/frontend/bytecode_reader.cc | 9 +-------- runtime/vm/kernel_loader.cc | 8 -------- runtime/vm/object.cc | 10 +++++----- runtime/vm/raw_object.cc | 1 + 7 files changed, 22 insertions(+), 30 deletions(-) diff --git a/runtime/vm/compiler/aot/precompiler.cc b/runtime/vm/compiler/aot/precompiler.cc index 836ae49a07f..edb6b3fe24c 100644 --- a/runtime/vm/compiler/aot/precompiler.cc +++ b/runtime/vm/compiler/aot/precompiler.cc @@ -894,6 +894,11 @@ void Precompiler::AddConstObject(const class Instance& instance) { return; } + if (instance.raw() == Object::sentinel().raw() || + instance.raw() == Object::transition_sentinel().raw()) { + return; + } + const Class& cls = Class::Handle(Z, instance.clazz()); AddInstantiatedClass(cls); @@ -971,14 +976,16 @@ void Precompiler::AddField(const Field& field) { if (field.is_static()) { const Object& value = Object::Handle(Z, field.StaticValue()); - if (value.IsInstance()) { + // Should not be in the middle of initialization while precompiling. + ASSERT(value.raw() != Object::transition_sentinel().raw()); + + if (value.raw() != Object::sentinel().raw() && + value.raw() != Object::null()) { + ASSERT(value.IsInstance()); AddConstObject(Instance::Cast(value)); } if (field.has_nontrivial_initializer()) { - // Should not be in the middle of initialization while precompiling. - ASSERT(value.raw() != Object::transition_sentinel().raw()); - if (!field.HasInitializerFunction() || !Function::Handle(Z, field.InitializerFunction()).HasCode()) { if (FLAG_trace_precompiler) { diff --git a/runtime/vm/compiler/backend/constant_propagator.cc b/runtime/vm/compiler/backend/constant_propagator.cc index 60c367003ce..11b10998a23 100644 --- a/runtime/vm/compiler/backend/constant_propagator.cc +++ b/runtime/vm/compiler/backend/constant_propagator.cc @@ -594,7 +594,11 @@ void ConstantPropagator::VisitStrictCompare(StrictCompareInstr* instr) { const intptr_t right_cid = instr->right()->Type()->ToCid(); // If exact classes (cids) are known and they differ, the result // of strict compare can be computed. + // The only exception is comparison with special sentinel value + // (used for lazy initialization) which can be compared to a + // value of any type. Sentinel value has kNeverCid. if ((left_cid != kDynamicCid) && (right_cid != kDynamicCid) && + (left_cid != kNeverCid) && (right_cid != kNeverCid) && (left_cid != right_cid)) { const bool result = (instr->kind() != Token::kEQ_STRICT); SetValue(instr, Bool::Get(result)); diff --git a/runtime/vm/compiler/backend/slot.cc b/runtime/vm/compiler/backend/slot.cc index 79c147ba09a..bab1c7aa19b 100644 --- a/runtime/vm/compiler/backend/slot.cc +++ b/runtime/vm/compiler/backend/slot.cc @@ -225,11 +225,6 @@ const Slot& Slot::Get(const Field& field, is_nullable = false; } - if (field.is_late()) { - // TODO(dartbug.com/40796): Extend CompileType to handle lateness. - is_nullable = true; - } - const Slot& slot = SlotCache::Instance(thread).Canonicalize(Slot( Kind::kDartField, IsImmutableBit::encode((field.is_final() && !field.is_late()) || diff --git a/runtime/vm/compiler/frontend/bytecode_reader.cc b/runtime/vm/compiler/frontend/bytecode_reader.cc index 3f1de8551e6..d2050588a81 100644 --- a/runtime/vm/compiler/frontend/bytecode_reader.cc +++ b/runtime/vm/compiler/frontend/bytecode_reader.cc @@ -2052,14 +2052,7 @@ void BytecodeReaderHelper::ReadFieldDeclarations(const Class& cls, field.set_is_extension_member(is_extension_member); field.set_has_initializer(has_initializer); - if (has_nontrivial_initializer) { - if (field.is_late() && !is_static) { - // Late fields are initialized to Object::sentinel, which is a flavor of - // null. So we need to record that store so that the field guard doesn't - // prematurely optimise out the late field's sentinel checking logic. - field.RecordStore(Object::null_object()); - } - } else { + if (!has_nontrivial_initializer) { value ^= ReadObject(); if (is_static) { if (field.is_late() && !has_initializer) { diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc index 754d2e52e56..b4fea2c846b 100644 --- a/runtime/vm/kernel_loader.cc +++ b/runtime/vm/kernel_loader.cc @@ -2182,14 +2182,6 @@ void KernelLoader::GenerateFieldAccessors(const Class& klass, } ASSERT(field.NeedsGetter()); - if (field.is_late() && !field.is_static() && - field.has_nontrivial_initializer()) { - // Late fields are initialized to Object::sentinel, which is a flavor of - // null. So we need to record that store so that the field guard doesn't - // prematurely optimise out the late field's sentinel checking logic. - field.RecordStore(Object::null_object()); - } - const String& getter_name = H.DartGetterName(field_helper->canonical_name_); const Object& script_class = ClassForScriptAt(klass, field_helper->source_uri_index_); diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index aac5f8b7d17..6f1dc36c591 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -780,21 +780,21 @@ void Object::Init(Isolate* isolate) { cls.set_is_declaration_loaded(); cls.set_is_type_finalized(); - // Allocate and initialize the sentinel values of Null class. + // Allocate and initialize the sentinel values. { *sentinel_ ^= - Object::Allocate(kNullCid, Instance::InstanceSize(), Heap::kOld); + Object::Allocate(kNeverCid, Instance::InstanceSize(), Heap::kOld); *transition_sentinel_ ^= - Object::Allocate(kNullCid, Instance::InstanceSize(), Heap::kOld); + Object::Allocate(kNeverCid, Instance::InstanceSize(), Heap::kOld); } // Allocate and initialize optimizing compiler constants. { *unknown_constant_ ^= - Object::Allocate(kNullCid, Instance::InstanceSize(), Heap::kOld); + Object::Allocate(kNeverCid, Instance::InstanceSize(), Heap::kOld); *non_constant_ ^= - Object::Allocate(kNullCid, Instance::InstanceSize(), Heap::kOld); + Object::Allocate(kNeverCid, Instance::InstanceSize(), Heap::kOld); } // Allocate the remaining VM internal classes. diff --git a/runtime/vm/raw_object.cc b/runtime/vm/raw_object.cc index 871aa0b1363..c2270e003ad 100644 --- a/runtime/vm/raw_object.cc +++ b/runtime/vm/raw_object.cc @@ -358,6 +358,7 @@ intptr_t RawObject::VisitPointersPredefined(ObjectPointerVisitor* visitor, break; } case kNullCid: + case kNeverCid: size = HeapSize(); break; default: