Skip to content

Commit

Permalink
[vm/ffi] Support packed Structs backend - part 2
Browse files Browse the repository at this point in the history
With packed structs, the x64 non-Windows ABI only put structs in CPU/FPU
registers when all it fields happen to be aligned. The previous CL did
not check the alignment of nested structs and structs in inline arrays
based on their offset in an outer struct.

Follow up of: https://dart-review.googlesource.com/c/sdk/+/186142.
Split off: https://dart-review.googlesource.com/c/sdk/+/186143.

Bug: #38158

tools/build.py run_ffi_unit_tests && tools/test.py ffi_unit
TEST=runtime/vm/compiler/ffi/native_type_test.cc
These tests are exercised on vm-precomp-ffi-qemu-linux-release-arm-try.

Change-Id: Ic64bdef2c13ac737dbf58864911f043fc7a3d831
Cq-Include-Trybots: luci.dart.try:vm-precomp-ffi-qemu-linux-release-arm-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/191720
Reviewed-by: Aske Simon Christensen <askesc@google.com>
  • Loading branch information
dcharkes committed Mar 19, 2021
1 parent 6d1ea68 commit e7acc69
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 15 deletions.
31 changes: 20 additions & 11 deletions runtime/vm/compiler/ffi/native_type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -736,23 +736,32 @@ intptr_t NativeCompoundType::NumberOfWordSizeChunksNotOnlyFloat() const {
}
#endif // !defined(DART_PRECOMPILED_RUNTIME)

