Skip to content

Commit 1c7731d

Browse files
mkustermanncommit-bot@chromium.org
mkustermann
authored and
[email protected]
committed
[vm/compiler] Always use type testing stubs for AssertAssignable
As a preparation for being able to dynamically call TTS on any type object we ensure that also in the non-dynamic AssertAssignables we always use TTS. Issue #40813 Change-Id: Ia7a2aa98eff940e4ead28f6158f13c084ecb3818 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/162008 Commit-Queue: Martin Kustermann <[email protected]> Reviewed-by: Aske Simon Christensen <[email protected]> Reviewed-by: Tess Strickland <[email protected]>
1 parent b8d14d4 commit 1c7731d

8 files changed

+72
-244
lines changed

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2290,13 +2290,6 @@ bool FlowGraphCompiler::CheckAssertAssignableTypeTestingABILocations(
22902290
return true;
22912291
}
22922292

2293-
bool FlowGraphCompiler::ShouldUseTypeTestingStubFor(bool optimizing,
2294-
const AbstractType& type) {
2295-
return FLAG_precompiled_mode ||
2296-
(optimizing &&
2297-
(type.IsTypeParameter() || (type.IsType() && type.IsInstantiated())));
2298-
}
2299-
23002293
FlowGraphCompiler::TypeTestStubKind
23012294
FlowGraphCompiler::GetTypeTestStubKindForTypeParameter(
23022295
const TypeParameter& type_param) {

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -568,11 +568,6 @@ class FlowGraphCompiler : public ValueObject {
568568
const String& dst_name,
569569
LocationSummary* locs);
570570

571-
// Returns true if we can use a type testing stub based assert
572-
// assignable code pattern for the given type.
573-
static bool ShouldUseTypeTestingStubFor(bool optimizing,
574-
const AbstractType& type);
575-
576571
void GenerateAssertAssignableViaTypeTestingStub(CompileType* receiver_type,
577572
TokenPosition token_pos,
578573
intptr_t deopt_id,

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

Lines changed: 3 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -703,67 +703,19 @@ void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type,
703703
ASSERT(!token_pos.IsClassifying());
704704
ASSERT(CheckAssertAssignableTypeTestingABILocations(*locs));
705705

706-
compiler::Label is_assignable_fast, is_assignable, runtime_call;
707-
708-
// Generate inline type check, linking to runtime call if not assignable.
709-
SubtypeTestCache& test_cache = SubtypeTestCache::ZoneHandle(zone());
710-
static_assert(
711-
TypeTestABI::kFunctionTypeArgumentsReg <
712-
TypeTestABI::kInstantiatorTypeArgumentsReg,
713-
"Should be ordered to push and load arguments with one instruction");
714-
static RegList type_args = (1 << TypeTestABI::kFunctionTypeArgumentsReg) |
715-
(1 << TypeTestABI::kInstantiatorTypeArgumentsReg);
716-
717706
if (locs->in(1).IsConstant()) {
718707
const auto& dst_type = AbstractType::Cast(locs->in(1).constant());
719708
ASSERT(dst_type.IsFinalized());
720709

721710
if (dst_type.IsTopTypeForSubtyping()) return; // No code needed.
722711

723-
if (ShouldUseTypeTestingStubFor(is_optimizing(), dst_type)) {
724-
GenerateAssertAssignableViaTypeTestingStub(receiver_type, token_pos,
725-
deopt_id, dst_name, locs);
726-
return;
727-
}
728-
729-
if (Instance::NullIsAssignableTo(dst_type)) {
730-
__ CompareObject(TypeTestABI::kInstanceReg, Object::null_object());
731-
__ b(&is_assignable_fast, EQ);
732-
}
733-
734-
__ PushList(type_args);
735-
736-
test_cache = GenerateInlineInstanceof(token_pos, dst_type, &is_assignable,
737-
&runtime_call);
738-
} else {
739-
// TODO(dartbug.com/40813): Handle setting up the non-constant case.
740-
UNREACHABLE();
741-
}
742-
743-
__ Bind(&runtime_call);
744-
__ ldm(IA, SP, type_args);
745-
__ PushObject(Object::null_object()); // Make room for the result.
746-
__ Push(TypeTestABI::kInstanceReg); // Push the source object.
747-
// Push the type of the destination.
748-
if (locs->in(1).IsConstant()) {
749-
__ PushObject(locs->in(1).constant());
712+
GenerateAssertAssignableViaTypeTestingStub(receiver_type, token_pos,
713+
deopt_id, dst_name, locs);
714+
return;
750715
} else {
751716
// TODO(dartbug.com/40813): Handle setting up the non-constant case.
752717
UNREACHABLE();
753718
}
754-
__ PushList(type_args);
755-
__ PushObject(dst_name); // Push the name of the destination.
756-
__ LoadUniqueObject(R0, test_cache);
757-
__ Push(R0);
758-
__ PushImmediate(Smi::RawValue(kTypeCheckFromInline));
759-
GenerateRuntimeCall(token_pos, deopt_id, kTypeCheckRuntimeEntry, 7, locs);
760-
// Pop the parameters supplied to the runtime entry. The result of the
761-
// type check runtime call is the checked value.
762-
__ Drop(7);
763-
__ Pop(TypeTestABI::kInstanceReg);
764-
__ Bind(&is_assignable);
765-
__ PopList(type_args);
766-
__ Bind(&is_assignable_fast);
767719
}
768720

769721
void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(

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

Lines changed: 3 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -669,67 +669,19 @@ void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type,
669669
ASSERT(!TokenPosition(token_pos).IsClassifying());
670670
ASSERT(CheckAssertAssignableTypeTestingABILocations(*locs));
671671

672-
compiler::Label is_assignable_fast, is_assignable, runtime_call;
673-
// Generate inline type check, linking to runtime call if not assignable.
674-
SubtypeTestCache& test_cache = SubtypeTestCache::ZoneHandle(zone());
675-
676672
if (locs->in(1).IsConstant()) {
677673
const auto& dst_type = AbstractType::Cast(locs->in(1).constant());
678674
ASSERT(dst_type.IsFinalized());
679675

680676
if (dst_type.IsTopTypeForSubtyping()) return; // No code needed.
681677

682-
if (ShouldUseTypeTestingStubFor(is_optimizing(), dst_type)) {
683-
GenerateAssertAssignableViaTypeTestingStub(receiver_type, token_pos,
684-
deopt_id, dst_name, locs);
685-
return;
686-
}
687-
688-
if (Instance::NullIsAssignableTo(dst_type)) {
689-
__ CompareObject(TypeTestABI::kInstanceReg, Object::null_object());
690-
__ b(&is_assignable_fast, EQ);
691-
}
692-
693-
__ PushPair(TypeTestABI::kFunctionTypeArgumentsReg,
694-
TypeTestABI::kInstantiatorTypeArgumentsReg);
695-
696-
test_cache = GenerateInlineInstanceof(token_pos, dst_type, &is_assignable,
697-
&runtime_call);
698-
} else {
699-
// TODO(dartbug.com/40813): Handle setting up the non-constant case.
700-
UNREACHABLE();
701-
}
702-
703-
__ Bind(&runtime_call);
704-
__ ldp(TypeTestABI::kFunctionTypeArgumentsReg,
705-
TypeTestABI::kInstantiatorTypeArgumentsReg,
706-
compiler::Address(SP, 0 * kWordSize, compiler::Address::PairOffset));
707-
// Make room for the result and push the source object.
708-
__ PushPair(TypeTestABI::kInstanceReg, NULL_REG);
709-
// Push the destination type and the instantiator type arguments.
710-
if (locs->in(1).IsConstant()) {
711-
__ LoadObject(TMP, locs->in(1).constant());
712-
__ PushPair(TypeTestABI::kInstantiatorTypeArgumentsReg, TMP);
678+
GenerateAssertAssignableViaTypeTestingStub(receiver_type, token_pos,
679+
deopt_id, dst_name, locs);
680+
return;
713681
} else {
714682
// TODO(dartbug.com/40813): Handle setting up the non-constant case.
715683
UNREACHABLE();
716684
}
717-
// Push the function type arguments and the name of the destination.
718-
__ LoadObject(TMP, dst_name);
719-
__ PushPair(TMP, TypeTestABI::kFunctionTypeArgumentsReg);
720-
721-
__ LoadUniqueObject(R0, test_cache);
722-
__ LoadObject(TMP, Smi::ZoneHandle(zone(), Smi::New(kTypeCheckFromInline)));
723-
__ PushPair(TMP, R0);
724-
GenerateRuntimeCall(token_pos, deopt_id, kTypeCheckRuntimeEntry, 7, locs);
725-
// Pop the parameters supplied to the runtime entry. The result of the
726-
// type check runtime call is the checked value.
727-
__ Drop(7);
728-
__ Pop(TypeTestABI::kInstanceReg);
729-
__ Bind(&is_assignable);
730-
__ PopPair(TypeTestABI::kFunctionTypeArgumentsReg,
731-
TypeTestABI::kInstantiatorTypeArgumentsReg);
732-
__ Bind(&is_assignable_fast);
733685
}
734686

735687
void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(

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

Lines changed: 3 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -684,59 +684,19 @@ void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type,
684684
ASSERT(!token_pos.IsClassifying());
685685
ASSERT(CheckAssertAssignableTypeTestingABILocations(*locs));
686686

687-
compiler::Label is_assignable, runtime_call;
688-
689-
// Generate inline type check, linking to runtime call if not assignable.
690-
SubtypeTestCache& test_cache = SubtypeTestCache::ZoneHandle(zone());
691-
692687
if (locs->in(1).IsConstant()) {
693688
const auto& dst_type = AbstractType::Cast(locs->in(1).constant());
694689
ASSERT(dst_type.IsFinalized());
695690

696691
if (dst_type.IsTopTypeForSubtyping()) return; // No code needed.
697692

698-
if (ShouldUseTypeTestingStubFor(is_optimizing(), dst_type)) {
699-
GenerateAssertAssignableViaTypeTestingStub(receiver_type, token_pos,
700-
deopt_id, dst_name, locs);
701-
return;
702-
}
703-
704-
if (Instance::NullIsAssignableTo(dst_type)) {
705-
__ CompareObject(TypeTestABI::kInstanceReg, Object::null_object());
706-
__ j(EQUAL, &is_assignable);
707-
}
708-
709-
// The registers RAX, RCX, RDX are preserved across the call.
710-
test_cache = GenerateInlineInstanceof(token_pos, dst_type, &is_assignable,
711-
&runtime_call);
712-
713-
} else {
714-
// TODO(dartbug.com/40813): Handle setting up the non-constant case.
715-
UNREACHABLE();
716-
}
717-
718-
__ Bind(&runtime_call);
719-
__ PushObject(Object::null_object()); // Make room for the result.
720-
__ pushq(TypeTestABI::kInstanceReg); // Push the source object.
721-
// Push the destination type.
722-
if (locs->in(1).IsConstant()) {
723-
__ PushObject(locs->in(1).constant());
693+
GenerateAssertAssignableViaTypeTestingStub(receiver_type, token_pos,
694+
deopt_id, dst_name, locs);
695+
return;
724696
} else {
725697
// TODO(dartbug.com/40813): Handle setting up the non-constant case.
726698
UNREACHABLE();
727699
}
728-
__ pushq(TypeTestABI::kInstantiatorTypeArgumentsReg);
729-
__ pushq(TypeTestABI::kFunctionTypeArgumentsReg);
730-
__ PushObject(dst_name); // Push the name of the destination.
731-
__ LoadUniqueObject(RAX, test_cache);
732-
__ pushq(RAX);
733-
__ PushImmediate(compiler::Immediate(Smi::RawValue(kTypeCheckFromInline)));
734-
GenerateRuntimeCall(token_pos, deopt_id, kTypeCheckRuntimeEntry, 7, locs);
735-
// Pop the parameters supplied to the runtime entry. The result of the
736-
// type check runtime call is the checked value.
737-
__ Drop(7);
738-
__ popq(TypeTestABI::kInstanceReg);
739-
__ Bind(&is_assignable);
740700
}
741701

742702
void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(

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

Lines changed: 21 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -759,15 +759,12 @@ LocationSummary* AssertAssignableInstr::MakeLocationSummary(Zone* zone,
759759
auto const dst_type_loc =
760760
LocationFixedRegisterOrConstant(dst_type(), TypeTestABI::kDstTypeReg);
761761

762-
// When using a type testing stub, we want to prevent spilling of the
763-
// function/instantiator type argument vectors, since stub preserves them. So
764-
// we make this a `kNoCall` summary, even though most other registers can be
765-
// modified by the stub. To tell the register allocator about it, we reserve
766-
// all the other registers as temporary registers.
762+
// We want to prevent spilling of the inputs (e.g. function/instantiator tav),
763+
// since TTS preserves them. So we make this a `kNoCall` summary,
764+
// even though most other registers can be modified by the stub. To tell the
765+
// register allocator about it, we reserve all the other registers as
766+
// temporary registers.
767767
// TODO(http://dartbug.com/32788): Simplify this.
768-
const bool using_stub = dst_type_loc.IsConstant() &&
769-
FlowGraphCompiler::ShouldUseTypeTestingStubFor(
770-
opt, AbstractType::Cast(dst_type_loc.constant()));
771768

772769
const intptr_t kNonChangeableInputRegs =
773770
(1 << TypeTestABI::kInstanceReg) |
@@ -789,14 +786,11 @@ LocationSummary* AssertAssignableInstr::MakeLocationSummary(Zone* zone,
789786
<< kAbiFirstPreservedFpuReg) &
790787
~(1 << FpuTMP);
791788

792-
const intptr_t kNumTemps =
793-
using_stub ? (Utils::CountOneBits64(kCpuRegistersToPreserve) +
794-
Utils::CountOneBits64(kFpuRegistersToPreserve))
795-
: 0;
789+
const intptr_t kNumTemps = (Utils::CountOneBits64(kCpuRegistersToPreserve) +
790+
Utils::CountOneBits64(kFpuRegistersToPreserve));
796791

797792
LocationSummary* summary = new (zone) LocationSummary(
798-
zone, kNumInputs, kNumTemps,
799-
using_stub ? LocationSummary::kCallCalleeSafe : LocationSummary::kCall);
793+
zone, kNumInputs, kNumTemps, LocationSummary::kCallCalleeSafe);
800794
summary->set_in(0, Location::RegisterLocation(TypeTestABI::kInstanceReg));
801795
summary->set_in(1, dst_type_loc);
802796
summary->set_in(2, Location::RegisterLocation(
@@ -805,23 +799,21 @@ LocationSummary* AssertAssignableInstr::MakeLocationSummary(Zone* zone,
805799
3, Location::RegisterLocation(TypeTestABI::kFunctionTypeArgumentsReg));
806800
summary->set_out(0, Location::SameAsFirstInput());
807801

808-
if (using_stub) {
809-
// Let's reserve all registers except for the input ones.
810-
intptr_t next_temp = 0;
811-
for (intptr_t i = 0; i < kNumberOfCpuRegisters; ++i) {
812-
const bool should_preserve = ((1 << i) & kCpuRegistersToPreserve) != 0;
813-
if (should_preserve) {
814-
summary->set_temp(next_temp++,
815-
Location::RegisterLocation(static_cast<Register>(i)));
816-
}
802+
// Let's reserve all registers except for the input ones.
803+
intptr_t next_temp = 0;
804+
for (intptr_t i = 0; i < kNumberOfCpuRegisters; ++i) {
805+
const bool should_preserve = ((1 << i) & kCpuRegistersToPreserve) != 0;
806+
if (should_preserve) {
807+
summary->set_temp(next_temp++,
808+
Location::RegisterLocation(static_cast<Register>(i)));
817809
}
810+
}
818811

819-
for (intptr_t i = 0; i < kNumberOfFpuRegisters; i++) {
820-
const bool should_preserve = ((1 << i) & kFpuRegistersToPreserve) != 0;
821-
if (should_preserve) {
822-
summary->set_temp(next_temp++, Location::FpuRegisterLocation(
823-
static_cast<FpuRegister>(i)));
824-
}
812+
for (intptr_t i = 0; i < kNumberOfFpuRegisters; i++) {
813+
const bool should_preserve = ((1 << i) & kFpuRegistersToPreserve) != 0;
814+
if (should_preserve) {
815+
summary->set_temp(next_temp++, Location::FpuRegisterLocation(
816+
static_cast<FpuRegister>(i)));
825817
}
826818
}
827819

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

Lines changed: 21 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -658,15 +658,12 @@ LocationSummary* AssertAssignableInstr::MakeLocationSummary(Zone* zone,
658658
auto const dst_type_loc =
659659
LocationFixedRegisterOrConstant(dst_type(), TypeTestABI::kDstTypeReg);
660660

661-
// When using a type testing stub, we want to prevent spilling of the
662-
// function/instantiator type argument vectors, since stub preserves them. So
663-
// we make this a `kNoCall` summary, even though most other registers can be
664-
// modified by the stub. To tell the register allocator about it, we reserve
665-
// all the other registers as temporary registers.
661+
// We want to prevent spilling of the inputs (e.g. function/instantiator tav),
662+
// since TTS preserves them. So we make this a `kNoCall` summary,
663+
// even though most other registers can be modified by the stub. To tell the
664+
// register allocator about it, we reserve all the other registers as
665+
// temporary registers.
666666
// TODO(http://dartbug.com/32788): Simplify this.
667-
const bool using_stub = dst_type_loc.IsConstant() &&
668-
FlowGraphCompiler::ShouldUseTypeTestingStubFor(
669-
opt, AbstractType::Cast(dst_type_loc.constant()));
670667

671668
const intptr_t kNonChangeableInputRegs =
672669
(1 << TypeTestABI::kInstanceReg) |
@@ -686,14 +683,11 @@ LocationSummary* AssertAssignableInstr::MakeLocationSummary(Zone* zone,
686683
const intptr_t kFpuRegistersToPreserve =
687684
Utils::SignedNBitMask(kNumberOfFpuRegisters) & ~(1l << FpuTMP);
688685

689-
const intptr_t kNumTemps =
690-
using_stub ? (Utils::CountOneBits64(kCpuRegistersToPreserve) +
691-
Utils::CountOneBits64(kFpuRegistersToPreserve))
692-
: 0;
686+
const intptr_t kNumTemps = (Utils::CountOneBits64(kCpuRegistersToPreserve) +
687+
Utils::CountOneBits64(kFpuRegistersToPreserve));
693688

694689
LocationSummary* summary = new (zone) LocationSummary(
695-
zone, kNumInputs, kNumTemps,
696-
using_stub ? LocationSummary::kCallCalleeSafe : LocationSummary::kCall);
690+
zone, kNumInputs, kNumTemps, LocationSummary::kCallCalleeSafe);
697691
summary->set_in(0, Location::RegisterLocation(TypeTestABI::kInstanceReg));
698692
summary->set_in(1, dst_type_loc);
699693
summary->set_in(2, Location::RegisterLocation(
@@ -702,23 +696,21 @@ LocationSummary* AssertAssignableInstr::MakeLocationSummary(Zone* zone,
702696
3, Location::RegisterLocation(TypeTestABI::kFunctionTypeArgumentsReg));
703697
summary->set_out(0, Location::SameAsFirstInput());
704698

705-
if (using_stub) {
706-
// Let's reserve all registers except for the input ones.
707-
intptr_t next_temp = 0;
708-
for (intptr_t i = 0; i < kNumberOfCpuRegisters; ++i) {
709-
const bool should_preserve = ((1 << i) & kCpuRegistersToPreserve) != 0;
710-
if (should_preserve) {
711-
summary->set_temp(next_temp++,
712-
Location::RegisterLocation(static_cast<Register>(i)));
713-
}
699+
// Let's reserve all registers except for the input ones.
700+
intptr_t next_temp = 0;
701+
for (intptr_t i = 0; i < kNumberOfCpuRegisters; ++i) {
702+
const bool should_preserve = ((1 << i) & kCpuRegistersToPreserve) != 0;
703+
if (should_preserve) {
704+
summary->set_temp(next_temp++,
705+
Location::RegisterLocation(static_cast<Register>(i)));
714706
}
707+
}
715708

716-
for (intptr_t i = 0; i < kNumberOfFpuRegisters; i++) {
717-
const bool should_preserve = ((1l << i) & kFpuRegistersToPreserve) != 0;
718-
if (should_preserve) {
719-
summary->set_temp(next_temp++, Location::FpuRegisterLocation(
720-
static_cast<FpuRegister>(i)));
721-
}
709+
for (intptr_t i = 0; i < kNumberOfFpuRegisters; i++) {
710+
const bool should_preserve = ((1l << i) & kFpuRegistersToPreserve) != 0;
711+
if (should_preserve) {
712+
summary->set_temp(next_temp++, Location::FpuRegisterLocation(
713+
static_cast<FpuRegister>(i)));
722714
}
723715
}
724716

0 commit comments

Comments
 (0)