Hi, there, i am having a trouble with using KeyPath on s390x platform. Here is the example:
func generic<A: Equatable, B: Equatable>(a: A, b: B) {
typealias T = (A, B)
let kp_t_1 = \T.1
let tuple = (a, b)
let k1 = tuple[keyPath: kp_t_1]
print(k1)
}
generic(a: 2, b: 31)
It supposes to print 31, however it prints 2 instead, which is obviously wrong. I have managed to find a hack fix in the source code:
diff --git a/stdlib/public/core/KeyPath.swift b/stdlib/public/core/KeyPath.swift
index 599757bce3..7159d78e37 100644
--- a/stdlib/public/core/KeyPath.swift
+++ b/stdlib/public/core/KeyPath.swift
@@ -3134,14 +3134,24 @@ internal struct InstantiateKeyPathBuffer : KeyPathPatternVisitor {
// Look up offset in the type metadata. The value in the pattern is
// the offset within the metadata object.
let metadataPtr = unsafeBitCast(base, to: UnsafeRawPointer.self)
let offset: UInt32
switch kind {
case .class:
offset = UInt32(metadataPtr.load(fromByteOffset: Int(offsetOfOffset),
as: UInt.self))
case .struct:
offset = UInt32(metadataPtr.load(fromByteOffset: Int(offsetOfOffset),
- as: UInt32.self))
+ as: UInt.self))
}
The idea is to load the value at metadataPtr+offsetOfOffset as 8 bytes (UInt) wide rather than 4 bytes (UInt32) wide. After checking lib/IRGen/GenKeyPath.swift, it doesn’t seem there is a 64-bit offset stored anywhere. Does anyone have any idea where the root cause might be?
This is very similar to a PR proposed on an older version of swift fixed test case KeyPath.swift for s390x by samding01 · Pull Request #12796 · apple/swift · GitHub. But the difference is for the newer master branch, metadataPtr.load(fromByteOffset: Int(offsetOfOffset), as: UInt.self) doesn't always hold. Hi, @Joe_Groff, I notice that you helped with that PR, do you mind having a quick look at this one and sharing some ideas?
It doesn't seem to fix the issue on my system. I have been using the following two test cases:
import StdlibUnittest
func generic<A: Equatable, B: Equatable>(a: A, b: B) {
typealias T = (A, B)
let kp_t_1 = \T.1
let tuple = (a, b)
expectEqual(tuple[keyPath: kp_t_1], tuple.1)
}
generic(a: "Tuples Hello", b: 31)
import StdlibUnittest
struct Container<T, U> {
let x: (T, U)
var y: (a: T, b: U)
}
func generic<A: Equatable, B: Equatable>(a: A, b: B) {
let kp_c_y_a = \Container<A, B>.y.a
let container = Container(x: (a, b), y: (a, b))
expectEqual(container[keyPath: kp_c_y_a], container.y.0)
}
generic(a: "Tuples Hello", b: 31)
And both are failing with segmentation fault after applying the patch.
I see; I forgot that the compiler also emits direct offsets into tuple metadata to project, not only key paths. For ABI compatibility on Apple platforms, the field would have to remain zero-padded for the full 64 bits. On s390x, you have a few more options. For tuple key paths, as a short term solution, you might for instance be able to add four to the offset that the compiler emits so that it loads the correct 32 bits during key path instantiation. A more principled solution would be to take my patch and also change loadTupleOffsetFromMetadata to do a 32-bit load as well; if you want to go further, it would be nice to also reconfigure TupleTypeMetadata on platforms which aren't yet ABI-constrained so that it stores element types and offsets in separate arrays, so that the 32-bit offsets can be densely packed and save some memory.
To repeat, changing KeyPath.swift in that way is not correct; struct metadata does in fact store an array of UInt32 offsets, and that will do the wrong thing if a struct field is at an offset of zero. The first solution I was suggesting was to change GenKeyPath.cpp on the compiler side to emit the tuple metadata offset +4 so that it loads the least significant 32 bits.
For the second solution, you will need the change from my linked PR to make the field in TargetTupleMetadata::Element be a uint32_t. On the compiler side, you would also need to change IRGenModule::TupleTypeMetadataPtrTy to have an i32 field for .Offset so that the result of CreateInBoundsGEP is an i32* instead of an i64*.