Skip to content

Commit db51ed7

Browse files
sstricklcommit-bot@chromium.org
sstrickl
authored and
[email protected]
committed
[vm] Relax AssertAssignable to allow dst_type to be non-constant.
Previously, AssertAssignable instructions could only be created with a compile-time AbstractType value for dst_type. Relax this so that AssertAssignable instructions can be created with a possibly non-constant Value instead. This is only an IL-level change for the instruction. Code generation for AssertAssignable still requires the dst_type Value to be bound to a constant and is otherwise unchanged from before, thus this should have no impact on size or speed of generated code. (Generating the non-constant case will be done in follow-up work.) Bug: #40813 Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-product-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm_x64-try,vm-kernel-precomp-linux-release-simarm64-try,vm-dartkb-linux-release-simarm64-try,vm-dartkb-linux-release-x64-try Bug: #40813 Change-Id: I1f984039d8c2695c66161a69a3d80cfbe7110f19 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/146801 Commit-Queue: Tess Strickland <[email protected]> Reviewed-by: Martin Kustermann <[email protected]> Reviewed-by: Vyacheslav Egorov <[email protected]>
1 parent 84ca34b commit db51ed7

24 files changed

+313
-212
lines changed

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -431,13 +431,15 @@ void ConstantPropagator::VisitPushArgument(PushArgumentInstr* instr) {
431431
}
432432

