Skip to content

Commit 77e1c72

Browse files
alexmarkovcommit-bot@chromium.org
alexmarkov
authored and
[email protected]
committed
[vm/nnbd] Type testing stubs in NNBD strong mode
Issue: #38845 Change-Id: I82e7a1b3c4220abdd2a215529230fbb158adbe9e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/135627 Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Martin Kustermann <[email protected]> Reviewed-by: Régis Crelier <[email protected]>
1 parent b3a1299 commit 77e1c72

27 files changed

+536
-187
lines changed

‎runtime/vm/compiler/assembler/assembler_arm.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,11 @@ class Assembler : public AssemblerBase {
403403
cmp(value, Operand(TMP));
404404
}
405405

406+
void CompareTypeNullabilityWith(Register type, int8_t value) {
407+
ldrb(TMP, FieldAddress(type, compiler::target::Type::nullability_offset()));
408+
cmp(TMP, Operand(value));
409+
}
410+
406411
// Misc. functionality
407412
bool use_far_branches() const {
408413
return FLAG_use_far_branches || use_far_branches_;

‎runtime/vm/compiler/assembler/assembler_arm64.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,12 @@ class Assembler : public AssemblerBase {
482482
cmp(value, Operand(TMP));
483483
}
484484

485+
void CompareTypeNullabilityWith(Register type, int8_t value) {
486+
ldr(TMP, FieldAddress(type, compiler::target::Type::nullability_offset()),
487+
kUnsignedByte);
488+
cmp(TMP, Operand(value));
489+
}
490+
485491
bool use_far_branches() const {
486492
return FLAG_use_far_branches || use_far_branches_;
487493
}

‎runtime/vm/compiler/assembler/assembler_x64.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -857,6 +857,11 @@ class Assembler : public AssemblerBase {
857857
cmpq(value, address);
858858
}
859859

860+
void CompareTypeNullabilityWith(Register type, int8_t value) {
861+
cmpb(FieldAddress(type, compiler::target::Type::nullability_offset()),
862+
Immediate(value));
863+
}
864+
860865
void RestoreCodePointer();
861866
void LoadPoolPointer(Register pp = PP);
862867

‎runtime/vm/compiler/backend/flow_graph_compiler.cc

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2292,8 +2292,6 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
22922292
compiler::Label* done) {
22932293
TypeUsageInfo* type_usage_info = thread()->type_usage_info();
22942294

2295-
// TODO(regis): Take nnbd_mode() into account and pass it in a register.
2296-
22972295
// If the int type is assignable to [dst_type] we special case it on the
22982296
// caller side!
22992297
const Type& int_type = Type::Handle(zone(), Type::IntType());
@@ -2311,6 +2309,15 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
23112309
// We can handle certain types very efficiently on the call site (with a
23122310
// bailout to the normal stub, which will do a runtime call).
23132311
if (dst_type.IsTypeParameter()) {
2312+
// In NNBD strong mode we need to handle null instance before calling TTS
2313+
// if type parameter is nullable or legacy because type parameter can be
2314+
// instantiated with a non-nullable type which rejects null.
2315+
// In NNBD weak mode or if type parameter is non-nullable or has
2316+
// undetermined nullability null instance is correctly handled by TTS.
2317+
if (FLAG_null_safety && (dst_type.IsNullable() || dst_type.IsLegacy())) {
2318+
__ CompareObject(instance_reg, Object::null_object());
2319+
__ BranchIf(EQUAL, done);
2320+
}
23142321
const TypeParameter& type_param = TypeParameter::Cast(dst_type);
23152322
const Register kTypeArgumentsReg = type_param.IsClassTypeParameter()
23162323
? instantiator_type_args_reg
@@ -2338,10 +2345,10 @@ void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
23382345
const bool can_use_simple_cid_range_test =
23392346
hi->CanUseSubtypeRangeCheckFor(dst_type);
23402347
if (can_use_simple_cid_range_test) {
2341-
const CidRangeVector& ranges =
2342-
hi->SubtypeRangesForClass(type_class,
2343-
/*include_abstract=*/false,
2344-
/*exclude_null=*/false);
2348+
const CidRangeVector& ranges = hi->SubtypeRangesForClass(
2349+
type_class,
2350+
/*include_abstract=*/false,
2351+
/*exclude_null=*/!Instance::NullIsAssignableTo(dst_type));
23452352
if (ranges.length() <= kMaxNumberOfCidRangesToTest) {
23462353
if (is_non_smi) {
23472354
__ LoadClassId(scratch_reg, instance_reg);

‎runtime/vm/compiler/backend/flow_graph_compiler_arm.cc

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -701,8 +701,8 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
701701
ASSERT(!dst_type.IsNull());
702702
ASSERT(dst_type.IsFinalized());
703703
// Assignable check is skipped in FlowGraphBuilder, not here.
704-
ASSERT(!dst_type.IsDynamicType() && !dst_type.IsObjectType() &&
705-
!dst_type.IsVoidType());
704+
ASSERT(!dst_type.IsTopTypeForAssignability());
705+
706706
const Register kInstantiatorTypeArgumentsReg = R2;
707707
const Register kFunctionTypeArgumentsReg = R1;
708708

@@ -712,9 +712,10 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
712712
} else {
713713
compiler::Label is_assignable_fast, is_assignable, runtime_call;
714714

715-
// A null object is always assignable and is returned as result.
716-
__ CompareObject(R0, Object::null_object());
717-
__ b(&is_assignable_fast, EQ);
715+
if (Instance::NullIsAssignableTo(dst_type)) {
716+
__ CompareObject(R0, Object::null_object());
717+
__ b(&is_assignable_fast, EQ);
718+
}
718719

719720
__ PushList((1 << kInstantiatorTypeArgumentsReg) |
720721
(1 << kFunctionTypeArgumentsReg));

‎runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -678,8 +678,8 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
678678
ASSERT(!dst_type.IsNull());
679679
ASSERT(dst_type.IsFinalized());
680680
// Assignable check is skipped in FlowGraphBuilder, not here.
681-
ASSERT(!dst_type.IsDynamicType() && !dst_type.IsObjectType() &&
682-
!dst_type.IsVoidType());
681+
ASSERT(!dst_type.IsTopTypeForAssignability());
682+
683683
const Register kInstantiatorTypeArgumentsReg = R1;
684684
const Register kFunctionTypeArgumentsReg = R2;
685685

@@ -689,9 +689,10 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
689689
} else {
690690
compiler::Label is_assignable_fast, is_assignable, runtime_call;
691691

692-
// A null object is always assignable and is returned as result.
693-
__ CompareObject(R0, Object::null_object());
694-
__ b(&is_assignable_fast, EQ);
692+
if (Instance::NullIsAssignableTo(dst_type)) {
693+
__ CompareObject(R0, Object::null_object());
694+
__ b(&is_assignable_fast, EQ);
695+
}
695696

696697
__ PushPair(kFunctionTypeArgumentsReg, kInstantiatorTypeArgumentsReg);
697698

‎runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -682,16 +682,18 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
682682
ASSERT(!dst_type.IsNull());
683683
ASSERT(dst_type.IsFinalized());
684684
// Assignable check is skipped in FlowGraphBuilder, not here.
685-
ASSERT(!dst_type.IsDynamicType() && !dst_type.IsObjectType() &&
686-
!dst_type.IsVoidType());
685+
ASSERT(!dst_type.IsTopTypeForAssignability());
686+
687687
__ pushl(EDX); // Store instantiator type arguments.
688688
__ pushl(ECX); // Store function type arguments.
689-
// A null object is always assignable and is returned as result.
690-
const compiler::Immediate& raw_null =
691-
compiler::Immediate(reinterpret_cast<intptr_t>(Object::null()));
689+
692690
compiler::Label is_assignable, runtime_call;
693-
__ cmpl(EAX, raw_null);
694-
__ j(EQUAL, &is_assignable);
691+
if (Instance::NullIsAssignableTo(dst_type)) {
692+
const compiler::Immediate& raw_null =
693+
compiler::Immediate(reinterpret_cast<intptr_t>(Object::null()));
694+
__ cmpl(EAX, raw_null);
695+
__ j(EQUAL, &is_assignable);
696+
}
695697

696698
// Generate inline type check, linking to runtime call if not assignable.
697699
SubtypeTestCache& test_cache = SubtypeTestCache::ZoneHandle(zone());

‎runtime/vm/compiler/backend/flow_graph_compiler_x64.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -695,8 +695,7 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
695695
ASSERT(!dst_type.IsNull());
696696
ASSERT(dst_type.IsFinalized());
697697
// Assignable check is skipped in FlowGraphBuilder, not here.
698-
ASSERT(!dst_type.IsDynamicType() && !dst_type.IsObjectType() &&
699-
!dst_type.IsVoidType());
698+
ASSERT(!dst_type.IsTopTypeForAssignability());
700699

701700
const Register kInstantiatorTypeArgumentsReg = RDX;
702701
const Register kFunctionTypeArgumentsReg = RCX;
@@ -707,9 +706,10 @@ void FlowGraphCompiler::GenerateAssertAssignable(TokenPosition token_pos,
707706
} else {
708707
compiler::Label is_assignable, runtime_call;
709708

710-
// A null object is always assignable and is returned as result.
711-
__ CompareObject(RAX, Object::null_object());
712-
__ j(EQUAL, &is_assignable);
709+
if (Instance::NullIsAssignableTo(dst_type)) {
710+
__ CompareObject(RAX, Object::null_object());
711+
__ j(EQUAL, &is_assignable);
712+
}
713713

714714
// Generate inline type check, linking to runtime call if not assignable.
715715
SubtypeTestCache& test_cache = SubtypeTestCache::ZoneHandle(zone());

‎runtime/vm/compiler/backend/il.cc

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -115,20 +115,16 @@ const CidRangeVector& HierarchyInfo::SubtypeRangesForClass(
115115
bool exclude_null) {
116116
ClassTable* table = thread()->isolate()->class_table();
117117
const intptr_t cid_count = table->NumCids();
118-
CidRangeVector** cid_ranges = nullptr;
118+
std::unique_ptr<CidRangeVector[]>* cid_ranges = nullptr;
119119
if (include_abstract) {
120-
ASSERT(!exclude_null);
121-
cid_ranges = &cid_subtype_ranges_abstract_nullable_;
122-
} else if (exclude_null) {
123-
ASSERT(!include_abstract);
124-
cid_ranges = &cid_subtype_ranges_nonnullable_;
120+
cid_ranges = exclude_null ? &cid_subtype_ranges_abstract_nonnullable_
121+
: &cid_subtype_ranges_abstract_nullable_;
125122
} else {
126-
ASSERT(!include_abstract);
127-
ASSERT(!exclude_null);
128-
cid_ranges = &cid_subtype_ranges_nullable_;
123+
cid_ranges = exclude_null ? &cid_subtype_ranges_nonnullable_
124+
: &cid_subtype_ranges_nullable_;
129125
}
130126
if (*cid_ranges == nullptr) {
131-
*cid_ranges = new CidRangeVector[cid_count];
127+
cid_ranges->reset(new CidRangeVector[cid_count]);
132128
}
133129
CidRangeVector& ranges = (*cid_ranges)[klass.id()];
134130
if (ranges.length() == 0) {
@@ -147,8 +143,8 @@ const CidRangeVector& HierarchyInfo::SubclassRangesForClass(
147143
const Class& klass) {
148144
ClassTable* table = thread()->isolate()->class_table();
149145
const intptr_t cid_count = table->NumCids();
150-
if (cid_subclass_ranges_ == NULL) {
151-
cid_subclass_ranges_ = new CidRangeVector[cid_count];
146+
if (cid_subclass_ranges_ == nullptr) {
147+
cid_subclass_ranges_.reset(new CidRangeVector[cid_count]);
152148
}
153149

154150
CidRangeVector& ranges = cid_subclass_ranges_[klass.id()];
@@ -270,7 +266,6 @@ void HierarchyInfo::BuildRangesForJIT(ClassTable* table,
270266
exclude_null);
271267
return;
272268
}
273-
ASSERT(!exclude_null);
274269

275270
Zone* zone = thread()->zone();
276271
GrowableArray<intptr_t> cids;
@@ -2957,7 +2952,7 @@ Definition* AssertAssignableInstr::Canonicalize(FlowGraph* flow_graph) {
29572952
instantiator_type_arguments()->BindTo(flow_graph->constant_null());
29582953
function_type_arguments()->BindTo(flow_graph->constant_null());
29592954

2960-
if (new_dst_type.IsDynamicType() || new_dst_type.IsObjectType() ||
2955+
if (new_dst_type.IsTopTypeForAssignability() ||
29612956
(FLAG_eliminate_type_checks &&
29622957
value()->Type()->IsAssignableTo(nnbd_mode(), new_dst_type))) {
29632958
return value()->definition();

‎runtime/vm/compiler/backend/il.h

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#ifndef RUNTIME_VM_COMPILER_BACKEND_IL_H_
66
#define RUNTIME_VM_COMPILER_BACKEND_IL_H_
77

8+
#include <memory>
89
#include <utility>
910

1011
#include "vm/allocation.h"
@@ -230,27 +231,16 @@ class HierarchyInfo : public ThreadStackResource {
230231
public:
231232
explicit HierarchyInfo(Thread* thread)
232233
: ThreadStackResource(thread),
233-
cid_subtype_ranges_nullable_(NULL),
234-
cid_subtype_ranges_abstract_nullable_(NULL),
235-
cid_subtype_ranges_nonnullable_(NULL),
236-
cid_subclass_ranges_(NULL) {
234+
cid_subtype_ranges_nullable_(),
235+
cid_subtype_ranges_abstract_nullable_(),
236+
cid_subtype_ranges_nonnullable_(),
237+
cid_subtype_ranges_abstract_nonnullable_(),
238+
cid_subclass_ranges_() {
237239
thread->set_hierarchy_info(this);
238240
}
239241

240242
~HierarchyInfo() {
241243
thread()->set_hierarchy_info(NULL);
242-
243-
delete[] cid_subtype_ranges_nullable_;
244-
cid_subtype_ranges_nullable_ = NULL;
245-
246-
delete[] cid_subtype_ranges_abstract_nullable_;
247-
cid_subtype_ranges_abstract_nullable_ = NULL;
248-
249-
delete[] cid_subtype_ranges_nonnullable_;
250-
cid_subtype_ranges_nonnullable_ = NULL;
251-
252-
delete[] cid_subclass_ranges_;
253-
cid_subclass_ranges_ = NULL;
254244
}
255245

256246
const CidRangeVector& SubtypeRangesForClass(const Class& klass,
@@ -302,10 +292,11 @@ class HierarchyInfo : public ThreadStackResource {
302292
bool include_abstract,
303293
bool exclude_null);
304294

305-
CidRangeVector* cid_subtype_ranges_nullable_;
306-
CidRangeVector* cid_subtype_ranges_abstract_nullable_;
307-
CidRangeVector* cid_subtype_ranges_nonnullable_;
308-
CidRangeVector* cid_subclass_ranges_;
295+
std::unique_ptr<CidRangeVector[]> cid_subtype_ranges_nullable_;
296+
std::unique_ptr<CidRangeVector[]> cid_subtype_ranges_abstract_nullable_;
297+
std::unique_ptr<CidRangeVector[]> cid_subtype_ranges_nonnullable_;
298+
std::unique_ptr<CidRangeVector[]> cid_subtype_ranges_abstract_nonnullable_;
299+
std::unique_ptr<CidRangeVector[]> cid_subclass_ranges_;
309300
};
310301

311302
// An embedded container with N elements of type T. Used (with partial

‎runtime/vm/compiler/backend/type_propagator.cc

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -816,7 +816,7 @@ bool CompileType::IsSubtypeOf(NNBDMode mode, const AbstractType& other) {
816816
}
817817

818818
bool CompileType::IsAssignableTo(NNBDMode mode, const AbstractType& other) {
819-
if (other.IsTopType()) {
819+
if (other.IsTopTypeForAssignability()) {
820820
return true;
821821
}
822822

@@ -826,19 +826,8 @@ bool CompileType::IsAssignableTo(NNBDMode mode, const AbstractType& other) {
826826

827827
// Consider the compile type of the value.
828828
const AbstractType& compile_type = *ToAbstractType();
829-
830829
if (compile_type.IsNullType()) {
831-
if (!FLAG_null_safety) {
832-
// In weak mode, 'null' is assignable to any type.
833-
return true;
834-
}
835-
// In strong mode, 'null' is assignable to any nullable or legacy type.
836-
// It is also assignable to FutureOr<T> if it is assignable to T.
837-
const AbstractType& unwrapped_other =
838-
AbstractType::Handle(other.UnwrapFutureOr());
839-
// A nullable or legacy type parameter will still be either nullable or
840-
// legacy after instantiation.
841-
return unwrapped_other.IsNullable() || unwrapped_other.IsLegacy();
830+
return Instance::NullIsAssignableTo(other);
842831
}
843832
return compile_type.IsSubtypeOf(mode, other, Heap::kOld);
844833
}

‎runtime/vm/compiler/call_specializer.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,7 @@ bool CallSpecializer::TryInlineInstanceSetter(InstanceCallInstr* instr) {
858858

859859
// Build an AssertAssignable if necessary.
860860
const AbstractType& dst_type = AbstractType::ZoneHandle(zone(), field.type());
861-
if (I->argument_type_checks() && !dst_type.IsTopType()) {
861+
if (I->argument_type_checks() && !dst_type.IsTopTypeForAssignability()) {
862862
// Compute if we need to type check the value. Always type check if
863863
// at a dynamic invocation.
864864
bool needs_check = true;

‎runtime/vm/compiler/frontend/bytecode_reader.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,8 +418,8 @@ RawArray* BytecodeReaderHelper::CreateForwarderChecks(
418418
const bool has_optional_parameters = function.HasOptionalParameters();
419419
for (intptr_t i = function.NumImplicitParameters(); i < num_params; ++i) {
420420
type = function.ParameterTypeAt(i);
421-
if (!type.IsTopType() && !is_generic_covariant_impl.Contains(i) &&
422-
!is_covariant.Contains(i)) {
421+
if (!type.IsTopTypeForAssignability() &&
422+
!is_generic_covariant_impl.Contains(i) && !is_covariant.Contains(i)) {
423423
name = function.ParameterNameAt(i);
424424
intptr_t index;
425425
if (i >= num_pos_params) {

‎runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3685,7 +3685,7 @@ Fragment StreamingFlowGraphBuilder::BuildAsExpression(TokenPosition* p) {
36853685
Fragment instructions = BuildExpression(); // read operand.
36863686

36873687
const AbstractType& type = T.BuildType(); // read type.
3688-
if (type.IsInstantiated() && type.IsTopType()) {
3688+
if (type.IsInstantiated() && type.IsTopTypeForAssignability()) {
36893689
// We already evaluated the operand on the left and just leave it there as
36903690
// the result of the `obj as dynamic` expression.
36913691
} else {

‎runtime/vm/compiler/frontend/kernel_to_il.cc

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1589,8 +1589,7 @@ Fragment FlowGraphBuilder::CheckAssignable(const AbstractType& dst_type,
15891589
if (!I->should_emit_strong_mode_checks()) {
15901590
return Fragment();
15911591
}
1592-
if (!dst_type.IsDynamicType() && !dst_type.IsObjectType() &&
1593-
!dst_type.IsVoidType()) {
1592+
if (!dst_type.IsTopTypeForAssignability()) {
15941593
LocalVariable* top_of_stack = MakeTemporary();
15951594
instructions += LoadLocal(top_of_stack);
15961595
instructions += AssertAssignableLoadTypeArguments(TokenPosition::kNoSource,
@@ -1738,7 +1737,7 @@ void FlowGraphBuilder::BuildArgumentTypeChecks(
17381737
&AbstractType::ZoneHandle(Z, forwarding_target->ParameterTypeAt(i));
17391738
}
17401739

1741-
if (target_type->IsTopType()) continue;
1740+
if (target_type->IsTopTypeForAssignability()) continue;
17421741

17431742
const bool is_covariant = param->is_explicit_covariant_parameter();
17441743
Fragment* checks = is_covariant ? explicit_checks : implicit_checks;
@@ -2294,8 +2293,7 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfNoSuchMethodForwarder(
22942293
body += Drop(); // argument count
22952294

22962295
AbstractType& return_type = AbstractType::Handle(function.result_type());
2297-
if (!return_type.IsDynamicType() && !return_type.IsVoidType() &&
2298-
!return_type.IsObjectType()) {
2296+
if (!return_type.IsTopTypeForAssignability()) {
22992297
body += AssertAssignableLoadTypeArguments(TokenPosition::kNoSource,
23002298
return_type, Symbols::Empty());
23012299
}

0 commit comments

Comments
 (0)