Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 11ad25a

Browse files
creliercommit-bot@chromium.org
crelier
authored and
[email protected]
committed
Reland "[VM runtime] Support Smi instances in type test cache."
This is a reland of 6ba3e55 The issue was that SlowTypeTestStub used in precompiled mode did not handle a Smi instance before calling the Subtype2TestCache stub which does not support it. See PatchSet 2 for the fix. Is there a more efficient solution? Original change's description: > [VM runtime] Support Smi instances in type test cache. > > This adds SubtypeTestCache-based optimizations for type tests against > * dst_type = FutureOr<T> (when T=int/num) > * dst_type = T (when T = FutureOr<int/num>) > > Remove dangerous LoadClass pseudo assembler instruction (does not work for Smi). > Handle instantiated void in type tests (along with dynamic and Object). > > Change-Id: I0df0fc72ff173b9464d16cc971969132b055a429 > Reviewed-on: https://dart-review.googlesource.com/c/81182 > Commit-Queue: Régis Crelier <[email protected]> > Reviewed-by: Martin Kustermann <[email protected]> Change-Id: I333ca47aebd7f0b663059ab6afc5d1cd8d7d5210 Reviewed-on: https://dart-review.googlesource.com/c/81320 Commit-Queue: Régis Crelier <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
1 parent 3dc9119 commit 11ad25a

18 files changed

+150
-123
lines changed

‎runtime/vm/compiler/aot/precompiler.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,7 @@ void Precompiler::AddConstObject(const Instance& instance) {
705705
return;
706706
}
707707

708-
// Can't ask immediate objects if they're canoncial.
708+
// Can't ask immediate objects if they're canonical.
709709
if (instance.IsSmi()) return;
710710

711711
// Some Instances in the ObjectPool aren't const objects, such as

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1825,12 +1825,6 @@ void Assembler::LoadClassById(Register result, Register class_id) {
18251825
ldr(result, Address(result, class_id, LSL, kSizeOfClassPairLog2));
18261826
}
18271827