433433
void ConstantPropagator::VisitAssertAssignable(AssertAssignableInstr* instr) {
434-
const Object& value = instr->value()->definition()->constant_value();
435-
if (IsNonConstant(value)) {
434+
const auto& value = instr->value()->definition()->constant_value();
435+
const auto& dst_type = instr->dst_type()->definition()->constant_value();
436+
437+
if (IsNonConstant(value) || IsNonConstant(dst_type)) {
436438
SetValue(instr, non_constant_);
437-
} else if (IsConstant(value)) {
439+
} else if (IsConstant(value) && IsConstant(dst_type)) {
438440
// We are ignoring the instantiator and instantiator_type_arguments, but
439441
// still monotonic and safe.
440-
if (instr->value()->Type()->IsAssignableTo(instr->dst_type())) {
442+
if (instr->value()->Type()->IsAssignableTo(AbstractType::Cast(dst_type))) {
441443
SetValue(instr, value);
442444
} else {
443445
SetValue(instr, non_constant_);

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2308,6 +2308,22 @@ void FlowGraphCompiler::GenerateCidRangesCheck(
23082308
}
23092309
}
23102310

2311+
bool FlowGraphCompiler::CheckAssertAssignableTypeTestingABILocations(
2312+
const LocationSummary& locs) {
2313+
ASSERT(locs.in(0).IsRegister() &&
2314+
locs.in(0).reg() == TypeTestABI::kInstanceReg);
2315+
ASSERT((locs.in(1).IsConstant() && locs.in(1).constant().IsAbstractType()) ||
2316+
(locs.in(1).IsRegister() &&
2317+
locs.in(1).reg() == TypeTestABI::kDstTypeReg));
2318+
ASSERT(locs.in(2).IsRegister() &&
2319+
locs.in(2).reg() == TypeTestABI::kInstantiatorTypeArgumentsReg);
2320+
ASSERT(locs.in(3).IsRegister() &&
2321+
locs.in(3).reg() == TypeTestABI::kFunctionTypeArgumentsReg);
2322+
ASSERT(locs.out(0).IsRegister() &&
2323+
locs.out(0).reg() == TypeTestABI::kInstanceReg);
2324+
return true;
2325+
}
2326+
23112327
bool FlowGraphCompiler::ShouldUseTypeTestingStubFor(bool optimizing,
23122328
const AbstractType& type) {
23132329
return FLAG_precompiled_mode ||

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -568,10 +568,12 @@ class FlowGraphCompiler : public ValueObject {
568568
Representation src_type,
569569
TemporaryRegisterAllocator* temp);
570570

571+
bool CheckAssertAssignableTypeTestingABILocations(
572+
const LocationSummary& locs);
573+
571574
void GenerateAssertAssignable(CompileType* receiver_type,
572575
TokenPosition token_pos,
573576
intptr_t deopt_id,
574-
const AbstractType& dst_type,
575577
const String& dst_name,
576578
LocationSummary* locs);
577579

@@ -583,7 +585,6 @@ class FlowGraphCompiler : public ValueObject {
583585
void GenerateAssertAssignableViaTypeTestingStub(CompileType* receiver_type,
584586
TokenPosition token_pos,
585587
intptr_t deopt_id,
586-
const AbstractType& dst_type,
587588
const String& dst_name,
588589
LocationSummary* locs);
589590

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

Lines changed: 58 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -688,6 +688,7 @@ void FlowGraphCompiler::GenerateInstanceOf(TokenPosition token_pos,
688688
// - Class equality (only if class is not parameterized).
689689
// Inputs:
690690
// - R0: instance being type checked.
691+
// - R8: destination type (if non-constant).
691692
// - R2: instantiator type arguments or raw_null.
692693
// - R1: function type arguments or raw_null.
693694
// Returns:
@@ -697,72 +698,85 @@ void FlowGraphCompiler::GenerateInstanceOf(TokenPosition token_pos,
697698
void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type,
698699
TokenPosition token_pos,
699700
intptr_t deopt_id,
700-
const AbstractType& dst_type,
701701
const String& dst_name,
702702
LocationSummary* locs) {
703703
ASSERT(!token_pos.IsClassifying());
704-
ASSERT(!dst_type.IsNull());
705-
ASSERT(dst_type.IsFinalized());
706-
// Assignable check is skipped in FlowGraphBuilder, not here.
707-
ASSERT(!dst_type.IsTopTypeForSubtyping());
708-
709-
if (ShouldUseTypeTestingStubFor(is_optimizing(), dst_type)) {
710-
GenerateAssertAssignableViaTypeTestingStub(
711-
receiver_type, token_pos, deopt_id, dst_type, dst_name, locs);
712-
} else {
713-
compiler::Label is_assignable_fast, is_assignable, runtime_call;
704+
ASSERT(CheckAssertAssignableTypeTestingABILocations(*locs));
705+
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+
717+
if (locs->in(1).IsConstant()) {
718+
const auto& dst_type = AbstractType::Cast(locs->in(1).constant());
719+
ASSERT(dst_type.IsFinalized());
720+
721+
if (dst_type.IsTopTypeForSubtyping()) return; // No code needed.
722+
723+
if (ShouldUseTypeTestingStubFor(is_optimizing(), dst_type)) {
724+
GenerateAssertAssignableViaTypeTestingStub(receiver_type, token_pos,
725+
deopt_id, dst_name, locs);
726+
return;
727+
}
714728

715729
if (Instance::NullIsAssignableTo(dst_type)) {
716730
__ CompareObject(TypeTestABI::kInstanceReg, Object::null_object());
717731
__ b(&is_assignable_fast, EQ);
718732
}
719733

720-
static_assert(TypeTestABI::kFunctionTypeArgumentsReg <
721-
TypeTestABI::kInstantiatorTypeArgumentsReg,
722-
"Should be ordered to push arguments with one instruction");
723-
__ PushList((1 << TypeTestABI::kInstantiatorTypeArgumentsReg) |
724-
(1 << TypeTestABI::kFunctionTypeArgumentsReg));
734+
__ PushList(type_args);
725735

726-
// Generate inline type check, linking to runtime call if not assignable.
727-
SubtypeTestCache& test_cache = SubtypeTestCache::ZoneHandle(zone());
728736
test_cache = GenerateInlineInstanceof(token_pos, dst_type, &is_assignable,
729737
&runtime_call);
738+
} else {
739+
// TODO(dartbug.com/40813): Handle setting up the non-constant case.
740+
UNREACHABLE();
741+
}
730742

731-
__ Bind(&runtime_call);
732-
static_assert(TypeTestABI::kFunctionTypeArgumentsReg <
733-
TypeTestABI::kInstantiatorTypeArgumentsReg,
734-
"Should be ordered to load arguments with one instruction");
735-
__ ldm(IA, SP,
736-
(1 << TypeTestABI::kFunctionTypeArgumentsReg) |
737-
(1 << TypeTestABI::kInstantiatorTypeArgumentsReg));
738-
__ PushObject(Object::null_object()); // Make room for the result.
739-
__ Push(TypeTestABI::kInstanceReg); // Push the source object.
740-
__ PushObject(dst_type); // Push the type of the destination.
741-
__ PushList((1 << TypeTestABI::kInstantiatorTypeArgumentsReg) |
742-
(1 << TypeTestABI::kFunctionTypeArgumentsReg));
743-
__ PushObject(dst_name); // Push the name of the destination.
744-
__ LoadUniqueObject(R0, test_cache);
745-
__ Push(R0);
746-
__ PushImmediate(Smi::RawValue(kTypeCheckFromInline));
747-
GenerateRuntimeCall(token_pos, deopt_id, kTypeCheckRuntimeEntry, 7, locs);
748-
// Pop the parameters supplied to the runtime entry. The result of the
749-
// type check runtime call is the checked value.
750-
__ Drop(7);
751-
__ Pop(R0);
752-
__ Bind(&is_assignable);
753-
__ PopList((1 << TypeTestABI::kFunctionTypeArgumentsReg) |
754-
(1 << TypeTestABI::kInstantiatorTypeArgumentsReg));
755-
__ Bind(&is_assignable_fast);
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());
750+
} else {
751+
// TODO(dartbug.com/40813): Handle setting up the non-constant case.
752+
UNREACHABLE();
756753
}
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);
757767
}
758768

759769
void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
760770
CompileType* receiver_type,
761771
TokenPosition token_pos,
762772
intptr_t deopt_id,
763-
const AbstractType& dst_type,
764773
const String& dst_name,
765774
LocationSummary* locs) {
775+
ASSERT(CheckAssertAssignableTypeTestingABILocations(*locs));
776+
// We must have a constant dst_type for generating a call to the stub.
777+
ASSERT(locs->in(1).IsConstant());
778+
const auto& dst_type = AbstractType::Cast(locs->in(1).constant());
779+
766780
// If the dst_type is instantiated we know the target TTS stub at
767781
// compile-time and can therefore use a pc-relative call.
768782
const bool use_pc_relative_call = dst_type.IsInstantiated() &&

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

Lines changed: 55 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,7 @@ void FlowGraphCompiler::GenerateInstanceOf(TokenPosition token_pos,
654654
// - Class equality (only if class is not parameterized).
655655
// Inputs:
656656
// - R0: instance being type checked.
657+
// - R8: destination type (if non-constant).
657658
// - R2: instantiator type arguments or raw_null.
658659
// - R1: function type arguments or raw_null.
659660
// Returns:
@@ -663,20 +664,26 @@ void FlowGraphCompiler::GenerateInstanceOf(TokenPosition token_pos,
663664
void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type,
664665
TokenPosition token_pos,
665666
intptr_t deopt_id,
666-
const AbstractType& dst_type,
667667
const String& dst_name,
668668
LocationSummary* locs) {
669669
ASSERT(!TokenPosition(token_pos).IsClassifying());
670-
ASSERT(!dst_type.IsNull());
671-
ASSERT(dst_type.IsFinalized());
672-
// Assignable check is skipped in FlowGraphBuilder, not here.
673-
ASSERT(!dst_type.IsTopTypeForSubtyping());
674-
675-
if (ShouldUseTypeTestingStubFor(is_optimizing(), dst_type)) {
676-
GenerateAssertAssignableViaTypeTestingStub(
677-
receiver_type, token_pos, deopt_id, dst_type, dst_name, locs);
678-
} else {
679-
compiler::Label is_assignable_fast, is_assignable, runtime_call;
670+
ASSERT(CheckAssertAssignableTypeTestingABILocations(*locs));
671+
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+
676+
if (locs->in(1).IsConstant()) {
677+
const auto& dst_type = AbstractType::Cast(locs->in(1).constant());
678+
ASSERT(dst_type.IsFinalized());
679+
680+
if (dst_type.IsTopTypeForSubtyping()) return; // No code needed.
681+
682+
if (ShouldUseTypeTestingStubFor(is_optimizing(), dst_type)) {
683+
GenerateAssertAssignableViaTypeTestingStub(receiver_type, token_pos,
684+
deopt_id, dst_name, locs);
685+
return;
686+
}
680687

681688
if (Instance::NullIsAssignableTo(dst_type)) {
682689
__ CompareObject(TypeTestABI::kInstanceReg, Object::null_object());
@@ -686,45 +693,56 @@ void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type,
686693
__ PushPair(TypeTestABI::kFunctionTypeArgumentsReg,
687694
TypeTestABI::kInstantiatorTypeArgumentsReg);
688695

689-
// Generate inline type check, linking to runtime call if not assignable.
690-
SubtypeTestCache& test_cache = SubtypeTestCache::ZoneHandle(zone());
691696
test_cache = GenerateInlineInstanceof(token_pos, dst_type, &is_assignable,
692697
&runtime_call);
698+
} else {
699+
// TODO(dartbug.com/40813): Handle setting up the non-constant case.
700+
UNREACHABLE();
701+
}
693702

694-
__ Bind(&runtime_call);
695-
__ ldp(TypeTestABI::kFunctionTypeArgumentsReg,
696-
TypeTestABI::kInstantiatorTypeArgumentsReg,
697-
compiler::Address(SP, 0 * kWordSize, compiler::Address::PairOffset));
698-
__ PushPair(TypeTestABI::kInstanceReg,
699-
NULL_REG); // Make room for the result and
700-
// push the source object.
701-
__ LoadObject(TMP, dst_type); // Push the type of the dest.
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());
702712
__ PushPair(TypeTestABI::kInstantiatorTypeArgumentsReg, TMP);
703-
__ LoadObject(TMP, dst_name); // Push the name of the destination.
704-
__ PushPair(TMP, TypeTestABI::kFunctionTypeArgumentsReg);
705-
706-
__ LoadUniqueObject(R0, test_cache);
707-
__ LoadObject(TMP, Smi::ZoneHandle(zone(), Smi::New(kTypeCheckFromInline)));
708-
__ PushPair(TMP, R0);
709-
GenerateRuntimeCall(token_pos, deopt_id, kTypeCheckRuntimeEntry, 7, locs);
710-
// Pop the parameters supplied to the runtime entry. The result of the
711-
// type check runtime call is the checked value.
712-
__ Drop(7);
713-
__ Pop(R0);
714-
__ Bind(&is_assignable);
715-
__ PopPair(TypeTestABI::kFunctionTypeArgumentsReg,
716-
TypeTestABI::kInstantiatorTypeArgumentsReg);
717-
__ Bind(&is_assignable_fast);
713+
} else {
714+
// TODO(dartbug.com/40813): Handle setting up the non-constant case.
715+
UNREACHABLE();
718716
}
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);
719733
}
720734

721735
void FlowGraphCompiler::GenerateAssertAssignableViaTypeTestingStub(
722736
CompileType* receiver_type,
723737
TokenPosition token_pos,
724738
intptr_t deopt_id,
725-
const AbstractType& dst_type,
726739
const String& dst_name,
727740
LocationSummary* locs) {
741+
ASSERT(CheckAssertAssignableTypeTestingABILocations(*locs));
742+
// We must have a constant dst_type for generating a call to the stub.
743+
ASSERT(locs->in(1).IsConstant());
744+
const auto& dst_type = AbstractType::Cast(locs->in(1).constant());
745+
728746
// If the dst_type is instantiated we know the target TTS stub at
729747
// compile-time and can therefore use a pc-relative call.
730748
const bool use_pc_relative_call = dst_type.IsInstantiated() &&

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

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,7 @@ void FlowGraphCompiler::GenerateInstanceOf(TokenPosition token_pos,
664664
// - Class equality (only if class is not parameterized).
665665
// Inputs:
666666
// - EAX: object.
667+
// - EBX: destination type (if non-constant).
667668
// - EDX: instantiator type arguments or raw_null.
668669
// - ECX: function type arguments or raw_null.
669670
// Returns:
@@ -673,14 +674,21 @@ void FlowGraphCompiler::GenerateInstanceOf(TokenPosition token_pos,
673674
void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type,
674675
TokenPosition token_pos,
675676
intptr_t deopt_id,
676-
const AbstractType& dst_type,
677677
const String& dst_name,
678678
LocationSummary* locs) {
679679
ASSERT(!token_pos.IsClassifying());
680-
ASSERT(!dst_type.IsNull());
680+
ASSERT(CheckAssertAssignableTypeTestingABILocations(*locs));
681+
682+
if (!locs->in(1).IsConstant()) {
683+
// TODO(dartbug.com/40813): Handle setting up the non-constant case.
684+
UNREACHABLE();
685+
}
686+
687+
ASSERT(locs->in(1).constant().IsAbstractType());
688+
const auto& dst_type = AbstractType::Cast(locs->in(1).constant());
681689
ASSERT(dst_type.IsFinalized());
682-
// Assignable check is skipped in FlowGraphBuilder, not here.
683-
ASSERT(!dst_type.IsTopTypeForSubtyping());
690+
691+
if (dst_type.IsTopTypeForSubtyping()) return; // No code needed.
684692

685693
__ pushl(TypeTestABI::kInstantiatorTypeArgumentsReg);
686694
__ pushl(TypeTestABI::kFunctionTypeArgumentsReg);
@@ -689,7 +697,7 @@ void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type,
689697
if (Instance::NullIsAssignableTo(dst_type)) {
690698
const compiler::Immediate& raw_null =
691699
compiler::Immediate(static_cast<intptr_t>(Object::null()));
692-
__ cmpl(EAX, raw_null);
700+
__ cmpl(TypeTestABI::kInstanceReg, raw_null);
693701
__ j(EQUAL, &is_assignable);
694702
}
695703

@@ -705,8 +713,13 @@ void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type,
705713
__ movl(TypeTestABI::kFunctionTypeArgumentsReg,
706714
compiler::Address(ESP, 0 * kWordSize)); // Get function type args.
707715
__ PushObject(Object::null_object()); // Make room for the result.
708-
__ pushl(EAX); // Push the source object.
709-
__ PushObject(dst_type); // Push the type of the destination.
716+
__ pushl(TypeTestABI::kInstanceReg); // Push the source object.
717+
if (locs->in(1).IsConstant()) {
718+
__ PushObject(locs->in(1).constant()); // Push the type of the destination.
719+
} else {
720+
// TODO(dartbug.com/40813): Handle setting up the non-constant case.
721+
UNREACHABLE();
722+
}
710723
__ pushl(TypeTestABI::kInstantiatorTypeArgumentsReg);
711724
__ pushl(TypeTestABI::kFunctionTypeArgumentsReg);
712725
__ PushObject(dst_name); // Push the name of the destination.
@@ -717,7 +730,7 @@ void FlowGraphCompiler::GenerateAssertAssignable(CompileType* receiver_type,
717730
// Pop the parameters supplied to the runtime entry. The result of the
718731
// type check runtime call is the checked value.
719732
__ Drop(7);
720-
__ popl(EAX);
733+
__ popl(TypeTestABI::kInstanceReg);
721734

722735
__ Bind(&is_assignable);
723736
__ popl(TypeTestABI::kFunctionTypeArgumentsReg);

0 commit comments

Comments
 (0)