Skip to content

Commit d9685d1

Browse files
sstricklcommit-bot@chromium.org
sstrickl
authored and
[email protected]
committed
[vm/compiler] Move type argument type checks out of closure bodies.
These checks are now performed in the dynamic closure call dispatcher. To avoid having to create type argument vectors (TAVs) for default type arguments at runtime, we cache a compile-time created TAV in the ClosureData for the closure function which is retrieved by the dispatcher when needed. We also keep an associated packed field of information that can also be determined at compile time: * Whether the cached TAV needs instantiation or can share its instantiator or function type arguments. * The number of parent type parameters. The former allows the generated IL to keep the invariant that the InstantiateTypeArguments instruction (and the runtime entry it calls) is only used for uninstantiated TAVs. Also changes the destination name to an Value input for the AssertSubtype instruction and adds handling for non-constant types and names in that instruction's backend. Additional changes: * Adds new slots for ClosureData, Function, and TypeArguments. * Adds a new kUnboxedUint8 representation (needed for TypeParameterLayout::flags_, which is of type uint8_t). * Extends LoadField to handle uint8_t unboxed native fields. * Adds BoxUint8 for boxing unboxed uint8_t values. Code size impact on Flutter gallery in release mode: * arm7: total +0.08%, vmisolate: +0.56%, isolate: +0.42%, readonly: -0.04%, instructions: -0.03% * arm8: total +0.12%, vmisolate: +0.56%, isolate: +0.42%, readonly: -0.002%, instructions: +0.03% Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-debug-ia32-try,vm-kernel-linux-debug-x64-try,vm-kernel-nnbd-linux-debug-ia32-try,vm-kernel-nnbd-linux-debug-x64-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-nnbd-linux-debug-simarm_x64-try,vm-kernel-precomp-nnbd-linux-debug-x64-try,vm-kernel-linux-release-simarm-try,vm-kernel-linux-release-simarm64-try,vm-kernel-nnbd-linux-release-simarm-try,vm-kernel-nnbd-linux-release-simarm64-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try,vm-kernel-precomp-nnbd-linux-release-simarm64-try Bug: #40813 Change-Id: I5a7de27a17e3119e27752bd0d10e1c6bc1b52a16 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/158844 Commit-Queue: Tess Strickland <[email protected]> Reviewed-by: Martin Kustermann <[email protected]> Reviewed-by: Régis Crelier <[email protected]>
1 parent 6cc06b3 commit d9685d1

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+1618
-432
lines changed

‎runtime/vm/class_finalizer.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -847,6 +847,7 @@ void ClassFinalizer::FinalizeSignature(const Function& function,
847847
type_param.set_default_argument(finalized_type);
848848
}
849849
}
850+
function.UpdateCachedDefaultTypeArguments(Thread::Current());
850851
}
851852
// Finalize result type.
852853
type = function.result_type();

‎runtime/vm/clustered_snapshot.cc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -790,6 +790,8 @@ class ClosureDataSerializationCluster : public SerializationCluster {
790790
s->Push(data->ptr()->parent_function_);
791791
s->Push(data->ptr()->signature_type_);
792792
s->Push(data->ptr()->closure_);
793+
s->Push(data->ptr()->default_type_arguments_);
794+
s->Push(data->ptr()->default_type_arguments_info_);
793795
}
794796

795797
void WriteAlloc(Serializer* s) {
@@ -813,6 +815,8 @@ class ClosureDataSerializationCluster : public SerializationCluster {
813815
WriteField(data, parent_function_);
814816
WriteField(data, signature_type_);
815817
WriteField(data, closure_);
818+
WriteField(data, default_type_arguments_);
819+
WriteField(data, default_type_arguments_info_);
816820
}
817821
}
818822

@@ -851,6 +855,10 @@ class ClosureDataDeserializationCluster : public DeserializationCluster {
851855
data->ptr()->parent_function_ = static_cast<FunctionPtr>(d->ReadRef());
852856
data->ptr()->signature_type_ = static_cast<TypePtr>(d->ReadRef());
853857
data->ptr()->closure_ = static_cast<InstancePtr>(d->ReadRef());
858+
data->ptr()->default_type_arguments_ =
859+
static_cast<TypeArgumentsPtr>(d->ReadRef());
860+
data->ptr()->default_type_arguments_info_ =
861+
static_cast<SmiPtr>(d->ReadRef());
854862
}
855863
}
856864
};

