Skip to content

Commit 95d1ae9

Browse files
sstricklCommit Bot
authored and
Commit Bot
committed
[vm] Reduce class id reloads in TTSes when possible.
Now GenerateCidRangesCheck in the flow graph compiler returns whether the class id register was clobbered, so users that make multiple calls to it for the same cid know whether the cid needs reloading after the generated check. Also unifies FlowGraphCompiler::EmitTestAndCheckCid across architectures, since the non-Intel versions would have always forced reloading. TEST=vm/cc/TTS Change-Id: I08835dc97ba457b7dc8dfa51206b4741a607db22 Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-mac-release-arm64-try,vm-kernel-precomp-linux-release-simarm_x64-try,vm-kernel-nnbd-mac-release-arm64-try,vm-kernel-nnbd-linux-release-simarm64-try,vm-kernel-nnbd-linux-release-simarm-try,vm-kernel-precomp-nnbd-linux-release-simarm_x64-try,vm-precomp-ffi-qemu-linux-release-arm-try,vm-kernel-linux-debug-simriscv64-try,vm-kernel-precomp-linux-debug-simriscv64-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/238583 Reviewed-by: Slava Egorov <[email protected]> Commit-Queue: Tess Strickland <[email protected]>
1 parent 6bec28d commit 95d1ae9

9 files changed

+40
-147
lines changed

runtime/vm/compiler/backend/flow_graph_compiler.cc

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2453,7 +2453,7 @@ bool FlowGraphCompiler::GenerateSubtypeRangeCheck(Register class_id_reg,
24532453
return false;
24542454
}
24552455

