Skip to content

Commit 2b1c0f5

Browse files
sstricklCommit Queue
sstrickl
authored and
Commit Queue
committed
[vm] Reorder SubtypeTestCache inputs to improve non-dynamic cases.
In particular, avoid storing/comparing the destination type unless the SubtypeTestCache was created for a dynamic AssertAssignable instruction. TEST=vm/cc/STC_{Linear,Hash}Lookup, vm/cc/TTS Bug: #48345 Change-Id: I279c69a668ce19785785cf71707acf951f2b2133 Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-simriscv64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-release-x64-try,vm-aot-linux-product-x64-try,vm-aot-linux-release-simarm64-try,vm-aot-linux-release-simarm_x64-try,vm-aot-linux-debug-x64c-try,vm-aot-tsan-linux-release-x64-try,vm-aot-mac-release-arm64-try,vm-kernel-precomp-linux-release-x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-aot-dwarf-linux-product-x64-try,vm-linux-release-ia32-try,vm-linux-debug-x64c-try,vm-linux-debug-x64-try,vm-linux-debug-simriscv64-try,vm-linux-release-simarm64-try,vm-linux-release-simarm-try,vm-mac-release-arm64-try,vm-mac-release-x64-try,vm-tsan-linux-release-x64-try,vm-reload-rollback-linux-release-x64-try,vm-reload-linux-release-x64-try,vm-ffi-qemu-linux-release-arm-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310340 Reviewed-by: Martin Kustermann <[email protected]> Reviewed-by: Alexander Markov <[email protected]> Commit-Queue: Tess Strickland <[email protected]>
1 parent 194767e commit 2b1c0f5

22 files changed

+905
-906
lines changed

‎runtime/vm/app_snapshot.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3560,6 +3560,7 @@ class SubtypeTestCacheSerializationCluster : public SerializationCluster {
35603560
AutoTraceObject(cache);
35613561
WriteField(cache, cache_);
35623562
s->Write<uint32_t>(cache->untag()->num_inputs_);
3563+
s->Write<uint32_t>(cache->untag()->num_occupied_);
35633564
}
35643565
}
35653566

@@ -3588,6 +3589,7 @@ class SubtypeTestCacheDeserializationCluster : public DeserializationCluster {
35883589
SubtypeTestCache::InstanceSize());
35893590
cache->untag()->cache_ = static_cast<ArrayPtr>(d.ReadRef());
35903591
cache->untag()->num_inputs_ = d.Read<uint32_t>();
3592+
cache->untag()->num_occupied_ = d.Read<uint32_t>();
35913593
}
35923594
}
35933595
};

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

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2329,12 +2329,9 @@ SubtypeTestCachePtr FlowGraphCompiler::GenerateFunctionTypeTest(
23292329
__ Comment("FunctionTypeTest");
23302330

23312331
__ BranchIfSmi(TypeTestABI::kInstanceReg, is_not_instance_lbl);
2332-
// Load the type into the right register for the subtype test cache check.
2333-
__ LoadUniqueObject(TypeTestABI::kDstTypeReg, type);
23342332
// Uninstantiated type class is known at compile time, but the type
23352333
// arguments are determined at runtime by the instantiator(s).
2336-
2337-
return GenerateCallSubtypeTestStub(TypeTestStubKind::kTestTypeSevenArgs,
2334+
return GenerateCallSubtypeTestStub(TypeTestStubKind::kTestTypeSixArgs,
23382335
is_instance_lbl, is_not_instance_lbl);
23392336
}
23402337

@@ -2407,14 +2404,14 @@ FlowGraphCompiler::TypeTestStubKind
24072404
FlowGraphCompiler::GetTypeTestStubKindForTypeParameter(
24082405
const TypeParameter& type_param) {
24092406
// If it's guaranteed, by type-parameter bound, that the type parameter will
2410-
// never have a value of a function type, then we can safely do a 5-type
2411-
// test instead of a 7-type test.
2407+
// never have a value of a function type, then we can safely do a 4-type
2408+
// test instead of a 6-type test.
24122409
AbstractType& bound = AbstractType::Handle(zone(), type_param.bound());
24132410
bound = bound.UnwrapFutureOr();
24142411
return !bound.IsTopTypeForSubtyping() && !bound.IsObjectType() &&
24152412
!bound.IsDartFunctionType() && bound.IsType()
2416-
? TypeTestStubKind::kTestTypeFiveArgs
2417-
: TypeTestStubKind::kTestTypeSevenArgs;
2413+
? TypeTestStubKind::kTestTypeFourArgs
2414+
: TypeTestStubKind::kTestTypeSixArgs;
24182415
}
24192416

24202417
// Generates quick and subtype cache tests when only the instance need be
@@ -2533,10 +2530,8 @@ FlowGraphCompiler::GenerateInstantiatedTypeWithArgumentsTest(
25332530
}
25342531
}
25352532

