Skip to content

Commit

Permalink
Revert "[VM runtime] Support Smi instances in type test cache."
Browse files Browse the repository at this point in the history 
This reverts commit 6ba3e55.

Reason for revert: unhappy kernel-precomp bots

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 <regis@google.com>
> Reviewed-by: Martin Kustermann <kustermann@google.com>

TBR=kustermann@google.com,alexmarkov@google.com,regis@google.com

Change-Id: I73be5fc068cd24e0a13ba0872a99a24ab5a8eeca
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/81284
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Régis Crelier <regis@google.com>
  • Loading branch information
@crelier
crelier authored and commit-bot@chromium.org committed Oct 23, 2018
1 parent 4e49d19 commit 6134ac8
Show file tree
Hide file tree
Showing 17 changed files with 109 additions and 122 deletions.
6 changes: 6 additions & 0 deletions runtime/vm/compiler/assembler/assembler_arm.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1825,6 +1825,12 @@ void Assembler::LoadClassById(Register result, Register class_id) {
ldr(result, Address(result, class_id, LSL, kSizeOfClassPairLog2));
}

void Assembler::LoadClass(Register result, Register object, Register scratch) {
ASSERT(scratch != result);
LoadClassId(scratch, object);
LoadClassById(result, scratch);
}

void Assembler::CompareClassId(Register object,
intptr_t class_id,
Register scratch) {
Expand Down
1 change: 1 addition & 0 deletions runtime/vm/compiler/assembler/assembler_arm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,7 @@ class Assembler : public ValueObject {

void LoadClassId(Register result, Register object, Condition cond = AL);
void LoadClassById(Register result, Register class_id);
void LoadClass(Register result, Register object, Register scratch);
void CompareClassId(Register object, intptr_t class_id, Register scratch);
void LoadClassIdMayBeSmi(Register result, Register object);
void LoadTaggedClassIdMayBeSmi(Register result, Register object);
Expand Down
6 changes: 6 additions & 0 deletions runtime/vm/compiler/assembler/assembler_arm64.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1115,6 +1115,12 @@ void Assembler::LoadClassById(Register result, Register class_id) {
ldr(result, Address(result, class_id, UXTX, Address::Scaled));
}

void Assembler::LoadClass(Register result, Register object) {
ASSERT(object != TMP);
LoadClassId(TMP, object);
LoadClassById(result, TMP);
}

void Assembler::CompareClassId(Register object,
intptr_t class_id,
Register scratch) {
Expand Down
1 change: 1 addition & 0 deletions runtime/vm/compiler/assembler/assembler_arm64.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1531,6 +1531,7 @@ class Assembler : public ValueObject {
void LoadClassId(Register result, Register object);
// Overwrites class_id register (it will be tagged afterwards).
void LoadClassById(Register result, Register class_id);
void LoadClass(Register result, Register object);
void CompareClassId(Register object,
intptr_t class_id,
Register scratch = kNoRegister);
Expand Down
6 changes: 6 additions & 0 deletions runtime/vm/compiler/assembler/assembler_ia32.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2434,6 +2434,12 @@ void Assembler::LoadClassById(Register result, Register class_id) {
movl(result, Address(result, class_id, TIMES_8, 0));
}

void Assembler::LoadClass(Register result, Register object, Register scratch) {
ASSERT(scratch != result);
LoadClassId(scratch, object);
LoadClassById(result, scratch);
}

void Assembler::CompareClassId(Register object,
intptr_t class_id,
Register scratch) {
Expand Down
2 changes: 2 additions & 0 deletions runtime/vm/compiler/assembler/assembler_ia32.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,8 @@ class Assembler : public ValueObject {

void LoadClassById(Register result, Register class_id);

void LoadClass(Register result, Register object, Register scratch);

void CompareClassId(Register object, intptr_t class_id, Register scratch);

void LoadClassIdMayBeSmi(Register result, Register object);
Expand Down
5 changes: 5 additions & 0 deletions runtime/vm/compiler/assembler/assembler_x64.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1952,6 +1952,11 @@ void Assembler::LoadClassById(Register result, Register class_id) {
movq(result, Address(result, class_id, TIMES_8, 0));
}

void Assembler::LoadClass(Register result, Register object) {
LoadClassId(TMP, object);
LoadClassById(result, TMP);
}

void Assembler::CompareClassId(Register object,
intptr_t class_id,
Register scratch) {
Expand Down
2 changes: 2 additions & 0 deletions runtime/vm/compiler/assembler/assembler_x64.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,8 @@ class Assembler : public ValueObject {
// Overwrites class_id register (it will be tagged afterwards).
void LoadClassById(Register result, Register class_id);

void LoadClass(Register result, Register object);

void CompareClassId(Register object,
intptr_t class_id,
Register scratch = kNoRegister);
Expand Down
34 changes: 12 additions & 22 deletions runtime/vm/compiler/backend/flow_graph_compiler_arm.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,14 +248,13 @@ FlowGraphCompiler::GenerateInstantiatedTypeWithArgumentsTest(
ASSERT(type_class.NumTypeArguments() > 0);
const Register kInstanceReg = R0;
Error& bound_error = Error::Handle(zone());
const Type& smi_type = Type::Handle(zone(), Type::SmiType());
const Type& int_type = Type::Handle(zone(), Type::IntType());
const bool smi_is_ok =
smi_type.IsSubtypeOf(type, &bound_error, NULL, Heap::kOld);
int_type.IsSubtypeOf(type, &bound_error, NULL, Heap::kOld);
// Malformed type should have been handled at graph construction time.
ASSERT(smi_is_ok || bound_error.IsNull());
__ tst(kInstanceReg, Operand(kSmiTagMask));
if (smi_is_ok) {
// Fast case for type = FutureOr<int/num/top-type>.
__ b(is_instance_lbl, EQ);
} else {
__ b(is_not_instance_lbl, EQ);
Expand Down Expand Up @@ -287,7 +286,7 @@ FlowGraphCompiler::GenerateInstantiatedTypeWithArgumentsTest(
ASSERT(!tp_argument.IsMalformed());
if (tp_argument.IsType()) {
ASSERT(tp_argument.HasResolvedTypeClass());
// Check if type argument is dynamic, Object, or void.
// Check if type argument is dynamic or Object.
const Type& object_type = Type::Handle(zone(), Type::ObjectType());
if (object_type.IsSubtypeOf(tp_argument, NULL, NULL, Heap::kOld)) {
// Instance class test only necessary.
Expand Down Expand Up @@ -342,7 +341,6 @@ bool FlowGraphCompiler::GenerateInstantiatedTypeNoArgumentsTest(
if (smi_class.IsSubtypeOf(Object::null_type_arguments(), type_class,
Object::null_type_arguments(), NULL, NULL,
Heap::kOld)) {
// Fast case for type = int/num/top-type.
__ b(is_instance_lbl, EQ);
} else {
__ b(is_not_instance_lbl, EQ);
Expand Down Expand Up @@ -400,14 +398,7 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateSubtype1TestCacheLookup(
Label* is_not_instance_lbl) {
__ Comment("Subtype1TestCacheLookup");
const Register kInstanceReg = R0;
#if defined(DEBUG)
Label ok;
__ BranchIfNotSmi(kInstanceReg, &ok);
__ Breakpoint();
__ Bind(&ok);
#endif
__ LoadClassId(R2, kInstanceReg);
__ LoadClassById(R1, R2);
__ LoadClass(R1, kInstanceReg, R2);
// R1: instance class.
// Check immediate superclass equality.
__ ldr(R2, FieldAddress(R1, Class::super_type_offset()));
Expand Down Expand Up @@ -456,13 +447,12 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateUninstantiatedTypeTest(
__ ldr(R3, FieldAddress(kTypeArgumentsReg,
TypeArguments::type_at_offset(type_param.index())));
// R3: concrete type of type.
// Check if type argument is dynamic, Object, or void.
// Check if type argument is dynamic.
__ CompareObject(R3, Object::dynamic_type());
__ b(is_instance_lbl, EQ);
__ CompareObject(R3, Type::ZoneHandle(zone(), Type::ObjectType()));
__ b(is_instance_lbl, EQ);
__ CompareObject(R3, Object::void_type());
__ b(is_instance_lbl, EQ);
// TODO(regis): Optimize void type as well once allowed as type argument.

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

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

__ Bind(&not_smi);
const SubtypeTestCache& type_test_cache = SubtypeTestCache::ZoneHandle(
zone(), GenerateCallSubtypeTestStub(
test_kind, kInstanceReg, kInstantiatorTypeArgumentsReg,
kFunctionTypeArgumentsReg, kTempReg, is_instance_lbl,
is_not_instance_lbl));
__ Bind(&fall_through);
return type_test_cache.raw();
}
if (type.IsType()) {
// Smi is FutureOr<T>, when T is a top type or int or num.
if (!FLAG_strong || !Class::Handle(type.type_class()).IsFutureOrClass()) {
__ BranchIfSmi(kInstanceReg, is_not_instance_lbl);
}
__ BranchIfSmi(kInstanceReg, is_not_instance_lbl);
__ ldm(IA, SP,
(1 << kFunctionTypeArgumentsReg) |
(1 << kInstantiatorTypeArgumentsReg));
Expand Down
34 changes: 12 additions & 22 deletions runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,11 @@ FlowGraphCompiler::GenerateInstantiatedTypeWithArgumentsTest(
ASSERT(type_class.NumTypeArguments() > 0);
const Register kInstanceReg = R0;
Error& bound_error = Error::Handle(zone());
const Type& smi_type = Type::Handle(zone(), Type::SmiType());
const Type& int_type = Type::Handle(zone(), Type::IntType());
const bool smi_is_ok =
smi_type.IsSubtypeOf(type, &bound_error, NULL, Heap::kOld);
int_type.IsSubtypeOf(type, &bound_error, NULL, Heap::kOld);
// Malformed type should have been handled at graph construction time.
ASSERT(smi_is_ok || bound_error.IsNull());
// Fast case for type = FutureOr<int/num/top-type>.
__ BranchIfSmi(kInstanceReg,
smi_is_ok ? is_instance_lbl : is_not_instance_lbl);

Expand Down Expand Up @@ -279,7 +278,7 @@ FlowGraphCompiler::GenerateInstantiatedTypeWithArgumentsTest(
ASSERT(!tp_argument.IsMalformed());
if (tp_argument.IsType()) {
ASSERT(tp_argument.HasResolvedTypeClass());
// Check if type argument is dynamic, Object, or void.
// Check if type argument is dynamic or Object.
const Type& object_type = Type::Handle(zone(), Type::ObjectType());
if (object_type.IsSubtypeOf(tp_argument, NULL, NULL, Heap::kOld)) {
// Instance class test only necessary.
Expand Down Expand Up @@ -332,7 +331,6 @@ bool FlowGraphCompiler::GenerateInstantiatedTypeNoArgumentsTest(
if (smi_class.IsSubtypeOf(Object::null_type_arguments(), type_class,
Object::null_type_arguments(), NULL, NULL,
Heap::kOld)) {
// Fast case for type = int/num/top-type.
__ BranchIfSmi(kInstanceReg, is_instance_lbl);
} else {
__ BranchIfSmi(kInstanceReg, is_not_instance_lbl);
Expand Down Expand Up @@ -390,14 +388,7 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateSubtype1TestCacheLookup(
Label* is_not_instance_lbl) {
__ Comment("Subtype1TestCacheLookup");
const Register kInstanceReg = R0;
#if defined(DEBUG)
Label ok;
__ BranchIfNotSmi(kInstanceReg, &ok);
__ Breakpoint();
__ Bind(&ok);
#endif
__ LoadClassId(TMP, kInstanceReg);
__ LoadClassById(R1, TMP);
__ LoadClass(R1, kInstanceReg);
// R1: instance class.
// Check immediate superclass equality.
__ LoadFieldFromOffset(R2, R1, Class::super_type_offset());
Expand Down Expand Up @@ -444,13 +435,12 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateUninstantiatedTypeTest(
__ LoadFieldFromOffset(R3, kTypeArgumentsReg,
TypeArguments::type_at_offset(type_param.index()));
// R3: concrete type of type.
// Check if type argument is dynamic, Object, or void.
// Check if type argument is dynamic.
__ CompareObject(R3, Object::dynamic_type());
__ b(is_instance_lbl, EQ);
__ CompareObject(R3, Type::ZoneHandle(zone(), Type::ObjectType()));
__ b(is_instance_lbl, EQ);
__ CompareObject(R3, Object::void_type());
__ b(is_instance_lbl, EQ);
// TODO(regis): Optimize void type as well once allowed as type argument.

// For Smi check quickly against int and num interfaces.
Label not_smi;
Expand All @@ -459,8 +449,9 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateUninstantiatedTypeTest(
__ b(is_instance_lbl, EQ);
__ CompareObject(R3, Type::ZoneHandle(zone(), Type::Number()));
__ b(is_instance_lbl, EQ);
// Smi can be handled by type test cache.
__ Bind(&not_smi);
// Smi must be handled in runtime.
Label fall_through;
__ b(&fall_through);

// If it's guaranteed, by type-parameter bound, that the type parameter will
// never have a value of a function type, then we can safely do a 4-type
Expand All @@ -470,18 +461,17 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateUninstantiatedTypeTest(
? kTestTypeSixArgs
: kTestTypeFourArgs;

__ Bind(&not_smi);
const SubtypeTestCache& type_test_cache = SubtypeTestCache::ZoneHandle(
zone(), GenerateCallSubtypeTestStub(
test_kind, kInstanceReg, kInstantiatorTypeArgumentsReg,
kFunctionTypeArgumentsReg, kTempReg, is_instance_lbl,
is_not_instance_lbl));
__ Bind(&fall_through);
return type_test_cache.raw();
}
if (type.IsType()) {
// Smi is FutureOr<T>, when T is a top type or int or num.
if (!FLAG_strong || !Class::Handle(type.type_class()).IsFutureOrClass()) {
__ BranchIfSmi(kInstanceReg, is_not_instance_lbl);
}
__ BranchIfSmi(kInstanceReg, is_not_instance_lbl);
__ ldp(kFunctionTypeArgumentsReg, kInstantiatorTypeArgumentsReg,
Address(SP, 0 * kWordSize, Address::PairOffset));
// Uninstantiated type class is known at compile time, but the type
Expand Down
36 changes: 13 additions & 23 deletions runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -256,14 +256,13 @@ FlowGraphCompiler::GenerateInstantiatedTypeWithArgumentsTest(
ASSERT(type_class.NumTypeArguments() > 0);
const Register kInstanceReg = EAX;
Error& bound_error = Error::Handle(zone());
const Type& smi_type = Type::Handle(zone(), Type::SmiType());
const Type& int_type = Type::Handle(zone(), Type::IntType());
const bool smi_is_ok =
smi_type.IsSubtypeOf(type, &bound_error, NULL, Heap::kOld);
int_type.IsSubtypeOf(type, &bound_error, NULL, Heap::kOld);
// Malformed type should have been handled at graph construction time.
ASSERT(smi_is_ok || bound_error.IsNull());
__ testl(kInstanceReg, Immediate(kSmiTagMask));
if (smi_is_ok) {
// Fast case for type = FutureOr<int/num/top-type>.
__ j(ZERO, is_instance_lbl);
} else {
__ j(ZERO, is_not_instance_lbl);
Expand Down Expand Up @@ -295,7 +294,7 @@ FlowGraphCompiler::GenerateInstantiatedTypeWithArgumentsTest(
ASSERT(!tp_argument.IsMalformed());
if (tp_argument.IsType()) {
ASSERT(tp_argument.HasResolvedTypeClass());
// Check if type argument is dynamic, Object, or void.
// Check if type argument is dynamic or Object.
const Type& object_type = Type::Handle(zone(), Type::ObjectType());
if (object_type.IsSubtypeOf(tp_argument, NULL, NULL, Heap::kOld)) {
// Instance class test only necessary.
Expand Down Expand Up @@ -348,7 +347,6 @@ bool FlowGraphCompiler::GenerateInstantiatedTypeNoArgumentsTest(
if (smi_class.IsSubtypeOf(Object::null_type_arguments(), type_class,
Object::null_type_arguments(), NULL, NULL,
Heap::kOld)) {
// Fast case for type = int/num/top-type.
__ j(ZERO, is_instance_lbl);
} else {
__ j(ZERO, is_not_instance_lbl);
Expand Down Expand Up @@ -406,14 +404,7 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateSubtype1TestCacheLookup(
Label* is_not_instance_lbl) {
__ Comment("Subtype1TestCacheLookup");
const Register kInstanceReg = EAX;
#if defined(DEBUG)
Label ok;
__ BranchIfNotSmi(kInstanceReg, &ok);
__ Breakpoint();
__ Bind(&ok);
#endif
__ LoadClassId(EDI, kInstanceReg);
__ LoadClassById(ECX, EDI);
__ LoadClass(ECX, kInstanceReg, EDI);
// ECX: instance class.
// Check immediate superclass equality.
__ movl(EDI, FieldAddress(ECX, Class::super_type_offset()));
Expand Down Expand Up @@ -464,13 +455,12 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateUninstantiatedTypeTest(
__ movl(EDI, FieldAddress(kTypeArgumentsReg, TypeArguments::type_at_offset(
type_param.index())));
// EDI: concrete type of type.
// Check if type argument is dynamic, Object, or void.
// Check if type argument is dynamic.
__ CompareObject(EDI, Object::dynamic_type());
__ j(EQUAL, is_instance_lbl);
__ CompareObject(EDI, Type::ZoneHandle(zone(), Type::ObjectType()));
__ j(EQUAL, is_instance_lbl);
__ CompareObject(EDI, Object::void_type());
__ j(EQUAL, is_instance_lbl);
// TODO(regis): Optimize void type as well once allowed as type argument.

// For Smi check quickly against int and num interfaces.
Label not_smi;
Expand All @@ -480,8 +470,9 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateUninstantiatedTypeTest(
__ j(EQUAL, is_instance_lbl);
__ CompareObject(EDI, Type::ZoneHandle(zone(), Type::Number()));
__ j(EQUAL, is_instance_lbl);
// Smi can be handled by type test cache.
__ Bind(&not_smi);
// Smi must be handled in runtime.
Label fall_through;
__ jmp(&fall_through);

// If it's guaranteed, by type-parameter bound, that the type parameter will
// never have a value of a function type.
Expand All @@ -490,19 +481,18 @@ RawSubtypeTestCache* FlowGraphCompiler::GenerateUninstantiatedTypeTest(
? kTestTypeSixArgs
: kTestTypeFourArgs;

__ Bind(&not_smi);
const SubtypeTestCache& type_test_cache = SubtypeTestCache::ZoneHandle(
zone(), GenerateCallSubtypeTestStub(
test_kind, kInstanceReg, kInstantiatorTypeArgumentsReg,
kFunctionTypeArgumentsReg, kTempReg, is_instance_lbl,
is_not_instance_lbl));
__ Bind(&fall_through);
return type_test_cache.raw();
}
if (type.IsType()) {
// Smi is FutureOr<T>, when T is a top type or int or num.
if (!FLAG_strong || !Class::Handle(type.type_class()).IsFutureOrClass()) {
__ testl(kInstanceReg, Immediate(kSmiTagMask)); // Is instance Smi?
__ j(ZERO, is_not_instance_lbl);
}
__ testl(kInstanceReg, Immediate(kSmiTagMask)); // Is instance Smi?
__ j(ZERO, is_not_instance_lbl);
__ movl(kInstantiatorTypeArgumentsReg, Address(ESP, 1 * kWordSize));
__ movl(kFunctionTypeArgumentsReg, Address(ESP, 0 * kWordSize));
// Uninstantiated type class is known at compile time, but the type
Expand Down
Loading

0 comments on commit 6134ac8

Please sign in to comment.