1828-
void Assembler::LoadClass(Register result, Register object, Register scratch) {
1829-
ASSERT(scratch != result);
1830-
LoadClassId(scratch, object);
1831-
LoadClassById(result, scratch);
1832-
}
1833-
18341828
void Assembler::CompareClassId(Register object,
18351829
intptr_t class_id,
18361830
Register scratch) {

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -860,7 +860,6 @@ class Assembler : public ValueObject {
860860

861861
void LoadClassId(Register result, Register object, Condition cond = AL);
862862
void LoadClassById(Register result, Register class_id);
863-
void LoadClass(Register result, Register object, Register scratch);
864863
void CompareClassId(Register object, intptr_t class_id, Register scratch);
865864
void LoadClassIdMayBeSmi(Register result, Register object);
866865
void LoadTaggedClassIdMayBeSmi(Register result, Register object);

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,12 +1115,6 @@ void Assembler::LoadClassById(Register result, Register class_id) {
11151115
ldr(result, Address(result, class_id, UXTX, Address::Scaled));
11161116
}
11171117

1118-
void Assembler::LoadClass(Register result, Register object) {
1119-
ASSERT(object != TMP);
1120-
LoadClassId(TMP, object);
1121-
LoadClassById(result, TMP);
1122-
}
1123-
11241118
void Assembler::CompareClassId(Register object,
11251119
intptr_t class_id,
11261120
Register scratch) {

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1531,7 +1531,6 @@ class Assembler : public ValueObject {
15311531
void LoadClassId(Register result, Register object);
15321532
// Overwrites class_id register (it will be tagged afterwards).
15331533
void LoadClassById(Register result, Register class_id);
1534-
void LoadClass(Register result, Register object);
15351534
void CompareClassId(Register object,
15361535
intptr_t class_id,
15371536
Register scratch = kNoRegister);

‎runtime/vm/compiler/assembler/assembler_ia32.cc‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2434,12 +2434,6 @@ void Assembler::LoadClassById(Register result, Register class_id) {
24342434
movl(result, Address(result, class_id, TIMES_8, 0));
24352435
}
24362436

2437-
void Assembler::LoadClass(Register result, Register object, Register scratch) {
2438-
ASSERT(scratch != result);
2439-
LoadClassId(scratch, object);
2440-
LoadClassById(result, scratch);
2441-
}
2442-
24432437
void Assembler::CompareClassId(Register object,
24442438
intptr_t class_id,
24452439
Register scratch) {

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -666,8 +666,6 @@ class Assembler : public ValueObject {
666666

667667
void LoadClassById(Register result, Register class_id);
668668

669-
void LoadClass(Register result, Register object, Register scratch);
670-
671669
void CompareClassId(Register object, intptr_t class_id, Register scratch);
672670

673671
void LoadClassIdMayBeSmi(Register result, Register object);

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1952,11 +1952,6 @@ void Assembler::LoadClassById(Register result, Register class_id) {
19521952
movq(result, Address(result, class_id, TIMES_8, 0));
19531953
}
19541954

1955-
void Assembler::LoadClass(Register result, Register object) {
1956-
LoadClassId(TMP, object);
1957-
LoadClassById(result, TMP);
1958-
}
1959-
19601955
void Assembler::CompareClassId(Register object,
19611956
intptr_t class_id,
19621957
Register scratch) {

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -782,8 +782,6 @@ class Assembler : public ValueObject {
782782
// Overwrites class_id register (it will be tagged afterwards).
783783
void LoadClassById(Register result, Register class_id);
784784

785-
void LoadClass(Register result, Register object);
786-
787785
void CompareClassId(Register object,
788786
intptr_t class_id,
789787
Register scratch = kNoRegister);

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

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -248,13 +248,14 @@ FlowGraphCompiler::GenerateInstantiatedTypeWithArgumentsTest(
248248
ASSERT(type_class.NumTypeArguments() > 0);
249249
const Register kInstanceReg = R0;
250250
Error& bound_error = Error::Handle(zone());
251-
const Type& int_type = Type::Handle(zone(), Type::IntType());
251+
const Type& smi_type = Type::Handle(zone(), Type::SmiType());
252252
const bool smi_is_ok =
253-
int_type.IsSubtypeOf(type, &bound_error, NULL, Heap::kOld);
253+
smi_type.IsSubtypeOf(type, &bound_error, NULL, Heap::kOld);
254254
// Malformed type should have been handled at graph construction time.
255255
ASSERT(smi_is_ok || bound_error.IsNull());
256256
__ tst(kInstanceReg, Operand(kSmiTagMask));
257257
if (smi_is_ok) {
258+
// Fast case for type = FutureOr<int/num/top-type>.
258259
__ b(is_instance_lbl, EQ);
259260
} else {
260261
__ b(is_not_instance_lbl, EQ);
@@ -286,7 +287,7 @@ FlowGraphCompiler::GenerateInstantiatedTypeWithArgumentsTest(
286287
ASSERT(!tp_argument.IsMalformed());
287288
if (tp_argument.IsType()) {
288289
ASSERT(tp_argument.HasResolvedTypeClass());
289-
// Check if type argument is dynamic or Object.
290+
// Check if type argument is dynamic, Object, or void.
290291
const Type& object_type = Type::Handle(zone(), Type::ObjectType());
291292
if (object_type.IsSubtypeOf(tp_argument, NULL, NULL, Heap::kOld)) {
292293
// Instance class test only necessary.
@@ -341,6 +342,7 @@ bool FlowGraphCompiler::GenerateInstantiatedTypeNoArgumentsTest(
341342
if (smi_class.IsSubtypeOf(Object::null_type_arguments(), type_class,
342343
Object::null_type_arguments(), NULL, NULL,
343344
Heap::kOld)) {
345+
// Fast case for type = int/num/top-type.
344346
__ b(is_instance_lbl, EQ);
345347
} else {
346348
__ b(is_not_instance_lbl, EQ);
@@ -398,7 +400,14 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateSubtype1TestCacheLookup(
398400
Label* is_not_instance_lbl) {
399401
__ Comment("Subtype1TestCacheLookup");
400402
const Register kInstanceReg = R0;
401-
__ LoadClass(R1, kInstanceReg, R2);
403+
#if defined(DEBUG)
404+
Label ok;
405+
__ BranchIfNotSmi(kInstanceReg, &ok);
406+
__ Breakpoint();
407+
__ Bind(&ok);
408+
#endif
409+
__ LoadClassId(R2, kInstanceReg);
410+
__ LoadClassById(R1, R2);
402411
// R1: instance class.
403412
// Check immediate superclass equality.
404413
__ ldr(R2, FieldAddress(R1, Class::super_type_offset()));
@@ -447,12 +456,13 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateUninstantiatedTypeTest(
447456
__ ldr(R3, FieldAddress(kTypeArgumentsReg,
448457
TypeArguments::type_at_offset(type_param.index())));
449458
// R3: concrete type of type.
450-
// Check if type argument is dynamic.
459+
// Check if type argument is dynamic, Object, or void.
451460
__ CompareObject(R3, Object::dynamic_type());
452461
__ b(is_instance_lbl, EQ);
453462
__ CompareObject(R3, Type::ZoneHandle(zone(), Type::ObjectType()));
454463
__ b(is_instance_lbl, EQ);
455-
// TODO(regis): Optimize void type as well once allowed as type argument.
464+
__ CompareObject(R3, Object::void_type());
465+
__ b(is_instance_lbl, EQ);
456466

457467
// For Smi check quickly against int and num interfaces.
458468
Label not_smi;
@@ -462,9 +472,8 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateUninstantiatedTypeTest(
462472
__ b(is_instance_lbl, EQ);
463473
__ CompareObject(R3, Type::ZoneHandle(zone(), Type::Number()));
464474
__ b(is_instance_lbl, EQ);
465-
// Smi must be handled in runtime.
466-
Label fall_through;
467-
__ b(&fall_through);
475+
// Smi can be handled by type test cache.
476+
__ Bind(&not_smi);
468477

469478
// If it's guaranteed, by type-parameter bound, that the type parameter will
470479
// never have a value of a function type, then we can safely do a 4-type
@@ -474,17 +483,18 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateUninstantiatedTypeTest(
474483
? kTestTypeSixArgs
475484
: kTestTypeFourArgs;
476485

477-
__ Bind(&not_smi);
478486
const SubtypeTestCache& type_test_cache = SubtypeTestCache::ZoneHandle(
479487
zone(), GenerateCallSubtypeTestStub(
480488
test_kind, kInstanceReg, kInstantiatorTypeArgumentsReg,
481489
kFunctionTypeArgumentsReg, kTempReg, is_instance_lbl,
482490
is_not_instance_lbl));
483-
__ Bind(&fall_through);
484491
return type_test_cache.raw();
485492
}
486493
if (type.IsType()) {
487-
__ BranchIfSmi(kInstanceReg, is_not_instance_lbl);
494+
// Smi is FutureOr<T>, when T is a top type or int or num.
495+
if (!FLAG_strong || !Class::Handle(type.type_class()).IsFutureOrClass()) {
496+
__ BranchIfSmi(kInstanceReg, is_not_instance_lbl);
497+
}
488498
__ ldm(IA, SP,
489499
(1 << kFunctionTypeArgumentsReg) |
490500
(1 << kInstantiatorTypeArgumentsReg));

0 commit comments

Comments
 (0)