‎runtime/vm/compiler/aot/precompiler.cc

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,14 @@ void Precompiler::AddTypesOf(const Function& function) {
796796
type = function.ParameterTypeAt(i);
797797
AddType(type);
798798
}
799+
// At this point, ensure any cached default type arguments are canonicalized.
800+
function.UpdateCachedDefaultTypeArguments(thread());
801+
if (function.CachesDefaultTypeArguments()) {
802+
const auto& defaults = TypeArguments::Handle(
803+
Z, function.default_type_arguments(/*kind_out=*/nullptr));
804+
ASSERT(defaults.IsCanonical());
805+
AddTypeArguments(defaults);
806+
}
799807
Code& code = Code::Handle(Z, function.CurrentCode());
800808
if (code.IsNull()) {
801809
ASSERT(function.kind() == FunctionLayout::kSignatureFunction);
@@ -834,18 +842,17 @@ void Precompiler::AddType(const AbstractType& abstype) {
834842
if (abstype.IsNull()) return;
835843

836844
if (abstype.IsTypeParameter()) {
837-
if (typeparams_to_retain_.HasKey(&TypeParameter::Cast(abstype))) return;
838-
typeparams_to_retain_.Insert(
839-
&TypeParameter::ZoneHandle(Z, TypeParameter::Cast(abstype).raw()));
845+
const auto& param = TypeParameter::Cast(abstype);
846+
if (typeparams_to_retain_.HasKey(&param)) return;
847+
typeparams_to_retain_.Insert(&TypeParameter::ZoneHandle(Z, param.raw()));
840848

841-
const AbstractType& type =
842-
AbstractType::Handle(Z, TypeParameter::Cast(abstype).bound());
849+
auto& type = AbstractType::Handle(Z, param.bound());
850+
AddType(type);
851+
type = param.default_argument();
843852
AddType(type);
844-
const auto& function = Function::Handle(
845-
Z, TypeParameter::Cast(abstype).parameterized_function());
853+
const auto& function = Function::Handle(Z, param.parameterized_function());
846854
AddTypesOf(function);
847-
const Class& cls =
848-
Class::Handle(Z, TypeParameter::Cast(abstype).parameterized_class());
855+
const Class& cls = Class::Handle(Z, param.parameterized_class());
849856
AddTypesOf(cls);
850857
return;
851858
}

‎runtime/vm/compiler/assembler/assembler_arm.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2207,6 +2207,7 @@ OperandSize Address::OperandSizeFor(intptr_t cid) {
22072207
switch (cid) {
22082208
case kArrayCid:
22092209
case kImmutableArrayCid:
2210+
case kTypeArgumentsCid:
22102211
return kWord;
22112212
case kOneByteStringCid:
22122213
case kExternalOneByteStringCid:

‎runtime/vm/compiler/assembler/assembler_arm64.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,7 @@ class Address : public ValueObject {
266266
switch (cid) {
267267
case kArrayCid:
268268
case kImmutableArrayCid:
269+
case kTypeArgumentsCid:
269270
return kWord;
270271
case kOneByteStringCid:
271272
case kExternalOneByteStringCid:

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

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,10 +1056,13 @@ void ConstantPropagator::VisitInstantiateTypeArguments(
10561056
InstantiateTypeArgumentsInstr* instr) {
10571057
const auto& type_arguments_obj =
10581058
instr->type_arguments()->definition()->constant_value();
1059-
ASSERT(!type_arguments_obj.IsNull());
10601059
if (IsUnknown(type_arguments_obj)) {
10611060
return;
10621061
}
1062+
if (type_arguments_obj.IsNull()) {
1063+
SetValue(instr, type_arguments_obj);
1064+
return;
1065+
}
10631066
if (!type_arguments_obj.IsTypeArguments()) {
10641067
SetValue(instr, non_constant_);
10651068
return;
@@ -1078,14 +1081,13 @@ void ConstantPropagator::VisitInstantiateTypeArguments(
10781081
if (IsUnknown(instantiator_type_args_obj)) {
10791082
return;
10801083
}
1081-
if (!instantiator_type_args_obj.IsTypeArguments()) {
1084+
if (!instantiator_type_args_obj.IsNull() &&
1085+
!instantiator_type_args_obj.IsTypeArguments()) {
10821086
SetValue(instr, non_constant_);
10831087
return;
10841088
}
10851089
instantiator_type_args ^= instantiator_type_args_obj.raw();
1086-
ASSERT(!instr->instantiator_class().IsNull());
1087-
if (type_arguments.CanShareInstantiatorTypeArguments(
1088-
instr->instantiator_class())) {
1090+
if (instr->CanShareInstantiatorTypeArguments()) {
10891091
SetValue(instr, instantiator_type_args);
10901092
return;
10911093
}
@@ -1098,13 +1100,13 @@ void ConstantPropagator::VisitInstantiateTypeArguments(
10981100
if (IsUnknown(function_type_args_obj)) {
10991101
return;
11001102
}
1101-
if (!function_type_args_obj.IsTypeArguments()) {
1103+
if (!function_type_args_obj.IsNull() &&
1104+
!function_type_args_obj.IsTypeArguments()) {
11021105
SetValue(instr, non_constant_);
11031106
return;
11041107
}
11051108
function_type_args ^= function_type_args_obj.raw();
1106-
ASSERT(!instr->function().IsNull());
1107-
if (type_arguments.CanShareFunctionTypeArguments(instr->function())) {
1109+
if (instr->CanShareFunctionTypeArguments()) {
11081110
SetValue(instr, function_type_args);
11091111
return;
11101112
}
@@ -1424,6 +1426,11 @@ void ConstantPropagator::VisitBox(BoxInstr* instr) {
14241426
SetValue(instr, non_constant_);
14251427
}
14261428

1429+
void ConstantPropagator::VisitBoxUint8(BoxUint8Instr* instr) {
1430+
// TODO(kmillikin): Handle box operation.
1431+
SetValue(instr, non_constant_);
1432+
}
1433+
14271434
void ConstantPropagator::VisitBoxUint32(BoxUint32Instr* instr) {
14281435
// TODO(kmillikin): Handle box operation.
14291436
SetValue(instr, non_constant_);

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2470,14 +2470,16 @@ void FlowGraphCompiler::FrameStatePush(Definition* defn) {
24702470
rep = kTagged;
24712471
}
24722472
ASSERT(!is_optimizing());
2473-
ASSERT((rep == kTagged) || (rep == kUntagged) || (rep == kUnboxedUint32));
2473+
ASSERT((rep == kTagged) || (rep == kUntagged) || (rep == kUnboxedUint32) ||
2474+
(rep == kUnboxedUint8));
24742475
ASSERT(rep != kUntagged || flow_graph_.IsIrregexpFunction());
24752476
const auto& function = flow_graph_.parsed_function().function();
2476-
// Currently, we only allow unboxed uint32 on the stack in unoptimized code
2477-
// when building a dynamic closure call dispatcher, where any unboxed values
2478-
// on the stack are consumed before possible FrameStateIsSafeToCall() checks.
2477+
// Currently, we only allow unboxed uint8 and uint32 on the stack in
2478+
// unoptimized code when building a dynamic closure call dispatcher, where
2479+
// any unboxed values on the stack are consumed before possible
2480+
// FrameStateIsSafeToCall() checks.
24792481
// See FlowGraphBuilder::BuildDynamicCallVarsInit().
2480-
ASSERT(rep != kUnboxedUint32 ||
2482+
ASSERT((rep != kUnboxedUint32 && rep != kUnboxedUint8) ||
24812483
function.IsDynamicClosureCallDispatcher(thread()));
24822484
frame_state_.Add(rep);
24832485
}

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

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2719,6 +2719,7 @@ bool LoadFieldInstr::IsImmutableLengthLoad() const {
27192719
case Slot::Kind::kArgumentsDescriptor_positional_count:
27202720
case Slot::Kind::kArgumentsDescriptor_count:
27212721
case Slot::Kind::kArgumentsDescriptor_size:
2722+
case Slot::Kind::kArrayElement:
27222723
case Slot::Kind::kTypeArguments:
27232724
case Slot::Kind::kTypedDataView_offset_in_bytes:
27242725
case Slot::Kind::kTypedDataView_data:
@@ -2730,16 +2731,22 @@ bool LoadFieldInstr::IsImmutableLengthLoad() const {
27302731
case Slot::Kind::kClosure_function_type_arguments:
27312732
case Slot::Kind::kClosure_instantiator_type_arguments:
27322733
case Slot::Kind::kClosure_hash:
2734+
case Slot::Kind::kClosureData_default_type_arguments:
2735+
case Slot::Kind::kClosureData_default_type_arguments_info:
27332736
case Slot::Kind::kCapturedVariable:
27342737
case Slot::Kind::kDartField:
2738+
case Slot::Kind::kFunction_data:
2739+
case Slot::Kind::kFunction_kind_tag:
27352740
case Slot::Kind::kFunction_packed_fields:
27362741
case Slot::Kind::kFunction_parameter_names:
27372742
case Slot::Kind::kFunction_parameter_types:
27382743
case Slot::Kind::kFunction_type_parameters:
27392744
case Slot::Kind::kPointerBase_data_field:
27402745
case Slot::Kind::kType_arguments:
27412746
case Slot::Kind::kTypeArgumentsIndex:
2742-
case Slot::Kind::kArrayElement:
2747+
case Slot::Kind::kTypeParameter_bound:
2748+
case Slot::Kind::kTypeParameter_flags:
2749+
case Slot::Kind::kTypeParameter_name:
27432750
case Slot::Kind::kUnhandledException_exception:
27442751
case Slot::Kind::kUnhandledException_stacktrace:
27452752
return false;
@@ -2756,6 +2763,7 @@ bool LoadFieldInstr::IsFixedLengthArrayCid(intptr_t cid) {
27562763
switch (cid) {
27572764
case kArrayCid:
27582765
case kImmutableArrayCid:
2766+
case kTypeArgumentsCid:
27592767
return true;
27602768
default:
27612769
return false;
@@ -3779,6 +3787,9 @@ bool CheckNullInstr::AttributesEqual(Instruction* other) const {
37793787

37803788
BoxInstr* BoxInstr::Create(Representation from, Value* value) {
37813789
switch (from) {
3790+
case kUnboxedUint8:
3791+
return new BoxUint8Instr(value);
3792+
37823793
case kUnboxedInt32:
37833794
return new BoxInt32Instr(value);
37843795

@@ -5267,36 +5278,40 @@ void AssertAssignableInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
52675278

52685279
LocationSummary* AssertSubtypeInstr::MakeLocationSummary(Zone* zone,
52695280
bool opt) const {
5270-
if (!sub_type()->BindsToConstant() || !super_type()->BindsToConstant()) {
5271-
// TODO(dartbug.com/40813): Handle setting up the non-constant case.
5272-
UNREACHABLE();
5273-
}
5274-
const intptr_t kNumInputs = 4;
5281+
const intptr_t kNumInputs = 5;
52755282
const intptr_t kNumTemps = 0;
52765283
LocationSummary* summary = new (zone)
52775284
LocationSummary(zone, kNumInputs, kNumTemps, LocationSummary::kCall);
5278-
summary->set_in(0, Location::RegisterLocation(
5279-
TypeTestABI::kInstantiatorTypeArgumentsReg));
5285+
summary->set_in(kInstantiatorTAVPos,
5286+
Location::RegisterLocation(
5287+
AssertSubtypeABI::kInstantiatorTypeArgumentsReg));
52805288
summary->set_in(
5281-
1, Location::RegisterLocation(TypeTestABI::kFunctionTypeArgumentsReg));
5282-
summary->set_in(2,
5283-
Location::Constant(sub_type()->definition()->AsConstant()));
5284-
summary->set_in(3,
5285-
Location::Constant(super_type()->definition()->AsConstant()));
5289+
kFunctionTAVPos,
5290+
Location::RegisterLocation(AssertSubtypeABI::kFunctionTypeArgumentsReg));
5291+
summary->set_in(kSubTypePos,
5292+
Location::RegisterLocation(AssertSubtypeABI::kSubTypeReg));
5293+
summary->set_in(kSuperTypePos,
5294+
Location::RegisterLocation(AssertSubtypeABI::kSuperTypeReg));
5295+
summary->set_in(kDstNamePos,
5296+
Location::RegisterLocation(AssertSubtypeABI::kDstNameReg));
52865297
return summary;
52875298
}
52885299

52895300
void AssertSubtypeInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
5290-
__ PushRegister(locs()->in(0).reg());
5291-
__ PushRegister(locs()->in(1).reg());
5292-
__ PushObject(locs()->in(2).constant());
5293-
__ PushObject(locs()->in(3).constant());
5294-
__ PushObject(dst_name());
5295-
5301+
#if defined(TARGET_ARCH_IA32)
5302+
__ PushRegister(AssertSubtypeABI::kInstantiatorTypeArgumentsReg);
5303+
__ PushRegister(AssertSubtypeABI::kFunctionTypeArgumentsReg);
5304+
__ PushRegister(AssertSubtypeABI::kSubTypeReg);
5305+
__ PushRegister(AssertSubtypeABI::kSuperTypeReg);
5306+
__ PushRegister(AssertSubtypeABI::kDstNameReg);
52965307
compiler->GenerateRuntimeCall(token_pos(), deopt_id(),
52975308
kSubtypeCheckRuntimeEntry, 5, locs());
52985309

52995310
__ Drop(5);
5311+
#else
5312+
compiler->GenerateStubCall(token_pos(), StubCode::AssertSubtype(),
5313+
PcDescriptorsLayout::kOther, locs());
5314+
#endif
53005315
}
53015316

53025317
LocationSummary* DeoptimizeInstr::MakeLocationSummary(Zone* zone,

0 commit comments

Comments
 (0)