Skip to content

Commit 575a8f8

Browse files
mkustermanncommit-bot@chromium.org
authored andcommitted
[VM] Extend subtype-test mechanism with support for generic methods
Until now the subtype-test cache mechanism did not work (i.e. could return the wrong result) for partially instantiated generic closures. Additionally, closures which close over generic methods were always handled in runtime. This caused a servere performance regression for any code hitting this (e.g. code which uses `package:stack_trace`). Fixes dart-lang/sdk#34051 Fixes dart-lang/sdk#34054 Change-Id: Idb73e6f348c2fe0c737f42c57009f5f7a636c9a6 Reviewed-on: https://dart-review.googlesource.com/68369 Commit-Queue: Martin Kustermann <[email protected]> Reviewed-by: Régis Crelier <[email protected]>
1 parent 278d962 commit 575a8f8

18 files changed

Lines changed: 1016 additions & 590 deletions

runtime/vm/compiler/assembler/assembler_arm64.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1511,7 +1511,7 @@ class Assembler : public ValueObject {
15111511
void CompareObject(Register reg, const Object& object);
15121512

15131513
void LoadClassId(Register result, Register object);
1514-
// Overwrites class_id register.
1514+
// Overwrites class_id register (it will be tagged afterwards).
15151515
void LoadClassById(Register result, Register class_id);
15161516
void LoadClass(Register result, Register object);
15171517
void CompareClassId(Register object,

runtime/vm/compiler/assembler/assembler_x64.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ class Assembler : public ValueObject {
771771
// Loading and comparing classes of objects.
772772
void LoadClassId(Register result, Register object);
773773

774-
// Overwrites class_id register.
774+
// Overwrites class_id register (it will be tagged afterwards).
775775
void LoadClassById(Register result, Register class_id);
776776

777777
void LoadClass(Register result, Register object);

runtime/vm/compiler/backend/flow_graph_compiler.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,11 @@ class FlowGraphCompiler : public ValueObject {
815815
Label* is_instance_lbl,
816816
Label* is_not_instance_label);
817817

818+
RawSubtypeTestCache* GenerateFunctionTypeTest(TokenPosition token_pos,
819+
const AbstractType& dst_type,
820+
Label* is_instance_lbl,
821+
Label* is_not_instance_label);
822+
818823
RawSubtypeTestCache* GenerateSubtype1TestCacheLookup(
819824
TokenPosition token_pos,
820825
const Class& type_class,
@@ -825,6 +830,7 @@ class FlowGraphCompiler : public ValueObject {
825830
kTestTypeOneArg,
826831
kTestTypeTwoArgs,
827832
kTestTypeFourArgs,
833+
kTestTypeSixArgs,
828834
};
829835

830836
RawSubtypeTestCache* GenerateCallSubtypeTestStub(

runtime/vm/compiler/backend/flow_graph_compiler_arm.cc

Lines changed: 90 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,10 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateCallSubtypeTestStub(
223223
ASSERT(instantiator_type_arguments_reg == R2);
224224
ASSERT(function_type_arguments_reg == R1);
225225
__ BranchLink(*StubCode::Subtype4TestCache_entry());
226+
} else if (test_kind == kTestTypeSixArgs) {
227+
ASSERT(instantiator_type_arguments_reg == R2);
228+
ASSERT(function_type_arguments_reg == R1);
229+
__ BranchLink(*StubCode::Subtype6TestCache_entry());
226230
} else {
227231
UNREACHABLE();
228232
}
@@ -244,8 +248,9 @@ FlowGraphCompiler::GenerateInstantiatedTypeWithArgumentsTest(
244248
Label* is_not_instance_lbl) {
245249
__ Comment("InstantiatedTypeWithArgumentsTest");
246250
ASSERT(type.IsInstantiated());
251+
ASSERT(!type.IsFunctionType());
247252
const Class& type_class = Class::ZoneHandle(zone(), type.type_class());
248-
ASSERT(type.IsFunctionType() || (type_class.NumTypeArguments() > 0));
253+
ASSERT(type_class.NumTypeArguments() > 0);
249254
const Register kInstanceReg = R0;
250255
Error& bound_error = Error::Handle(zone());
251256
const Type& int_type = Type::Handle(zone(), Type::IntType());
@@ -259,45 +264,43 @@ FlowGraphCompiler::GenerateInstantiatedTypeWithArgumentsTest(
259264
} else {
260265
__ b(is_not_instance_lbl, EQ);
261266
}
262-
// A function type test requires checking the function signature.
263-
if (!type.IsFunctionType()) {
264-
const intptr_t num_type_args = type_class.NumTypeArguments();
265-
const intptr_t num_type_params = type_class.NumTypeParameters();
266-
const intptr_t from_index = num_type_args - num_type_params;
267-
const TypeArguments& type_arguments =
268-
TypeArguments::ZoneHandle(zone(), type.arguments());
269-
const bool is_raw_type = type_arguments.IsNull() ||
270-
type_arguments.IsRaw(from_index, num_type_params);
271-
if (is_raw_type) {
272-
const Register kClassIdReg = R2;
273-
// dynamic type argument, check only classes.
274-
__ LoadClassId(kClassIdReg, kInstanceReg);
275-
__ CompareImmediate(kClassIdReg, type_class.id());
276-
__ b(is_instance_lbl, EQ);
277-
// List is a very common case.
278-
if (IsListClass(type_class)) {
279-
GenerateListTypeCheck(kClassIdReg, is_instance_lbl);
280-
}
281-
return GenerateSubtype1TestCacheLookup(
282-
token_pos, type_class, is_instance_lbl, is_not_instance_lbl);
267+
const intptr_t num_type_args = type_class.NumTypeArguments();
268+
const intptr_t num_type_params = type_class.NumTypeParameters();
269+
const intptr_t from_index = num_type_args - num_type_params;
270+
const TypeArguments& type_arguments =
271+
TypeArguments::ZoneHandle(zone(), type.arguments());
272+
const bool is_raw_type = type_arguments.IsNull() ||
273+
type_arguments.IsRaw(from_index, num_type_params);
274+
if (is_raw_type) {
275+
const Register kClassIdReg = R2;
276+
// dynamic type argument, check only classes.
277+
__ LoadClassId(kClassIdReg, kInstanceReg);
278+
__ CompareImmediate(kClassIdReg, type_class.id());
279+
__ b(is_instance_lbl, EQ);
280+
// List is a very common case.
281+
if (IsListClass(type_class)) {
282+
GenerateListTypeCheck(kClassIdReg, is_instance_lbl);
283283
}
284-
// If one type argument only, check if type argument is Object or dynamic.
285-
if (type_arguments.Length() == 1) {
286-
const AbstractType& tp_argument =
287-
AbstractType::ZoneHandle(zone(), type_arguments.TypeAt(0));
288-
ASSERT(!tp_argument.IsMalformed());
289-
if (tp_argument.IsType()) {
290-
ASSERT(tp_argument.HasResolvedTypeClass());
291-
// Check if type argument is dynamic or Object.
292-
const Type& object_type = Type::Handle(zone(), Type::ObjectType());
293-
if (object_type.IsSubtypeOf(tp_argument, NULL, NULL, Heap::kOld)) {
294-
// Instance class test only necessary.
295-
return GenerateSubtype1TestCacheLookup(
296-
token_pos, type_class, is_instance_lbl, is_not_instance_lbl);
297-
}
284+
return GenerateSubtype1TestCacheLookup(
285+
token_pos, type_class, is_instance_lbl, is_not_instance_lbl);
286+
}
287+
// If one type argument only, check if type argument is Object or dynamic.
288+
if (type_arguments.Length() == 1) {
289+
const AbstractType& tp_argument =
290+
AbstractType::ZoneHandle(zone(), type_arguments.TypeAt(0));
291+
ASSERT(!tp_argument.IsMalformed());
292+
if (tp_argument.IsType()) {
293+
ASSERT(tp_argument.HasResolvedTypeClass());
294+
// Check if type argument is dynamic or Object.
295+
const Type& object_type = Type::Handle(zone(), Type::ObjectType());
296+
if (object_type.IsSubtypeOf(tp_argument, NULL, NULL, Heap::kOld)) {
297+
// Instance class test only necessary.
298+
return GenerateSubtype1TestCacheLookup(
299+
token_pos, type_class, is_instance_lbl, is_not_instance_lbl);
298300
}
299301
}
300302
}
303+
301304
// Regular subtype test cache involving instance's type arguments.
302305
const Register kInstantiatorTypeArgumentsReg = kNoRegister;
303306
const Register kFunctionTypeArgumentsReg = kNoRegister;
@@ -332,10 +335,7 @@ bool FlowGraphCompiler::GenerateInstantiatedTypeNoArgumentsTest(
332335
Label* is_not_instance_lbl) {
333336
__ Comment("InstantiatedTypeNoArgumentsTest");
334337
ASSERT(type.IsInstantiated());
335-
if (type.IsFunctionType()) {
336-
// Fallthrough.
337-
return true;
338-
}
338+
ASSERT(!type.IsFunctionType());
339339
const Class& type_class = Class::Handle(zone(), type.type_class());
340340
ASSERT(type_class.NumTypeArguments() == 0);
341341

@@ -428,12 +428,17 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateUninstantiatedTypeTest(
428428
Label* is_instance_lbl,
429429
Label* is_not_instance_lbl) {
430430
__ Comment("UninstantiatedTypeTest");
431+
const Register kInstanceReg = R0;
432+
const Register kInstantiatorTypeArgumentsReg = R2;
433+
const Register kFunctionTypeArgumentsReg = R1;
434+
const Register kTempReg = kNoRegister;
431435
ASSERT(!type.IsInstantiated());
436+
ASSERT(!type.IsFunctionType());
432437
// Skip check if destination is a dynamic type.
433438
if (type.IsTypeParameter()) {
434439
const TypeParameter& type_param = TypeParameter::Cast(type);
435-
const Register kInstantiatorTypeArgumentsReg = R2;
436-
const Register kFunctionTypeArgumentsReg = R1;
440+
const AbstractType& bound = AbstractType::Handle(type_param.bound());
441+
437442
__ ldm(IA, SP,
438443
(1 << kFunctionTypeArgumentsReg) |
439444
(1 << kInstantiatorTypeArgumentsReg));
@@ -466,32 +471,30 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateUninstantiatedTypeTest(
466471
Label fall_through;
467472
__ b(&fall_through);
468473

474+
// If it's guaranteed, by type-parameter bound, that the type parameter will
475+
// never have a value of a function type, then we can safely do a 4-type
476+
// test instead of a 6-type test.
477+
auto test_kind = !bound.IsTopType() && !bound.IsFunctionType() &&
478+
!bound.IsDartFunctionType() && bound.IsType()
479+
? kTestTypeSixArgs
480+
: kTestTypeFourArgs;
481+
469482
__ Bind(&not_smi);
470-
// R0: instance.
471-
// R2: instantiator type arguments.
472-
// R1: function type arguments.
473-
const Register kInstanceReg = R0;
474-
const Register kTempReg = kNoRegister;
475483
const SubtypeTestCache& type_test_cache = SubtypeTestCache::ZoneHandle(
476484
zone(), GenerateCallSubtypeTestStub(
477-
kTestTypeFourArgs, kInstanceReg,
478-
kInstantiatorTypeArgumentsReg, kFunctionTypeArgumentsReg,
479-
kTempReg, is_instance_lbl, is_not_instance_lbl));
485+
test_kind, kInstanceReg, kInstantiatorTypeArgumentsReg,
486+
kFunctionTypeArgumentsReg, kTempReg, is_instance_lbl,
487+
is_not_instance_lbl));
480488
__ Bind(&fall_through);
481489
return type_test_cache.raw();
482490
}
483491
if (type.IsType()) {
484-
const Register kInstanceReg = R0;
485-
const Register kInstantiatorTypeArgumentsReg = R2;
486-
const Register kFunctionTypeArgumentsReg = R1;
487-
__ tst(kInstanceReg, Operand(kSmiTagMask)); // Is instance Smi?
488-
__ b(is_not_instance_lbl, EQ);
492+
__ BranchIfSmi(kInstanceReg, is_not_instance_lbl);
489493
__ ldm(IA, SP,
490494
(1 << kFunctionTypeArgumentsReg) |
491495
(1 << kInstantiatorTypeArgumentsReg));
492496
// Uninstantiated type class is known at compile time, but the type
493497
// arguments are determined at runtime by the instantiator(s).
494-
const Register kTempReg = kNoRegister;
495498
return GenerateCallSubtypeTestStub(kTestTypeFourArgs, kInstanceReg,
496499
kInstantiatorTypeArgumentsReg,
497500
kFunctionTypeArgumentsReg, kTempReg,
@@ -500,6 +503,30 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateUninstantiatedTypeTest(
500503
return SubtypeTestCache::null();
501504
}
502505

506+
// Generates function type check.
507+
//
508+
// See [GenerateUninstantiatedTypeTest] for calling convention.
509+
RawSubtypeTestCache* FlowGraphCompiler::GenerateFunctionTypeTest(
510+
TokenPosition token_pos,
511+
const AbstractType& type,
512+
Label* is_instance_lbl,
513+
Label* is_not_instance_lbl) {
514+
const Register kInstanceReg = R0;
515+
const Register kInstantiatorTypeArgumentsReg = R2;
516+
const Register kFunctionTypeArgumentsReg = R1;
517+
__ BranchIfSmi(kInstanceReg, is_not_instance_lbl);
518+
__ ldm(
519+
IA, SP,
520+
(1 << kFunctionTypeArgumentsReg) | (1 << kInstantiatorTypeArgumentsReg));
521+
// Uninstantiated type class is known at compile time, but the type
522+
// arguments are determined at runtime by the instantiator(s).
523+
const Register kTempReg = kNoRegister;
524+
return GenerateCallSubtypeTestStub(kTestTypeSixArgs, kInstanceReg,
525+
kInstantiatorTypeArgumentsReg,
526+
kFunctionTypeArgumentsReg, kTempReg,
527+
is_instance_lbl, is_not_instance_lbl);
528+
}
529+
503530
// Inputs:
504531
// - R0: instance being type checked (preserved).
505532
// - R2: optional instantiator type arguments (preserved).
@@ -517,12 +544,18 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateInlineInstanceof(
517544
Label* is_instance_lbl,
518545
Label* is_not_instance_lbl) {
519546
__ Comment("InlineInstanceof");
547+
548+
if (type.IsFunctionType()) {
549+
return GenerateFunctionTypeTest(token_pos, type, is_instance_lbl,
550+
is_not_instance_lbl);
551+
}
552+
520553
if (type.IsInstantiated()) {
521554
const Class& type_class = Class::ZoneHandle(zone(), type.type_class());
522555
// A class equality check is only applicable with a dst type (not a
523556
// function type) of a non-parameterized class or with a raw dst type of
524557
// a parameterized class.
525-
if (type.IsFunctionType() || (type_class.NumTypeArguments() > 0)) {
558+
if (type_class.NumTypeArguments() > 0) {
526559
return GenerateInstantiatedTypeWithArgumentsTest(
527560
token_pos, type, is_instance_lbl, is_not_instance_lbl);
528561
// Fall through to runtime call.

0 commit comments

Comments
 (0)