Skip to content

Commit 88de61b

Browse files
dcharkescommit-bot@chromium.org
dcharkes
authored and
[email protected]
committed
[vm/ffi] Refactor Zone arguments to first position
We pass `Zone* zone` usually as first argument to functions. This was not the case in existing FFI code. Addressing: https://dart-review.googlesource.com/c/sdk/+/140290/37/runtime/vm/compiler/ffi/native_type.h#181 Change-Id: I858af575848d94381780800eda7a9c506f67eb3e Cq-Include-Trybots: luci.dart.try:vm-precomp-ffi-qemu-linux-release-arm-try,vm-ffi-android-debug-arm64-try,vm-ffi-android-debug-arm-try,vm-kernel-linux-debug-x64-try,vm-kernel-linux-debug-ia32-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170428 Reviewed-by: Martin Kustermann <[email protected]>
1 parent 60323eb commit 88de61b

File tree

11 files changed

+112
-109
lines changed

11 files changed

+112
-109
lines changed

‎runtime/lib/ffi.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,9 @@ static void CheckSized(const AbstractType& type_arg) {
5454
//
5555
// You must check [IsConcreteNativeType] and [CheckSized] first to verify that
5656
// this type has a defined size.
57-
static size_t SizeOf(const AbstractType& type, Zone* zone) {
57+
static size_t SizeOf(Zone* zone, const AbstractType& type) {
5858
if (IsFfiTypeClassId(type.type_class_id())) {
59-
return compiler::ffi::NativeType::FromAbstractType(type, zone)
59+
return compiler::ffi::NativeType::FromAbstractType(zone, type)
6060
.SizeInBytes();
6161
} else {
6262
Class& struct_class = Class::Handle(type.type_class());
@@ -122,7 +122,7 @@ DEFINE_NATIVE_ENTRY(Ffi_loadStruct, 0, 2) {
122122
// TODO(36370): Make representation consistent with kUnboxedFfiIntPtr.
123123
const size_t address =
124124
pointer.NativeAddress() + static_cast<intptr_t>(index.AsInt64Value()) *
125-
SizeOf(pointer_type_arg, zone);
125+
SizeOf(zone, pointer_type_arg);
126126
const Pointer& pointer_offset =
127127
Pointer::Handle(zone, Pointer::New(pointer_type_arg, address));
128128

@@ -142,7 +142,7 @@ DEFINE_NATIVE_ENTRY(Ffi_sizeOf, 1, 0) {
142142
GET_NATIVE_TYPE_ARGUMENT(type_arg, arguments->NativeTypeArgAt(0));
143143
CheckSized(type_arg);
144144

145-
return Integer::New(SizeOf(type_arg, zone));
145+
return Integer::New(SizeOf(zone, type_arg));
146146
}
147147

148148
// Static invocations to this method are translated directly in streaming FGB.

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

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2773,19 +2773,19 @@ void FlowGraphCompiler::EmitNativeMove(
27732773
// The upper bits of the source are already properly sign or zero
27742774
// extended, so just copy the required amount of bits.
27752775
return EmitNativeMove(destination.WithOtherNativeType(
2776-
dst_container_type, dst_container_type, zone_),
2776+
zone_, dst_container_type, dst_container_type),
27772777
source.WithOtherNativeType(
2778-
dst_container_type, dst_container_type, zone_),
2778+
zone_, dst_container_type, dst_container_type),
27792779
temp);
27802780
}
27812781
if (src_payload_size >= dst_payload_size &&
27822782
dst_container_size > dst_payload_size) {
27832783
// The upper bits of the source are not properly sign or zero extended
27842784
// to be copied to the target, so regard the source as smaller.
27852785
return EmitNativeMove(
2786-
destination.WithOtherNativeType(dst_container_type,
2787-
dst_container_type, zone_),
2788-
source.WithOtherNativeType(dst_payload_type, dst_payload_type, zone_),
2786+
destination.WithOtherNativeType(zone_, dst_container_type,
2787+
dst_container_type),
2788+
source.WithOtherNativeType(zone_, dst_payload_type, dst_payload_type),
27892789
temp);
27902790
}
27912791
UNREACHABLE();
@@ -2800,8 +2800,8 @@ void FlowGraphCompiler::EmitNativeMove(
28002800
!destination.IsFpuRegisters()) {
28012801
// TODO(40209): If this is stack to stack, we could use FpuTMP.
28022802
// Test the impact on code size and speed.
2803-
EmitNativeMove(destination.Split(0, zone_), source.Split(0, zone_), temp);
2804-
EmitNativeMove(destination.Split(1, zone_), source.Split(1, zone_), temp);
2803+
EmitNativeMove(destination.Split(zone_, 0), source.Split(zone_, 0), temp);
2804+
EmitNativeMove(destination.Split(zone_, 1), source.Split(zone_, 1), temp);
28052805
return;
28062806
}
28072807

@@ -2829,7 +2829,7 @@ void FlowGraphCompiler::EmitNativeMove(
28292829
if (sign_or_zero_extend && destination.IsStack()) {
28302830
ASSERT(source.IsRegisters());
28312831
const auto& intermediate =
2832-
source.WithOtherNativeType(dst_payload_type, dst_container_type, zone_);
2832+
source.WithOtherNativeType(zone_, dst_payload_type, dst_container_type);
28332833
EmitNativeMove(intermediate, source, temp);
28342834
EmitNativeMove(destination, intermediate, temp);
28352835
return;
@@ -2840,7 +2840,7 @@ void FlowGraphCompiler::EmitNativeMove(
28402840
if (sign_or_zero_extend && source.IsStack()) {
28412841
ASSERT(destination.IsRegisters());
28422842
const auto& intermediate = destination.WithOtherNativeType(
2843-
src_payload_type, src_container_type, zone_);
2843+
zone_, src_payload_type, src_container_type);
28442844
EmitNativeMove(intermediate, source, temp);
28452845
EmitNativeMove(destination, intermediate, temp);
28462846
return;
@@ -2875,12 +2875,12 @@ void FlowGraphCompiler::EmitMoveToNative(
28752875
if (src_loc.IsPairLocation()) {
28762876
for (intptr_t i : {0, 1}) {
28772877
const auto& src_split = compiler::ffi::NativeLocation::FromPairLocation(
2878-
src_loc, src_type, i, zone_);
2879-
EmitNativeMove(dst.Split(i, zone_), src_split, temp);
2878+
zone_, src_loc, src_type, i);
2879+
EmitNativeMove(dst.Split(zone_, i), src_split, temp);
28802880
}
28812881
} else {
28822882
const auto& src =
2883-
compiler::ffi::NativeLocation::FromLocation(src_loc, src_type, zone_);
2883+
compiler::ffi::NativeLocation::FromLocation(zone_, src_loc, src_type);
28842884
EmitNativeMove(dst, src, temp);
28852885
}
28862886
}
@@ -2895,12 +2895,12 @@ void FlowGraphCompiler::EmitMoveFromNative(
28952895
if (dst_loc.IsPairLocation()) {
28962896
for (intptr_t i : {0, 1}) {
28972897
const auto& dest_split = compiler::ffi::NativeLocation::FromPairLocation(
2898-
dst_loc, dst_type, i, zone_);
2899-
EmitNativeMove(dest_split, src.Split(i, zone_), temp);
2898+
zone_, dst_loc, dst_type, i);
2899+
EmitNativeMove(dest_split, src.Split(zone_, i), temp);
29002900
}
29012901
} else {
29022902
const auto& dest =
2903-
compiler::ffi::NativeLocation::FromLocation(dst_loc, dst_type, zone_);
2903+
compiler::ffi::NativeLocation::FromLocation(zone_, dst_loc, dst_type);
29042904
EmitNativeMove(dest, src, temp);
29052905
}
29062906
}
@@ -2935,20 +2935,20 @@ void FlowGraphCompiler::EmitMoveConst(const compiler::ffi::NativeLocation& dst,
29352935
if (src.IsPairLocation()) {
29362936
for (intptr_t i : {0, 1}) {
29372937
const Representation src_type_split =
2938-
compiler::ffi::NativeType::FromUnboxedRepresentation(src_type,
2939-
zone_)
2940-
.Split(i, zone_)
2938+
compiler::ffi::NativeType::FromUnboxedRepresentation(zone_,
2939+
src_type)
2940+
.Split(zone_, i)
29412941
.AsRepresentation();
29422942
const auto& intermediate_native =
2943-
compiler::ffi::NativeLocation::FromLocation(intermediate,
2944-
src_type_split, zone_);
2943+
compiler::ffi::NativeLocation::FromLocation(zone_, intermediate,
2944+
src_type_split);
29452945
EmitMove(intermediate, src.AsPairLocation()->At(i), temp);
2946-
EmitNativeMove(dst.Split(i, zone_), intermediate_native, temp);
2946+
EmitNativeMove(dst.Split(zone_, i), intermediate_native, temp);
29472947
}
29482948
} else {
29492949
const auto& intermediate_native =
2950-
compiler::ffi::NativeLocation::FromLocation(intermediate, src_type,
2951-
zone_);
2950+
compiler::ffi::NativeLocation::FromLocation(zone_, intermediate,
2951+
src_type);
29522952
EmitMove(intermediate, src, temp);
29532953
EmitNativeMove(dst, intermediate_native, temp);
29542954
}

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4509,10 +4509,10 @@ void NativeParameterInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
45094509
// for the two frame pointers and two return addresses of the entry frame.
45104510
constexpr intptr_t kEntryFramePadding = 4;
45114511
compiler::ffi::FrameRebase rebase(
4512+
compiler->zone(),
45124513
/*old_base=*/SPREG, /*new_base=*/FPREG,
45134514
(-kExitLinkSlotFromEntryFp + kEntryFramePadding) *
4514-
compiler::target::kWordSize,
4515-
compiler->zone());
4515+
compiler::target::kWordSize);
45164516
const auto& src =
45174517
rebase.Rebase(marshaller_.NativeLocationOfNativeParameter(index_));
45184518
NoTemporaryAllocator no_temp;
@@ -6198,8 +6198,9 @@ void FfiCallInstr::EmitParamMoves(FlowGraphCompiler* compiler) {
61986198
const Register saved_fp = locs()->temp(0).reg();
61996199
const Register temp = locs()->temp(1).reg();
62006200

6201-
compiler::ffi::FrameRebase rebase(/*old_base=*/FPREG, /*new_base=*/saved_fp,
6202-
/*stack_delta=*/0, zone_);
6201+
compiler::ffi::FrameRebase rebase(zone_, /*old_base=*/FPREG,
6202+
/*new_base=*/saved_fp,
6203+
/*stack_delta=*/0);
62036204
for (intptr_t i = 0, n = NativeArgCount(); i < n; ++i) {
62046205
const Location origin = rebase.Rebase(locs()->in(i));
62056206
const Representation origin_rep = RequiredInputRepresentation(i);

‎runtime/vm/compiler/ffi/frame_rebase.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,24 +31,24 @@ namespace ffi {
3131
// This class can be used to rebase both Locations and NativeLocations.
3232
class FrameRebase : public ValueObject {
3333
public:
34-
FrameRebase(const Register old_base,
34+
FrameRebase(Zone* zone,
35+
const Register old_base,
3536
const Register new_base,
36-
intptr_t stack_delta_in_bytes,
37-
Zone* zone)
38-
: old_base_(old_base),
37+
intptr_t stack_delta_in_bytes)
38+
: zone_(zone),
39+
old_base_(old_base),
3940
new_base_(new_base),
40-
stack_delta_in_bytes_(stack_delta_in_bytes),
41-
zone_(zone) {}
41+
stack_delta_in_bytes_(stack_delta_in_bytes) {}
4242

4343
const NativeLocation& Rebase(const NativeLocation& loc) const;
4444

4545
Location Rebase(const Location loc) const;
4646

4747
private:
48+
Zone* zone_;
4849
const Register old_base_;
4950
const Register new_base_;
5051
const intptr_t stack_delta_in_bytes_;
51-
Zone* zone_;
5252
};
5353

5454
} // namespace ffi

‎runtime/vm/compiler/ffi/marshaller.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ Location CallMarshaller::LocInFfiCall(intptr_t arg_index) const {
7878
class CallbackArgumentTranslator : public ValueObject {
7979
public:
8080
static NativeLocations& TranslateArgumentLocations(
81-
const NativeLocations& arg_locs,
82-
Zone* zone) {
81+
Zone* zone,
82+
const NativeLocations& arg_locs) {
8383
auto& pushed_locs = *(new NativeLocations(arg_locs.length()));
8484

8585
CallbackArgumentTranslator translator;
@@ -122,10 +122,10 @@ class CallbackArgumentTranslator : public ValueObject {
122122
stack_delta += StubCodeCompiler::kNativeCallbackTrampolineStackDelta;
123123
}
124124
FrameRebase rebase(
125+
zone,
125126
/*old_base=*/SPREG, /*new_base=*/SPREG,
126127
/*stack_delta=*/(argument_slots_required_ + stack_delta) *
127-
compiler::target::kWordSize,
128-
zone);
128+
compiler::target::kWordSize);
129129
return rebase.Rebase(arg);
130130
}
131131

@@ -153,8 +153,8 @@ CallbackMarshaller::CallbackMarshaller(Zone* zone,
153153
const Function& dart_signature)
154154
: BaseMarshaller(zone, dart_signature),
155155
callback_locs_(
156-
CallbackArgumentTranslator::TranslateArgumentLocations(arg_locs_,
157-
zone_)) {}
156+
CallbackArgumentTranslator::TranslateArgumentLocations(zone_,
157+
arg_locs_)) {}
158158

159159
} // namespace ffi
160160

‎runtime/vm/compiler/ffi/native_calling_convention.cc

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ static bool SoftFpAbi() {
3131
}
3232

3333
// In Soft FP, floats are treated as 4 byte ints, and doubles as 8 byte ints.
34-
static const NativeType& ConvertIfSoftFp(const NativeType& rep, Zone* zone) {
34+
static const NativeType& ConvertIfSoftFp(Zone* zone, const NativeType& rep) {
3535
if (SoftFpAbi() && rep.IsFloat()) {
3636
ASSERT(rep.IsFloat());
3737
if (rep.SizeInBytes() == 4) {
@@ -46,25 +46,25 @@ static const NativeType& ConvertIfSoftFp(const NativeType& rep, Zone* zone) {
4646

4747
// Representations of the arguments to a C signature function.
4848
static ZoneGrowableArray<const NativeType*>& ArgumentRepresentations(
49-
const Function& signature,
50-
Zone* zone) {
49+
Zone* zone,
50+
const Function& signature) {
5151
const intptr_t num_arguments =
5252
signature.num_fixed_parameters() - kNativeParamsStartAt;
5353
auto& result = *new ZoneGrowableArray<const NativeType*>(zone, num_arguments);
5454
for (intptr_t i = 0; i < num_arguments; i++) {
5555
AbstractType& arg_type = AbstractType::Handle(
5656
zone, signature.ParameterTypeAt(i + kNativeParamsStartAt));
57-
const auto& rep = NativeType::FromAbstractType(arg_type, zone);
57+
const auto& rep = NativeType::FromAbstractType(zone, arg_type);
5858
result.Add(&rep);
5959
}
6060
return result;
6161
}
6262

6363
// Representation of the result of a C signature function.
64-
static NativeType& ResultRepresentation(const Function& signature, Zone* zone) {
64+
static NativeType& ResultRepresentation(Zone* zone, const Function& signature) {
6565
AbstractType& result_type =
6666
AbstractType::Handle(zone, signature.result_type());
67-
return NativeType::FromAbstractType(result_type, zone);
67+
return NativeType::FromAbstractType(zone, result_type);
6868
}
6969

7070
// Represents the state of a stack frame going into a call, between allocations
@@ -74,7 +74,7 @@ class ArgumentAllocator : public ValueObject {
7474
explicit ArgumentAllocator(Zone* zone) : zone_(zone) {}
7575

7676
const NativeLocation& AllocateArgument(const NativeType& payload_type) {
77-
const auto& payload_type_converted = ConvertIfSoftFp(payload_type, zone_);
77+
const auto& payload_type_converted = ConvertIfSoftFp(zone_, payload_type);
7878
if (payload_type_converted.IsFloat()) {
7979
const auto kind = FpuRegKind(payload_type);
8080
const intptr_t reg_index = FirstFreeFpuRegisterIndex(kind);
@@ -224,8 +224,8 @@ class ArgumentAllocator : public ValueObject {
224224

225225
// Location for the arguments of a C signature function.
226226
static NativeLocations& ArgumentLocations(
227-
const ZoneGrowableArray<const NativeType*>& arg_reps,
228-
Zone* zone) {
227+
Zone* zone,
228+
const ZoneGrowableArray<const NativeType*>& arg_reps) {
229229
intptr_t num_arguments = arg_reps.length();
230230
auto& result = *new NativeLocations(zone, num_arguments);
231231

@@ -239,9 +239,9 @@ static NativeLocations& ArgumentLocations(
239239
}
240240

241241
// Location for the result of a C signature function.
242-
static NativeLocation& ResultLocation(const NativeType& payload_type,
243-
Zone* zone) {
244-
const auto& payload_type_converted = ConvertIfSoftFp(payload_type, zone);
242+
static NativeLocation& ResultLocation(Zone* zone,
243+
const NativeType& payload_type) {
244+
const auto& payload_type_converted = ConvertIfSoftFp(zone, payload_type);
245245
const auto& container_type =
246246
CallingConventions::kReturnRegisterExtension == kExtendedTo4
247247
? payload_type_converted.WidenTo4Bytes(zone)
@@ -267,10 +267,11 @@ NativeCallingConvention::NativeCallingConvention(Zone* zone,
267267
const Function& c_signature)
268268
: zone_(ASSERT_NOTNULL(zone)),
269269
c_signature_(c_signature),
270-
arg_locs_(ArgumentLocations(ArgumentRepresentations(c_signature_, zone_),
271-
zone_)),
270+
arg_locs_(
271+
ArgumentLocations(zone_,
272+
ArgumentRepresentations(zone_, c_signature_))),
272273
result_loc_(
273-
ResultLocation(ResultRepresentation(c_signature_, zone_), zone_)) {}
274+
ResultLocation(zone_, ResultRepresentation(zone_, c_signature_))) {}
274275

275276
intptr_t NativeCallingConvention::num_args() const {
276277
ASSERT(c_signature_.NumOptionalParameters() == 0);

0 commit comments

Comments
 (0)