2456-
void FlowGraphCompiler::GenerateCidRangesCheck(
2456+
bool FlowGraphCompiler::GenerateCidRangesCheck(
24572457
compiler::Assembler* assembler,
24582458
Register class_id_reg,
24592459
const CidRangeVector& cid_ranges,
@@ -2467,7 +2467,7 @@ void FlowGraphCompiler::GenerateCidRangesCheck(
24672467
if (fall_through_if_inside) {
24682468
assembler->Jump(outside_range_lbl);
24692469
}
2470-
return;
2470+
return false;
24712471
}
24722472

24732473
int bias = 0;
@@ -2484,6 +2484,27 @@ void FlowGraphCompiler::GenerateCidRangesCheck(
24842484
bias = EmitTestAndCallCheckCid(assembler, jump_label, class_id_reg, range,
24852485
bias, jump_on_miss);
24862486
}
2487+
return bias != 0;
2488+
}
2489+
2490+
int FlowGraphCompiler::EmitTestAndCallCheckCid(compiler::Assembler* assembler,
2491+
compiler::Label* label,
2492+
Register class_id_reg,
2493+
const CidRangeValue& range,
2494+
int bias,
2495+
bool jump_on_miss) {
2496+
const intptr_t cid_start = range.cid_start;
2497+
if (range.IsSingleCid()) {
2498+
assembler->CompareImmediate(class_id_reg, cid_start - bias);
2499+
assembler->BranchIf(jump_on_miss ? NOT_EQUAL : EQUAL, label);
2500+
} else {
2501+
assembler->AddImmediate(class_id_reg, bias - cid_start);
2502+
bias = cid_start;
2503+
assembler->CompareImmediate(class_id_reg, range.Extent());
2504+
assembler->BranchIf(jump_on_miss ? UNSIGNED_GREATER : UNSIGNED_LESS_EQUAL,
2505+
label);
2506+
}
2507+
return bias;
24872508
}
24882509

24892510
bool FlowGraphCompiler::CheckAssertAssignableTypeTestingABILocations(

runtime/vm/compiler/backend/flow_graph_compiler.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,9 @@ class FlowGraphCompiler : public ValueObject {
721721
// If [fall_through_if_inside] is `true`, then [outside_range_lbl] must be
722722
// supplied, since it will be jumped to in the last case if the cid is outside
723723
// the range.
724-
static void GenerateCidRangesCheck(compiler::Assembler* assembler,
724+
//
725+
// Returns whether [class_id_reg] is clobbered by the check.
726+
static bool GenerateCidRangesCheck(compiler::Assembler* assembler,
725727
Register class_id_reg,
726728
const CidRangeVector& cid_ranges,
727729
compiler::Label* inside_range_lbl,

runtime/vm/compiler/backend/flow_graph_compiler_arm.cc

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -804,32 +804,6 @@ void FlowGraphCompiler::EmitTestAndCallLoadCid(Register class_id_reg) {
804804
__ LoadClassId(class_id_reg, R0);
805805
}
806806

807-
#undef __
808-
#define __ assembler->
809-
810-
int FlowGraphCompiler::EmitTestAndCallCheckCid(compiler::Assembler* assembler,
811-
compiler::Label* label,
812-
Register class_id_reg,
813-
const CidRangeValue& range,
814-
int bias,
815-
bool jump_on_miss) {
816-
intptr_t cid_start = range.cid_start;
817-
if (range.IsSingleCid()) {
818-
__ AddImmediateSetFlags(class_id_reg, class_id_reg, bias - cid_start);
819-
__ BranchIf(jump_on_miss ? NOT_ZERO : ZERO, label);
820-
bias = cid_start;
821-
} else {
822-
__ AddImmediate(class_id_reg, class_id_reg, bias - cid_start);
823-
__ CompareImmediate(class_id_reg, range.Extent());
824-
__ BranchIf(jump_on_miss ? UNSIGNED_GREATER : UNSIGNED_LESS_EQUAL, label);
825-
bias = cid_start;
826-
}
827-
return bias;
828-
}
829-
830-
#undef __
831-
#define __ assembler()->
832-
833807
void FlowGraphCompiler::EmitMove(Location destination,
834808
Location source,
835809
TemporaryRegisterAllocator* allocator) {

runtime/vm/compiler/backend/flow_graph_compiler_arm64.cc

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -814,32 +814,6 @@ void FlowGraphCompiler::EmitTestAndCallLoadCid(Register class_id_reg) {
814814
__ LoadClassId(class_id_reg, R0);
815815
}
816816

817-
#undef __
818-
#define __ assembler->
819-
820-
int FlowGraphCompiler::EmitTestAndCallCheckCid(compiler::Assembler* assembler,
821-
compiler::Label* label,
822-
Register class_id_reg,
823-
const CidRangeValue& range,
824-
int bias,
825-
bool jump_on_miss) {
826-
const intptr_t cid_start = range.cid_start;
827-
if (range.IsSingleCid()) {
828-
__ AddImmediateSetFlags(class_id_reg, class_id_reg, bias - cid_start);
829-
__ BranchIf(jump_on_miss ? NOT_EQUAL : EQUAL, label);
830-
bias = cid_start;
831-
} else {
832-
__ AddImmediate(class_id_reg, bias - cid_start);
833-
bias = cid_start;
834-
__ CompareImmediate(class_id_reg, range.Extent());
835-
__ BranchIf(jump_on_miss ? UNSIGNED_GREATER : UNSIGNED_LESS_EQUAL, label);
836-
}
837-
return bias;
838-
}
839-
840-
#undef __
841-
#define __ assembler()->
842-
843817
void FlowGraphCompiler::EmitMove(Location destination,
844818
Location source,
845819
TemporaryRegisterAllocator* allocator) {

runtime/vm/compiler/backend/flow_graph_compiler_ia32.cc

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -808,31 +808,6 @@ void FlowGraphCompiler::EmitTestAndCallLoadCid(Register class_id_reg) {
808808
__ LoadClassId(class_id_reg, EAX);
809809
}
810810

811-
#undef __
812-
#define __ assembler->
813-
814-
int FlowGraphCompiler::EmitTestAndCallCheckCid(compiler::Assembler* assembler,
815-
compiler::Label* label,
816-
Register class_id_reg,
817-
const CidRangeValue& range,
818-
int bias,
819-
bool jump_on_miss) {
820-
intptr_t cid_start = range.cid_start;
821-
if (range.IsSingleCid()) {
822-
__ cmpl(class_id_reg, compiler::Immediate(cid_start - bias));
823-
__ j(jump_on_miss ? NOT_EQUAL : EQUAL, label);
824-
} else {
825-
__ addl(class_id_reg, compiler::Immediate(bias - cid_start));
826-
bias = cid_start;
827-
__ cmpl(class_id_reg, compiler::Immediate(range.Extent()));
828-
__ j(jump_on_miss ? ABOVE : BELOW_EQUAL, label); // Unsigned higher.
829-
}
830-
return bias;
831-
}
832-
833-
#undef __
834-
#define __ assembler()->
835-
836811
void FlowGraphCompiler::EmitMove(Location destination,
837812
Location source,
838813
TemporaryRegisterAllocator* tmp) {

runtime/vm/compiler/backend/flow_graph_compiler_riscv.cc

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -771,36 +771,6 @@ void FlowGraphCompiler::EmitTestAndCallLoadCid(Register class_id_reg) {
771771
__ LoadClassId(class_id_reg, A0);
772772
}
773773

774-
#undef __
775-
#define __ assembler->
776-
777-
int FlowGraphCompiler::EmitTestAndCallCheckCid(compiler::Assembler* assembler,
778-
compiler::Label* label,
779-
Register class_id_reg,
780-
const CidRangeValue& range,
781-
int bias,
782-
bool jump_on_miss) {
783-
const intptr_t cid_start = range.cid_start;
784-
if (range.IsSingleCid()) {
785-
__ AddImmediate(class_id_reg, class_id_reg, bias - cid_start);
786-
if (jump_on_miss) {
787-
__ bnez(class_id_reg, label);
788-
} else {
789-
__ beqz(class_id_reg, label);
790-
}
791-
bias = cid_start;
792-
} else {
793-
__ AddImmediate(class_id_reg, class_id_reg, bias - cid_start);
794-
bias = cid_start;
795-
__ CompareImmediate(class_id_reg, range.Extent());
796-
__ BranchIf(jump_on_miss ? UNSIGNED_GREATER : UNSIGNED_LESS_EQUAL, label);
797-
}
798-
return bias;
799-
}
800-
801-
#undef __
802-
#define __ assembler()->
803-
804774
void FlowGraphCompiler::EmitMove(Location destination,
805775
Location source,
806776
TemporaryRegisterAllocator* allocator) {

runtime/vm/compiler/backend/flow_graph_compiler_x64.cc

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -781,34 +781,6 @@ void FlowGraphCompiler::EmitTestAndCallLoadCid(Register class_id_reg) {
781781
__ LoadClassId(class_id_reg, RAX);
782782
}
783783

784-
#undef __
785-
#define __ assembler->
786-
787-
int FlowGraphCompiler::EmitTestAndCallCheckCid(compiler::Assembler* assembler,
788-
compiler::Label* label,
789-
Register class_id_reg,
790-
const CidRangeValue& range,
791-
int bias,
792-
bool jump_on_miss) {
793-
// Note of WARNING: Due to smaller instruction encoding we use the 32-bit
794-
// instructions on x64, which means the compare instruction has to be
795-
// 32-bit (since the subtraction instruction is as well).
796-
intptr_t cid_start = range.cid_start;
797-
if (range.IsSingleCid()) {
798-
__ cmpl(class_id_reg, compiler::Immediate(cid_start - bias));
799-
__ BranchIf(jump_on_miss ? NOT_EQUAL : EQUAL, label);
800-
} else {
801-
__ addl(class_id_reg, compiler::Immediate(bias - cid_start));
802-
bias = cid_start;
803-
__ cmpl(class_id_reg, compiler::Immediate(range.Extent()));
804-
__ BranchIf(jump_on_miss ? UNSIGNED_GREATER : UNSIGNED_LESS_EQUAL, label);
805-
}
806-
return bias;
807-
}
808-
809-
#undef __
810-
#define __ assembler()->
811-
812784
void FlowGraphCompiler::EmitMove(Location destination,
813785
Location source,
814786
TemporaryRegisterAllocator* tmp) {

runtime/vm/type_testing_stubs.cc

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -573,15 +573,15 @@ static void CommentSkippedClasses(compiler::Assembler* assembler,
573573
// type. Falls through or jumps to check_succeeded if the range contains the
574574
// cid, else jumps to check_failed.
575575
//
576-
// Clobbers class_id_reg.
577-
void TypeTestingStubGenerator::BuildOptimizedSubtypeRangeCheck(
576+
// Returns whether class_id_reg is clobbered.
577+
bool TypeTestingStubGenerator::BuildOptimizedSubtypeRangeCheck(
578578
compiler::Assembler* assembler,
579579
const CidRangeVector& ranges,
580580
Register class_id_reg,
581581
compiler::Label* check_succeeded,
582582
compiler::Label* check_failed) {
583583
CommentCheckedClasses(assembler, ranges);
584-
FlowGraphCompiler::GenerateCidRangesCheck(
584+
return FlowGraphCompiler::GenerateCidRangesCheck(
585585
assembler, class_id_reg, ranges, check_succeeded, check_failed, true);
586586
}
587587

@@ -908,6 +908,9 @@ bool TypeTestingStubGenerator::BuildLoadInstanceTypeArguments(
908908
(!cid_checks_only.is_empty() || !type_argument_checks.is_empty())) {
909909
__ BranchIfSmi(TypeTestABI::kInstanceReg, load_failed);
910910
}
911+
// Ensure that if the cid checks only block is skipped, the first iteration
912+
// of the type arguments check will generate a cid load.
913+
bool cid_needs_reload = true;
911914
if (!cid_checks_only.is_empty()) {
912915
compiler::Label is_subtype, keep_looking;
913916
compiler::Label* check_failed =
@@ -917,8 +920,8 @@ bool TypeTestingStubGenerator::BuildLoadInstanceTypeArguments(
917920
} else {
918921
__ LoadClassId(class_id_reg, TypeTestABI::kInstanceReg);
919922
}
920-
BuildOptimizedSubtypeRangeCheck(assembler, cid_checks_only, class_id_reg,
921-
&is_subtype, check_failed);
923+
cid_needs_reload = BuildOptimizedSubtypeRangeCheck(
924+
assembler, cid_checks_only, class_id_reg, &is_subtype, check_failed);
922925
__ Bind(&is_subtype);
923926
__ Ret();
924927
__ Bind(&keep_looking);
@@ -944,9 +947,11 @@ bool TypeTestingStubGenerator::BuildLoadInstanceTypeArguments(
944947
// and avoid emitting a jump to load_succeeded.
945948
compiler::Label* check_failed =
946949
i < vectors.length() - 1 ? &keep_looking : load_failed;
947-
__ LoadClassId(class_id_reg, TypeTestABI::kInstanceReg);
948-
BuildOptimizedSubtypeRangeCheck(assembler, *vector, class_id_reg,
949-
&load_tav, check_failed);
950+
if (cid_needs_reload) {
951+
__ LoadClassId(class_id_reg, TypeTestABI::kInstanceReg);
952+
}
953+
cid_needs_reload = BuildOptimizedSubtypeRangeCheck(
954+
assembler, *vector, class_id_reg, &load_tav, check_failed);
950955
__ Bind(&load_tav);
951956
__ LoadCompressedFieldFromOffset(instance_type_args_reg,
952957
TypeTestABI::kInstanceReg, tav_offset);

runtime/vm/type_testing_stubs.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ class TypeTestingStubGenerator {
7171
const Type& type,
7272
const Class& type_class);
7373

74-
static void BuildOptimizedSubtypeRangeCheck(compiler::Assembler* assembler,
74+
static bool BuildOptimizedSubtypeRangeCheck(compiler::Assembler* assembler,
7575
const CidRangeVector& ranges,
7676
Register class_id_reg,
7777
compiler::Label* check_succeeded,

0 commit comments

Comments
 (0)