Skip to content

Commit bad5cc2

Browse files
dcharkescommit-bot@chromium.org
dcharkes
authored and
[email protected]
committed
[vm/ffi] Support packed Structs backend
With packed structs, the x64 non-Windows ABI only put structs in CPU/FPU registers when all it fields happen to be aligned. This CL introduces the `NativeType::ContainsUnalignedMembers` query for calling convention calculations. Bug: #38158 tools/build.py run_ffi_unit_tests && tools/test.py ffi_unit TEST=runtime/vm/compiler/ffi/native_calling_convention_test.cc TEST=runtime/vm/compiler/ffi/native_type_test.cc These tests are exercised on vm-precomp-ffi-qemu-linux-release-arm-try. Change-Id: I504f67ae22a6af375d5eb57ff26b58d340099df8 Cq-Include-Trybots: luci.dart.try:vm-precomp-ffi-qemu-linux-release-arm-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/186142 Commit-Queue: Daco Harkes <[email protected]> Reviewed-by: Clement Skau <[email protected]>
1 parent 9a74bce commit bad5cc2

33 files changed

+390
-9
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ class ArgumentAllocator : public ValueObject {
141141
const NativeLocation& AllocateCompound(
142142
const NativeCompoundType& payload_type) {
143143
const intptr_t size = payload_type.SizeInBytes();
144-
if (size <= 16 && size > 0) {
144+
if (size <= 16 && size > 0 && !payload_type.ContainsUnalignedMembers()) {
145145
intptr_t required_regs =
146146
payload_type.NumberOfWordSizeChunksNotOnlyFloat();
147147
intptr_t required_xmm_regs =
@@ -516,7 +516,7 @@ static const NativeLocation& CompoundResultLocation(
516516
Zone* zone,
517517
const NativeCompoundType& payload_type) {
518518
const intptr_t size = payload_type.SizeInBytes();
519-
if (size <= 16 && size > 0) {
519+
if (size <= 16 && size > 0 && !payload_type.ContainsUnalignedMembers()) {
520520
// Allocate the same as argument, but use return registers instead of
521521
// argument registers.
522522
NativeLocations& multiple_locations =

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,77 @@ UNIT_TEST_CASE_WITH_ZONE(NativeCallingConvention_struct8bytesx1) {
398398
RunSignatureTest(Z, "struct8bytesx1", arguments, struct_type);
399399
}
400400

401+
// The struct is only 8 bytes with packing enabled.
402+
//
403+
// Many calling conventions pass this struct in single registers or less
404+
// stack slots because of this.
405+
//
406+
// Non-windows x64 passes this struct on the stack instead of in a single
407+
// CPU register, because it contains a mis-aligned member.
408+
//
409+
// See the *.expect in ./unit_tests for this behavior.
410+
UNIT_TEST_CASE_WITH_ZONE(NativeCallingConvention_struct8bytesPackedx10) {
411+
const auto& int8_type = *new (Z) NativePrimitiveType(kInt8);
412+
const auto& int32_type = *new (Z) NativePrimitiveType(kInt32);
413+
414+
auto& member_types = *new (Z) NativeTypes(Z, 5);
415+
member_types.Add(&int8_type);
416+
member_types.Add(&int32_type);
417+
member_types.Add(&int8_type);
418+
member_types.Add(&int8_type);
419+
member_types.Add(&int8_type);
420+
const auto& struct_type =
421+
NativeCompoundType::FromNativeTypes(Z, member_types, /*packing=*/1);
422+
EXPECT_EQ(8, struct_type.SizeInBytes());
423+
EXPECT(struct_type.ContainsUnalignedMembers());
424+
425+
auto& arguments = *new (Z) NativeTypes(Z, 10);
426+
arguments.Add(&struct_type);
427+
arguments.Add(&struct_type);
428+
arguments.Add(&struct_type);
429+
arguments.Add(&struct_type);
430+
arguments.Add(&struct_type);
431+
arguments.Add(&struct_type);
432+
arguments.Add(&struct_type);
433+
arguments.Add(&struct_type);
434+
arguments.Add(&struct_type);
435+
arguments.Add(&struct_type);
436+
437+
RunSignatureTest(Z, "struct8bytesPackedx10", arguments, struct_type);
438+
}
439+
440+
// Without packing, this would be a 16 byte struct. However, because of packing
441+
// it's 9 bytes.
442+
//
443+
// #pragma pack(push,1)
444+
// typedef struct {
445+
// int8_t a0;
446+
// double a1
447+
// } StructPacked;
448+
// #pragma pack(pop)
449+
//
450+
// See the *.expect in ./unit_tests for this behavior.
451+
UNIT_TEST_CASE_WITH_ZONE(NativeCallingConvention_structPacked) {
452+
const auto& int8_type = *new (Z) NativePrimitiveType(kInt8);
453+
const auto& double_type = *new (Z) NativePrimitiveType(kDouble);
454+
455+
auto& member_types = *new (Z) NativeTypes(Z, 2);
456+
member_types.Add(&int8_type);
457+
member_types.Add(&double_type);
458+
const auto& struct_type =
459+
NativeCompoundType::FromNativeTypes(Z, member_types, /*packing=*/1);
460+
EXPECT_EQ(9, struct_type.SizeInBytes());
461+
EXPECT(struct_type.ContainsUnalignedMembers());
462+
463+
auto& arguments = *new (Z) NativeTypes(Z, 11);
464+
arguments.Add(&struct_type);
465+
arguments.Add(&struct_type);
466+
arguments.Add(&int8_type); // Backfilling int registers.
467+
arguments.Add(&double_type); // Backfilling float registers.
468+
469+
RunSignatureTest(Z, "structPacked", arguments, struct_type);
470+
}
471+
401472
} // namespace ffi
402473
} // namespace compiler
403474
} // namespace dart

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

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@ static bool ContainsHomogenuousFloatsInternal(const NativeTypes& types);
164164
// pkg/vm/lib/transformations/ffi_definitions.dart:_calculateStructLayout.
165165
NativeCompoundType& NativeCompoundType::FromNativeTypes(
166166
Zone* zone,
167-
const NativeTypes& members) {
167+
const NativeTypes& members,
168+
intptr_t member_packing) {
168169
intptr_t offset = 0;
169170

170171
const intptr_t kAtLeast1ByteAligned = 1;
@@ -195,8 +196,13 @@ NativeCompoundType& NativeCompoundType::FromNativeTypes(
195196
for (intptr_t i = 0; i < members.length(); i++) {
196197
const NativeType& member = *members[i];
197198
const intptr_t member_size = member.SizeInBytes();
198-
const intptr_t member_align_field = member.AlignmentInBytesField();
199-
const intptr_t member_align_stack = member.AlignmentInBytesStack();
199+
const intptr_t member_align_field =
200+
Utils::Minimum(member.AlignmentInBytesField(), member_packing);
201+
intptr_t member_align_stack = member.AlignmentInBytesStack();
202+
if (member_align_stack > member_packing &&
203+
member_packing < compiler::target::kWordSize) {
204+
member_align_stack = compiler::target::kWordSize;
205+
}
200206
offset = Utils::RoundUp(offset, member_align_field);
201207
member_offsets.Add(offset);
202208
offset += member_size;
@@ -710,6 +716,29 @@ intptr_t NativeCompoundType::NumberOfWordSizeChunksNotOnlyFloat() const {
710716
}
711717
#endif // !defined(DART_PRECOMPILED_RUNTIME)
712718

719+
bool NativePrimitiveType::ContainsUnalignedMembers() const {
720+
return false;
721+
}
722+
723+
bool NativeArrayType::ContainsUnalignedMembers() const {
724+
return element_type_.ContainsUnalignedMembers();
725+
}
726+
727+
bool NativeCompoundType::ContainsUnalignedMembers() const {
728+
for (intptr_t i = 0; i < members_.length(); i++) {
729+
const auto& member = *members_.At(i);
730+
const intptr_t member_offset = member_offsets_.At(i);
731+
const intptr_t member_alignment = member.AlignmentInBytesField();
732+
if (member_offset % member_alignment != 0) {
733+
return true;
734+
}
735+
if (member.ContainsUnalignedMembers()) {
736+
return true;
737+
}
738+
}
739+
return false;
740+
}
741+
713742
static void ContainsHomogenuousFloatsRecursive(const NativeTypes& types,
714743
bool* only_float,
715744
bool* only_double) {

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ class NativeType : public ZoneAllocated {
9797
virtual bool ContainsOnlyFloats(Range range) const = 0;
9898
#endif // !defined(DART_PRECOMPILED_RUNTIME)
9999

100+
// True iff any members are misaligned recursively due to packing.
101+
virtual bool ContainsUnalignedMembers() const = 0;
102+
100103
#if !defined(DART_PRECOMPILED_RUNTIME) && !defined(FFI_UNIT_TESTS)
101104
// NativeTypes which are available as unboxed Representations.
102105
virtual bool IsExpressibleAsRepresentation() const { return false; }
@@ -185,6 +188,8 @@ class NativePrimitiveType : public NativeType {
185188
virtual bool ContainsOnlyFloats(Range range) const;
186189
#endif // !defined(DART_PRECOMPILED_RUNTIME)
187190

191+
virtual bool ContainsUnalignedMembers() const;
192+
188193
#if !defined(DART_PRECOMPILED_RUNTIME) && !defined(FFI_UNIT_TESTS)
189194
virtual bool IsExpressibleAsRepresentation() const;
190195
virtual Representation AsRepresentation() const;
@@ -234,6 +239,8 @@ class NativeArrayType : public NativeType {
234239
virtual bool ContainsOnlyFloats(Range range) const;
235240
#endif // !defined(DART_PRECOMPILED_RUNTIME)
236241

242+
virtual bool ContainsUnalignedMembers() const;
243+
237244
virtual bool Equals(const NativeType& other) const;
238245

239246
virtual void PrintTo(BaseTextBuffer* f,
@@ -255,8 +262,10 @@ using NativeTypes = ZoneGrowableArray<const NativeType*>;
255262
// TODO(dartbug.com/38491): Support unions.
256263
class NativeCompoundType : public NativeType {
257264
public:
258-
static NativeCompoundType& FromNativeTypes(Zone* zone,
259-
const NativeTypes& members);
265+
static NativeCompoundType& FromNativeTypes(
266+
Zone* zone,
267+
const NativeTypes& members,
268+
intptr_t member_packing = kMaxInt32);
260269

261270
const NativeTypes& members() const { return members_; }
262271
const ZoneGrowableArray<intptr_t>& member_offsets() const {
@@ -289,6 +298,8 @@ class NativeCompoundType : public NativeType {
289298
intptr_t NumberOfWordSizeChunksNotOnlyFloat() const;
290299
#endif // !defined(DART_PRECOMPILED_RUNTIME)
291300

301+
virtual bool ContainsUnalignedMembers() const;
302+
292303
// Whether this type has only same-size floating point members.
293304
//
294305
// Useful for determining whether struct is passed in FP registers in hardfp

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@ namespace ffi {
1414

1515
const NativeCompoundType& RunStructTest(dart::Zone* zone,
1616
const char* name,
17-
const NativeTypes& member_types) {
17+
const NativeTypes& member_types,
18+
intptr_t packing = kMaxInt32) {
1819
const auto& struct_type =
19-
NativeCompoundType::FromNativeTypes(zone, member_types);
20+
NativeCompoundType::FromNativeTypes(zone, member_types, packing);
2021

2122
const char* test_result = struct_type.ToCString(zone, /*multi_line=*/true);
2223

@@ -196,6 +197,23 @@ UNIT_TEST_CASE_WITH_ZONE(NativeCompoundType_floatarray) {
196197
struct_type.NumberOfWordSizeChunksOnlyFloat());
197198
}
198199

200+
UNIT_TEST_CASE_WITH_ZONE(NativeCompoundType_packed) {
201+
const auto& uint8_type = *new (Z) NativePrimitiveType(kUint8);
202+
const auto& uint16_type = *new (Z) NativePrimitiveType(kUint16);
203+
204+
auto& members = *new (Z) NativeTypes(Z, 2);
205+
members.Add(&uint8_type);
206+
members.Add(&uint16_type);
207+
const intptr_t packing = 1;
208+
209+
const auto& struct_type =
210+
NativeCompoundType::FromNativeTypes(Z, members, packing);
211+
212+
// Should be 3 bytes on every platform.
213+
EXPECT_EQ(3, struct_type.SizeInBytes());
214+
EXPECT(struct_type.ContainsUnalignedMembers());
215+
}
216+
199217
} // namespace ffi
200218
} // namespace compiler
201219
} // namespace dart

‎runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/arm64_android.expect

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
M(r0 int64) Compound(size: 8)
2+
M(r1 int64) Compound(size: 8)
3+
M(r2 int64) Compound(size: 8)
4+
M(r3 int64) Compound(size: 8)
5+
M(r4 int64) Compound(size: 8)
6+
M(r5 int64) Compound(size: 8)
7+
M(r6 int64) Compound(size: 8)
8+
M(r7 int64) Compound(size: 8)
9+
S+0 Compound(size: 8)
10+
S+8 Compound(size: 8)
11+
=>
12+
M(r0 int64) Compound(size: 8)

‎runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/arm64_ios.expect

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
M(r0 int64) Compound(size: 8)
2+
M(r1 int64) Compound(size: 8)
3+
M(r2 int64) Compound(size: 8)
4+
M(r3 int64) Compound(size: 8)
5+
M(r4 int64) Compound(size: 8)
6+
M(r5 int64) Compound(size: 8)
7+
M(r6 int64) Compound(size: 8)
8+
M(r7 int64) Compound(size: 8)
9+
S+0 Compound(size: 8)
10+
S+8 Compound(size: 8)
11+
=>
12+
M(r0 int64) Compound(size: 8)

‎runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/arm64_linux.expect

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
M(r0 int64) Compound(size: 8)
2+
M(r1 int64) Compound(size: 8)
3+
M(r2 int64) Compound(size: 8)
4+
M(r3 int64) Compound(size: 8)
5+
M(r4 int64) Compound(size: 8)
6+
M(r5 int64) Compound(size: 8)
7+
M(r6 int64) Compound(size: 8)
8+
M(r7 int64) Compound(size: 8)
9+
S+0 Compound(size: 8)
10+
S+8 Compound(size: 8)
11+
=>
12+
M(r0 int64) Compound(size: 8)

‎runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/arm64_macos.expect

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
M(r0 int64) Compound(size: 8)
2+
M(r1 int64) Compound(size: 8)
3+
M(r2 int64) Compound(size: 8)
4+
M(r3 int64) Compound(size: 8)
5+
M(r4 int64) Compound(size: 8)
6+
M(r5 int64) Compound(size: 8)
7+
M(r6 int64) Compound(size: 8)
8+
M(r7 int64) Compound(size: 8)
9+
S+0 Compound(size: 8)
10+
S+8 Compound(size: 8)
11+
=>
12+
M(r0 int64) Compound(size: 8)

‎runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/arm_android.expect

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
M(r1 int32, r2 int32) Compound(size: 8)
2+
M(r3 int32, S+0 int32) Compound(size: 8)
3+
M(S+4 int32, S+8 int32) Compound(size: 8)
4+
M(S+12 int32, S+16 int32) Compound(size: 8)
5+
M(S+20 int32, S+24 int32) Compound(size: 8)
6+
M(S+28 int32, S+32 int32) Compound(size: 8)
7+
M(S+36 int32, S+40 int32) Compound(size: 8)
8+
M(S+44 int32, S+48 int32) Compound(size: 8)
9+
M(S+52 int32, S+56 int32) Compound(size: 8)
10+
M(S+60 int32, S+64 int32) Compound(size: 8)
11+
=>
12+
P(r0 uint32) Compound(size: 8)

‎runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/arm_ios.expect

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
M(r1 int32, r2 int32) Compound(size: 8)
2+
M(r3 int32, S+0 int32) Compound(size: 8)
3+
M(S+4 int32, S+8 int32) Compound(size: 8)
4+
M(S+12 int32, S+16 int32) Compound(size: 8)
5+
M(S+20 int32, S+24 int32) Compound(size: 8)
6+
M(S+28 int32, S+32 int32) Compound(size: 8)
7+
M(S+36 int32, S+40 int32) Compound(size: 8)
8+
M(S+44 int32, S+48 int32) Compound(size: 8)
9+
M(S+52 int32, S+56 int32) Compound(size: 8)
10+
M(S+60 int32, S+64 int32) Compound(size: 8)
11+
=>
12+
P(r0 uint32) Compound(size: 8)

‎runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/arm_linux.expect

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
M(r1 int32, r2 int32) Compound(size: 8)
2+
M(r3 int32, S+0 int32) Compound(size: 8)
3+
M(S+4 int32, S+8 int32) Compound(size: 8)
4+
M(S+12 int32, S+16 int32) Compound(size: 8)
5+
M(S+20 int32, S+24 int32) Compound(size: 8)
6+
M(S+28 int32, S+32 int32) Compound(size: 8)
7+
M(S+36 int32, S+40 int32) Compound(size: 8)
8+
M(S+44 int32, S+48 int32) Compound(size: 8)
9+
M(S+52 int32, S+56 int32) Compound(size: 8)
10+
M(S+60 int32, S+64 int32) Compound(size: 8)
11+
=>
12+
P(r0 uint32) Compound(size: 8)

‎runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/ia32_android.expect

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
S+4 Compound(size: 8)
2+
S+12 Compound(size: 8)
3+
S+20 Compound(size: 8)
4+
S+28 Compound(size: 8)
5+
S+36 Compound(size: 8)
6+
S+44 Compound(size: 8)
7+
S+52 Compound(size: 8)
8+
S+60 Compound(size: 8)
9+
S+68 Compound(size: 8)
10+
S+76 Compound(size: 8)
11+
=>
12+
P(S+0 uint32, ret:eax uint32) Compound(size: 8)

‎runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/ia32_linux.expect

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
S+4 Compound(size: 8)
2+
S+12 Compound(size: 8)
3+
S+20 Compound(size: 8)
4+
S+28 Compound(size: 8)
5+
S+36 Compound(size: 8)
6+
S+44 Compound(size: 8)
7+
S+52 Compound(size: 8)
8+
S+60 Compound(size: 8)
9+
S+68 Compound(size: 8)
10+
S+76 Compound(size: 8)
11+
=>
12+
P(S+0 uint32, ret:eax uint32) Compound(size: 8)

‎runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/ia32_win.expect

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
S+0 Compound(size: 8)
2+
S+8 Compound(size: 8)
3+
S+16 Compound(size: 8)
4+
S+24 Compound(size: 8)
5+
S+32 Compound(size: 8)
6+
S+40 Compound(size: 8)
7+
S+48 Compound(size: 8)
8+
S+56 Compound(size: 8)
9+
S+64 Compound(size: 8)
10+
S+72 Compound(size: 8)
11+
=>
12+
M(eax uint32, edx uint32) Compound(size: 8)

‎runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/x64_ios.expect

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
S+0 Compound(size: 8)
2+
S+8 Compound(size: 8)
3+
S+16 Compound(size: 8)
4+
S+24 Compound(size: 8)
5+
S+32 Compound(size: 8)
6+
S+40 Compound(size: 8)
7+
S+48 Compound(size: 8)
8+
S+56 Compound(size: 8)
9+
S+64 Compound(size: 8)
10+
S+72 Compound(size: 8)
11+
=>
12+
P(rdi int64, ret:rax int64) Compound(size: 8)

‎runtime/vm/compiler/ffi/unit_tests/struct8bytesPackedx10/x64_linux.expect

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
S+0 Compound(size: 8)
2+
S+8 Compound(size: 8)
3+
S+16 Compound(size: 8)
4+
S+24 Compound(size: 8)
5+
S+32 Compound(size: 8)
6+
S+40 Compound(size: 8)
7+
S+48 Compound(size: 8)
8+
S+56 Compound(size: 8)
9+
S+64 Compound(size: 8)
10+
S+72 Compound(size: 8)
11+
=>
12+
P(rdi int64, ret:rax int64) Compound(size: 8)

0 commit comments

Comments
 (0)