Cap the alignment of all types in Swift at 16 #15691
Conversation
|
@swift-ci Please test. |
|
@stephentyrone I would like to get your opinion on this. (I can also post our old email conversation if that's okay with you.) |
|
Build failed |
|
If we're doing this then we can remove most of the recently-added unlimited alignment in |
|
Build failed |
|
How does this interact with imported types that require higher alignment, like the vector types from simd?
|
|
Riskily. If you passed a pointer to a Swift-allocated object of such a type to a C API, that API might miscompile. We could decline to import such APIs, or import such pointers as opaque; I don't think we'd want to go so far as to decline to import the type at all. |
|
(This PR is meant to be more of a discussion than something we're going to merge right away.) |
|
It'd be nice if we can get away with this. If not, maybe we can cap alignment only during unspecialized use, but still use the proper alignment in specialized code. We can only ever call C code that demands higher alignment at the ABI level from specialized contexts, so we could maintain C compatibility in most situations. We could still record the ideal full alignment in the runtime metadata somewhere so that things like |
|
@mikeash you are correct about the malloc assumptions. Linux (including android) and Windows have 8-byte alignment on 32-bit and 16-byte alignment on 64-bit IIRC. |
|
As far as pure-Swift operation is concerned, this all seems fine to me. I'll discuss the importer issues with John offline and see if we can get a little bit more control over this. |
rjmccall
force-pushed
the
max-alignment-16
branch
from
657cd26
to
f325310
Compare
|
@jckarter Interesting idea. The idea of honoring alignment on imported types but capping it generically is definitely appealing, and you're right, it wouldn't be hard to make generic value witnesses assume a weaker alignment bound than the type normally guarantees. We'd also have to apply that to potentially-abstracted function parameters in general, I think. So the big questions are (1) whether you can otherwise exfiltrate a pointer from generic code and (2) which alignment we use for things like struct layout, which of course can be generic. It does look like we do prevent you from expressing a However, there are other ways of getting a pointer from generic code, like by passing a local inout to a callback which then passes that as a pointer to C. I guess we could do writeback in such a situation. But you can also just pass the callback a pointer, which we're then stuck about. |
|
I want to explore the idea of just importing pointers to types with a >16-byte alignment as opaque pointers. It looks like this shouldn't give us much trouble; the majority of vector types in APIs are passed directly, and the pointers-to-vectors that I've found so far (e.g. in ARKit) are pointers to simd_float3 or simd_float2, which are at most 16-byte-aligned. |
|
Also, @stephentyrone reminded me that clang does not provide a consistent ABI for larger vectors anyway — it assigns them a different alignment based on the current compiler flags. So it would be quite dangerous for an API to traffic in pointers to 32-byte vectors. (This is obviously a clang bug, but fixing it means admitting that we can't use 32-byte-aligned loads and stores for those types because of historical compatibility requirements, which might be noticeably sub-optimal on more aggressive CPU targets.) |
rjmccall
force-pushed
the
max-alignment-16
branch
from
f325310
to
283fff9
Compare
|
@swift-ci Please test. |
|
Build failed |
|
Build failed |
|
Okay, so this is perhaps obvious in retrospect, but we can't just completely refuse to import higher-aligned types into Swift because simd declares a number of such types. Unless we can change simd to cap the alignment of those types, of course. |
|
Hmm, this is a bit of a weird case, which may be a bug that we should fix. These simd types only have 16 byte alignment in C unless you're compiling with -mavx2 or -mavx512xxx. I think that the right fix here is to relax the Swift and C alignment to always be 16B, and possibly introduce a set of explicitly higher-aligned C types that cannot be imported. If the x86_64h slice were more widely used, we probably wouldn't be able to do this without breaking anyone, but I think we can still make this change at present. |
|
By your comments about Clang above, does any C code in the OS rely in practice on those simd types having higher alignment? |
|
"Probably not"; you only get it when compiling with -mavx2 or similar, and we own most of the code that does that, and tend to do explicit alignment. Clang actually changed it's behavior with these a few years ago and ~nothing broke, so at least then it worked. It will need some investigation, though. |
|
It looks like the build actually broke on the armv7s build: (which is the terrible diagnostic you get when Swift refuses to import a type) |
|
Apparently clang aligns 32B vectors to 32B on armv7s, but not on arm64 or i386 or x86_64. That's a bug (but one that we can safely fix; there's no aligned load/store on armv7 that requires more than 4 byte alignment). |
|
Well, it would be an ABI break for e.g. structs containing such fields. Do we think that's okay? |
|
It's an ABI break that clang made for all the other targets 2 or 3 years ago without anyone noticing. =/ (The only thing worse than an ABI break is an ABI break unevenly applied?) |
|
@bob-wilson @fredriss Thoughts on whether it's okay to change the alignment of 32-byte vectors on armv7? I wouldn't want to do that without consulting you first. |
|
If this is only for swift, I don't really have an objection. Why do you think I should care? |
|
@fredriss The question John is asking would be for C. Originally ext_vectors had alignment equal to their size. At some point (~3 years ago?) an open-source clang changed the default alignment to min(16B, legal_vector_size) for all targets except armv7. That was an ABI break, but it didn't effect anything visibly so no one noticed it until long after it had shipped in Xcode. So now a type like simd_double4 is 16B aligned on arm64, i386, x86_64, but 32B aligned on armv7 and x86 with -mavx. Public library boundaries can't assume 32B alignment on x86 because we don't know what flags the caller was compiled with, so that's not a huge problem (there's a chance of breaking someone who assumes higher alignment internally, but we won't break APIs), but we don't have that on armv7. Fortunately, there are no armv7 load/store instructions that require high alignment, so the chance of introducing actual bugs is pretty small, but it's something we need to be careful about. |
|
I've been thinking the same thing. |
|
SGTM. |
|
Hmm. What should I do with the x86 AVX intrinsics and their typedefs? Intel's documentation says that |
|
Posted a Clang patch: https://reviews.llvm.org/D46042 |
rjmccall
force-pushed
the
max-alignment-16
branch
from
283fff9
to
27f04c9
Compare
rjmccall
force-pushed
the
max-alignment-16
branch
from
27f04c9
to
354db19
Compare
|
@swift-ci Please test. |
|
The plan right now is that we are going to apply this rule unconditionally in Swift. Whether we make it in the Darwin C ABI is a separable question. It's fine if some corner-case C type — like a wide vector type or a struct with a large explicit alignment — doesn't import well into Swift, as long as this doesn't impact any of the vector-centric APIs we want to expose. |
|
Build failed |
rjmccall
force-pushed
the
max-alignment-16
branch
from
354db19
to
14d54fc
Compare
|
@swift-ci Please test. |
|
@swift-ci Please test source compatibility. |
|
Build failed |
|
Build failed |
rdar://31411216
rjmccall
force-pushed
the
max-alignment-16
branch
from
14d54fc
to
c1f99b5
Compare
|
@swift-ci Please test. |
1 similar comment
|
@swift-ci Please test. |
|
Build failed |
|
@swift-ci Please test Linux. |
|
Fake failure that should be fixed by #19029. |
|
Build failed |
|
@swift-ci Please test source compatibility. |
|
One project in the source-compatibility suite timed out; gonna ignore it. |
|
🤘🏻 |
This is the result of some (fairly old) conversations with @stephentyrone. High-alignment types generate a variety of problems for the implementation, and meanwhile the architectural arc seems to be towards de-emphasizing the need for really high alignments just to make larger vectors performant.
rdar://31411216