Fix swift_bridgeObjectRetain() dropping integer bits

[nickolas-pohilets]

In particular this was breaking deferred type-checking of arrays.

Regression was introduced in 724a9a7

[mikeash]

Which bits are these?

[nickolas-pohilets]

This flag in _BridgeStorage:

swift/stdlib/public/core/BridgeStorage.swift

Line 49 in 3c9f1eb

flag ? (1 as UInt) << _objectPointerLowSpareBitShift : 0)

Set e.g. here:

swift/stdlib/public/core/ArrayBuffer.swift

Line 76 in 90e3d1c

storage: _ArrayBridgeStorage(native: _native._storage, isFlagged: true))

And later read here:

swift/stdlib/public/core/Array.swift

Line 755 in 7088022

let wasNativeTypeChecked = _hoistableIsNativeTypeChecked()

As a result of this bug, deferred type checking was forgotten when copying Array in some cases.

let arr = ["abc" as NSString, "xyz" as NSString]

// native.storesOnlyElementsOfType(TargetElement.self) returns false
// constructs array with deferred type checking
let x = arr as! [@convention(block) () -> Void]
_ = x[0] // triggers assertion, as expected

let y = x // calls `swift_bridgeObjectRetain()`, but does not use it's result
_ = y[0] // also triggers assertion

var iter = x.makeIterator() // calls `swift_bridgeObjectRetain()` and stores result inside the iterator
_ = iter.next() // does not trigger assertion, returning `NSString` bit-casted to block.

[nickolas-pohilets]

@swift-ci Please smoke test

[mikeash]

Got it, thanks for the explanation. I'm worried that making swift_retain not be a tail call will hurt performance. I'll kick off benchmarks to see. If it is a problem, I think we can make a new swift_retainWithSpareBits or something like that which accepts a pointer with extra bits set and preserves them in the return value.

[mikeash]

@swift-ci please Apple Silicon benchmark

[nickolas-pohilets]

We can still make a tail call if objectRef == object. But I'm not sure if tail call is worth extra conditional jump.

New function (swift_retainWithSpareBits) won't help much, because IIUC returning object is part of the contract of the swift_bridgeObjectRetain().

We could ensure tail calls for swift_retain if this contract is changed. E.g. swift_bridgeObjectRetain returns void. Then recovering object becomes responsibility of the caller to swift_bridgeObjectRetain. But that would be an ABI-breaking change.

[mikeash]

The new function would take and return object without needing a stack frame on the fast path. The implementation would look just like your new swift_retain call, but with inlining of swift_retain it would only need an extra register for the value rather than pushing a stack frame. If we could inline swift_retain into swift_bridgeObjectRetain then that would do it too.

[mikeash]

Thinking a little more, we may be able to just make swift_retain itself do the masking and return the original value. I expect one additional and and one additional register to be approximately free.

[mikeash]

I'm working on putting the fast paths of retain/release into a library that can be linked into users, which improves performance. I tried adding this fix to my changes there. I made it so swift_retain accepts a pointer with extra bits set, and returns it with the same bits set. Seems to work pretty well. That's here if you're interested: https://github.com/swiftlang/swift/pull/84159/files#diff-00cf1af423a03af9669aba702c716cbe22c01074ee3c0e5f04aed95d8f1eb0fc

[mikeash]

I just put up a proper PR with my changes here: #85044

Although it's focused on providing optimized retain/release implementations that can be linked into clients, it does also fix the runtime's own swift_bridgeObjectRetain to preserve the extra bits. The new static library is off by default while we work out some build system issues, but that fix will apply regardless.