From e7acc69507cdd95d8c69051981182c1412126be8 Mon Sep 17 00:00:00 2001 From: Daco Harkes Date: Fri, 19 Mar 2021 15:34:42 +0000 Subject: [PATCH] [vm/ffi] Support packed `Struct`s backend - part 2 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: https://github.com/dart-lang/sdk/issues/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 --- runtime/vm/compiler/ffi/native_type.cc | 31 +++++++----- runtime/vm/compiler/ffi/native_type.h | 8 +-- runtime/vm/compiler/ffi/native_type_test.cc | 55 +++++++++++++++++++++ 3 files changed, 79 insertions(+), 15 deletions(-) diff --git a/runtime/vm/compiler/ffi/native_type.cc b/runtime/vm/compiler/ffi/native_type.cc index 19a2ace217a7..0f939594e52f 100644 --- a/runtime/vm/compiler/ffi/native_type.cc +++ b/runtime/vm/compiler/ffi/native_type.cc @@ -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; } } diff --git a/runtime/vm/compiler/ffi/native_type.h b/runtime/vm/compiler/ffi/native_type.h index 8fbcbc5bfb43..e7a156eb1ee8 100644 --- a/runtime/vm/compiler/ffi/native_type.h +++ b/runtime/vm/compiler/ffi/native_type.h @@ -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. @@ -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; @@ -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; @@ -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. // diff --git a/runtime/vm/compiler/ffi/native_type_test.cc b/runtime/vm/compiler/ffi/native_type_test.cc index d6d5ece42c95..63cc7b412b4e 100644 --- a/runtime/vm/compiler/ffi/native_type_test.cc +++ b/runtime/vm/compiler/ffi/native_type_test.cc @@ -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