bool NativePrimitiveType::ContainsUnalignedMembers() const {
bool NativePrimitiveType::ContainsUnalignedMembers(intptr_t offset) const {
return offset % AlignmentInBytesField() != 0;
}

bool NativeArrayType::ContainsUnalignedMembers(intptr_t offset) const {
const intptr_t element_size = element_type_.SizeInBytes();
// We're only checking the first two elements of the array.
//
// If the element size is divisible by the alignment of the largest type
// contained within the element type, the alignment of all elements is the
// same. If not, either the first or the second element is unaligned.
const intptr_t max_check = 2;
for (intptr_t i = 0; i < Utils::Minimum(length_, max_check); i++) {
const intptr_t element_offset = i * element_size;
if (element_type_.ContainsUnalignedMembers(offset + element_offset)) {
return true;
}
}
return false;
}

bool NativeArrayType::ContainsUnalignedMembers() const {
return element_type_.ContainsUnalignedMembers();
}

bool NativeCompoundType::ContainsUnalignedMembers() const {
bool NativeCompoundType::ContainsUnalignedMembers(intptr_t offset) const {
for (intptr_t i = 0; i < members_.length(); i++) {
const auto& member = *members_.At(i);
const intptr_t member_offset = member_offsets_.At(i);
const intptr_t member_alignment = member.AlignmentInBytesField();
if (member_offset % member_alignment != 0) {
return true;
}
if (member.ContainsUnalignedMembers()) {
if (member.ContainsUnalignedMembers(offset + member_offset)) {
return true;
}
}
Expand Down
8 changes: 4 additions & 4 deletions runtime/vm/compiler/ffi/native_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class NativeType : public ZoneAllocated {
#endif // !defined(DART_PRECOMPILED_RUNTIME)

// True iff any members are misaligned recursively due to packing.
virtual bool ContainsUnalignedMembers() const = 0;
virtual bool ContainsUnalignedMembers(intptr_t offset = 0) const = 0;

#if !defined(DART_PRECOMPILED_RUNTIME) && !defined(FFI_UNIT_TESTS)
// NativeTypes which are available as unboxed Representations.
Expand Down Expand Up @@ -188,7 +188,7 @@ class NativePrimitiveType : public NativeType {
virtual bool ContainsOnlyFloats(Range range) const;
#endif // !defined(DART_PRECOMPILED_RUNTIME)

virtual bool ContainsUnalignedMembers() const;
virtual bool ContainsUnalignedMembers(intptr_t offset = 0) const;

#if !defined(DART_PRECOMPILED_RUNTIME) && !defined(FFI_UNIT_TESTS)
virtual bool IsExpressibleAsRepresentation() const;
Expand Down Expand Up @@ -239,7 +239,7 @@ class NativeArrayType : public NativeType {
virtual bool ContainsOnlyFloats(Range range) const;
#endif // !defined(DART_PRECOMPILED_RUNTIME)

virtual bool ContainsUnalignedMembers() const;
virtual bool ContainsUnalignedMembers(intptr_t offset = 0) const;

virtual bool Equals(const NativeType& other) const;

Expand Down Expand Up @@ -298,7 +298,7 @@ class NativeCompoundType : public NativeType {
intptr_t NumberOfWordSizeChunksNotOnlyFloat() const;
#endif // !defined(DART_PRECOMPILED_RUNTIME)

virtual bool ContainsUnalignedMembers() const;
virtual bool ContainsUnalignedMembers(intptr_t offset = 0) const;

// Whether this type has only same-size floating point members.
//
Expand Down
55 changes: 55 additions & 0 deletions runtime/vm/compiler/ffi/native_type_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,61 @@ UNIT_TEST_CASE_WITH_ZONE(NativeCompoundType_packed) {
EXPECT(struct_type.ContainsUnalignedMembers());
}

UNIT_TEST_CASE_WITH_ZONE(NativeCompoundType_packed_array) {
const auto& uint8_type = *new (Z) NativePrimitiveType(kUint8);
const auto& uint16_type = *new (Z) NativePrimitiveType(kUint16);

auto& inner_members = *new (Z) NativeTypes(Z, 2);
inner_members.Add(&uint16_type);
inner_members.Add(&uint8_type);
const intptr_t packing = 1;
const auto& inner_struct_type =
NativeCompoundType::FromNativeTypes(Z, inner_members, packing);

EXPECT_EQ(3, inner_struct_type.SizeInBytes());
// Non-windows x64 considers this struct as all members aligned, even though
// its size is not a multiple of its individual member alignment.
EXPECT(!inner_struct_type.ContainsUnalignedMembers());

const auto& array_type = *new (Z) NativeArrayType(inner_struct_type, 2);

auto& members = *new (Z) NativeTypes(Z, 1);
members.Add(&array_type);
const auto& struct_type = NativeCompoundType::FromNativeTypes(Z, members);

EXPECT_EQ(6, struct_type.SizeInBytes());
// Non-windows x64 passes this as a struct with unaligned members, because
// the second element of the array contains unaligned members.
EXPECT(struct_type.ContainsUnalignedMembers());
}

UNIT_TEST_CASE_WITH_ZONE(NativeCompoundType_packed_nested) {
const auto& uint8_type = *new (Z) NativePrimitiveType(kUint8);
const auto& uint32_type = *new (Z) NativePrimitiveType(kUint32);

auto& inner_members = *new (Z) NativeTypes(Z, 2);
inner_members.Add(&uint32_type);
inner_members.Add(&uint8_type);
const intptr_t packing = 1;
const auto& inner_struct_type =
NativeCompoundType::FromNativeTypes(Z, inner_members, packing);

EXPECT_EQ(5, inner_struct_type.SizeInBytes());
// Non-windows x64 considers this struct as all members aligned, even though
// its size is not a multiple of its individual member alignment.
EXPECT(!inner_struct_type.ContainsUnalignedMembers());

auto& members = *new (Z) NativeTypes(Z, 2);
members.Add(&uint8_type);
members.Add(&inner_struct_type);
const auto& struct_type = NativeCompoundType::FromNativeTypes(Z, members);

EXPECT_EQ(6, struct_type.SizeInBytes());
// Non-windows x64 passes this as a struct with unaligned members, even
// though the nested struct itself has all its members aligned in isolation.
EXPECT(struct_type.ContainsUnalignedMembers());
}

} // namespace ffi
} // namespace compiler
} // namespace dart

0 comments on commit e7acc69

Please sign in to comment.