2536-
// Load the type into the right register for the subtype test cache check.
2537-
__ LoadUniqueObject(TypeTestABI::kDstTypeReg, type);
25382533
// Regular subtype test cache involving instance's type arguments.
2539-
return GenerateCallSubtypeTestStub(TypeTestStubKind::kTestTypeThreeArgs,
2534+
return GenerateCallSubtypeTestStub(TypeTestStubKind::kTestTypeTwoArgs,
25402535
is_instance_lbl, is_not_instance_lbl);
25412536
}
25422537

@@ -2664,8 +2659,6 @@ SubtypeTestCachePtr FlowGraphCompiler::GenerateUninstantiatedTypeTest(
26642659
// Smi can be handled by type test cache.
26652660
__ Bind(&not_smi);
26662661

2667-
// Load the type into the right register for the subtype test cache check.
2668-
__ LoadUniqueObject(TypeTestABI::kDstTypeReg, type);
26692662
const auto test_kind = GetTypeTestStubKindForTypeParameter(type_param);
26702663
return GenerateCallSubtypeTestStub(test_kind, is_instance_lbl,
26712664
is_not_instance_lbl);
@@ -2676,11 +2669,9 @@ SubtypeTestCachePtr FlowGraphCompiler::GenerateUninstantiatedTypeTest(
26762669
if (!type.IsFutureOrType()) {
26772670
__ BranchIfSmi(TypeTestABI::kInstanceReg, is_not_instance_lbl);
26782671
}
2679-
// Load the type into the right register for the subtype test cache check.
2680-
__ LoadUniqueObject(TypeTestABI::kDstTypeReg, type);
26812672
// Uninstantiated type class is known at compile time, but the type
26822673
// arguments are determined at runtime by the instantiator(s).
2683-
return GenerateCallSubtypeTestStub(TypeTestStubKind::kTestTypeFiveArgs,
2674+
return GenerateCallSubtypeTestStub(TypeTestStubKind::kTestTypeFourArgs,
26842675
is_instance_lbl, is_not_instance_lbl);
26852676
}
26862677
return SubtypeTestCache::null();
@@ -2752,11 +2743,10 @@ void FlowGraphCompiler::GenerateInstanceOf(const InstructionSource& source,
27522743
#if !defined(TARGET_ARCH_IA32)
27532744
// Expected inputs (from TypeTestABI):
27542745
// - kInstanceReg: instance (preserved).
2755-
// - kDstTypeReg: destination type (for test_kind != kTestTypeOneArg).
27562746
// - kInstantiatorTypeArgumentsReg: instantiator type arguments
2757-
// (for test_kind == kTestTypeFiveArg or test_kind == kTestTypeSevenArg).
2747+
// (for test_kind == kTestTypeFourArg or test_kind == kTestTypeSixArg).
27582748
// - kFunctionTypeArgumentsReg: function type arguments
2759-
// (for test_kind == kTestTypeFiveArg or test_kind == kTestTypeSevenArg).
2749+
// (for test_kind == kTestTypeFourArg or test_kind == kTestTypeSixArg).
27602750
//
27612751
// See the arch-specific GenerateSubtypeNTestCacheStub method to see which
27622752
// registers may need saving across this call.

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,8 +1024,9 @@ class FlowGraphCompiler : public ValueObject {
10241024

10251025
enum class TypeTestStubKind {
10261026
kTestTypeOneArg = 1,
1027-
kTestTypeThreeArgs = 3,
1028-
kTestTypeFiveArgs = 5,
1027+
kTestTypeTwoArgs = 2,
1028+
kTestTypeFourArgs = 4,
1029+
kTestTypeSixArgs = 6,
10291030
kTestTypeSevenArgs = 7,
10301031
};
10311032

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

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -188,27 +188,18 @@ void FlowGraphCompiler::GenerateBoolToJump(Register bool_register,
188188

189189
// Input registers (from TypeTestABI):
190190
// - kInstanceReg: instance.
191-
// - kDstTypeReg: destination type (for test_kind != kTestTypeOneArg).
191+
// - kDstTypeReg: destination type (for test_kind == kTestTypeSevenArg).
192192
// - kInstantiatorTypeArgumentsReg: instantiator type arguments
193-
// (for test_kind == kTestTypeFiveArg or test_kind == kTestTypeSevenArg).
193+
// (for test_kind == kTestTypeFourArg, kTestTypeSixArg, or kTestTypeSevenArg).
194194
// - kFunctionTypeArgumentsReg: function type arguments
195-
// (for test_kind == kTestTypeFiveArg or test_kind == kTestTypeSevenArg).
195+
// (for test_kind == kTestTypeFourArg, kTestTypeSixArg, or kTestTypeSevenArg).
196196
//
197197
// Only preserves kInstanceReg from TypeTestABI, all other TypeTestABI
198198
// registers may be used and thus must be saved by the caller.
199199
SubtypeTestCachePtr FlowGraphCompiler::GenerateCallSubtypeTestStub(
200200
TypeTestStubKind test_kind,
201201
compiler::Label* is_instance_lbl,
202202
compiler::Label* is_not_instance_lbl) {
203-
// Used for registers that may not have GC-safe values to push. Pushes
204-
// the null object if the condition is not met.
205-
auto conditional_push = [&](Register reg, bool condition) {
206-
if (condition) {
207-
__ pushl(reg);
208-
} else {
209-
__ PushObject(Object::null_object());
210-
}
211-
};
212203
const intptr_t num_inputs = UsedInputsForTTSKind(test_kind);
213204
const SubtypeTestCache& type_test_cache =
214205
SubtypeTestCache::ZoneHandle(zone(), SubtypeTestCache::New(num_inputs));
@@ -217,9 +208,23 @@ SubtypeTestCachePtr FlowGraphCompiler::GenerateCallSubtypeTestStub(
217208
__ LoadObject(TypeTestABI::kSubtypeTestCacheReg, type_test_cache);
218209
__ pushl(TypeTestABI::kSubtypeTestCacheReg);
219210
__ pushl(TypeTestABI::kInstanceReg);
220-
conditional_push(TypeTestABI::kDstTypeReg, num_inputs >= 3);
221-
conditional_push(TypeTestABI::kInstantiatorTypeArgumentsReg, num_inputs >= 4);
222-
conditional_push(TypeTestABI::kFunctionTypeArgumentsReg, num_inputs >= 5);
211+
// Registers for unused inputs may not have GC-safe values to push, so push
212+
// the null object if the input is unused instead.
213+
if (num_inputs >= 7) {
214+
__ pushl(TypeTestABI::kDstTypeReg);
215+
} else {
216+
__ PushObject(Object::null_object());
217+
}
218+
if (num_inputs >= 3) {
219+
__ pushl(TypeTestABI::kInstantiatorTypeArgumentsReg);
220+
} else {
221+
__ PushObject(Object::null_object());
222+
}
223+
if (num_inputs >= 4) {
224+
__ pushl(TypeTestABI::kFunctionTypeArgumentsReg);
225+
} else {
226+
__ PushObject(Object::null_object());
227+
}
223228
__ Call(stub_entry);
224229
// Restore all but kSubtypeTestCacheReg (since it is the same as
225230
// kSubtypeTestCacheResultReg). Since the generated code is documented as

‎runtime/vm/compiler/runtime_api.cc

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -293,12 +293,16 @@ const Code& StubCodeAllocateArray() {
293293
return dart::StubCode::AllocateArray();
294294
}
295295

296-
const Code& StubCodeSubtype3TestCache() {
297-
return dart::StubCode::Subtype3TestCache();
296+
const Code& StubCodeSubtype2TestCache() {
297+
return dart::StubCode::Subtype2TestCache();
298298
}
299299

300-
const Code& StubCodeSubtype5TestCache() {
301-
return dart::StubCode::Subtype5TestCache();
300+
const Code& StubCodeSubtype4TestCache() {
301+
return dart::StubCode::Subtype4TestCache();
302+
}
303+
304+
const Code& StubCodeSubtype6TestCache() {
305+
return dart::StubCode::Subtype6TestCache();
302306
}
303307

304308
const Code& StubCodeSubtype7TestCache() {

‎runtime/vm/compiler/runtime_api.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,9 @@ uword SymbolsPredefinedAddress();
215215
#endif
216216

217217
const Code& StubCodeAllocateArray();
218-
const Code& StubCodeSubtype3TestCache();
219-
const Code& StubCodeSubtype5TestCache();
218+
const Code& StubCodeSubtype2TestCache();
219+
const Code& StubCodeSubtype4TestCache();
220+
const Code& StubCodeSubtype6TestCache();
220221
const Code& StubCodeSubtype7TestCache();
221222

222223
class RuntimeEntry : public ValueObject {
@@ -1433,7 +1434,6 @@ class SubtypeTestCache : public AllStatic {
14331434
static word cache_offset();
14341435
static word num_inputs_offset();
14351436

1436-
static const word kHeaderSize;
14371437
static const word kMaxInputs;
14381438
static const word kTestEntryLength;
14391439
static const word kInstanceCidOrSignature;

0 commit comments

Comments
 (0)