Reordering autolink-extract runtime orderings #65375
Conversation
|
@swift-ci please test |
| "-lswiftSwiftOnoneSupport", | ||
| "-lswiftCore", | ||
| "-lswift_Concurrency", | ||
| "-lswift_StringProcessing", | ||
| "-lswift_RegexBuilder", | ||
| "-lswift_RegexParser", | ||
| "-lswift_Backtracing", |
There was a problem hiding this comment.
I think that these should be sunk lower? Right before swiftCore.
| "-lswiftGlibc", | ||
| "-lBlocksRuntime", | ||
| // Dispatch-specific Swift runtime libs | ||
| "-lswift_Concurrency", |
There was a problem hiding this comment.
Doesn't this have a dependency on swiftCore?
Best quote of 2023. We can go home now. |
etcwilde
force-pushed
the
ewilde/reorder-autolinks
branch
from
470a4bc to
a2f3443
Compare
|
@swift-ci please test |
etcwilde
force-pushed
the
ewilde/reorder-autolinks
branch
from
a2f3443 to
d59af2e
Compare
Now let's just hope we don't have too many cycles. |
|
@swift-ci please test |
| @@ -247,35 +247,31 @@ int autolink_extract_main(ArrayRef<const char *> Args, const char *Argv0, | |||
| // in most object files | |||
|
|
|||
| std::vector<std::string> SwiftRuntimeLibsOrdered = { | |||
There was a problem hiding this comment.
Would this benefit from a code comment for posterity that clarifies that ordering matters here and why it's ordered the way it is?
There was a problem hiding this comment.
There should probably also be a note that it's safe to list all libraries in this list regardless of whether any given link operation actually uses any particular library or whether the library exists in the "current" toolchain (the list only controls ordering and deduplication).
|
swiftlang/swift-integration-tests#78 @swift-ci build toolchain |
| "-lswift_StringProcessing", | ||
| "-lswift_RegexBuilder", | ||
| "-lswift_RegexParser", | ||
| "-lswift_Backtracing", |
There was a problem hiding this comment.
Just FWIW, _Backtracing is going to depend on _StringProcessing on Linux. (It's using a regex to parse some things from /proc.)
|
Can this be closed given that we don't have tests that confirm that changing ordering fixes the issue we were chasing and we've merged alternative fixes since then? |
|
We may still want to fix the ordering since, even though the group makes it so that ordering “doesn’t matter”, it’s basically a while-loop around the linker algorithm until things converge. Putting things in the right order will help them converge faster while consuming less memory. It’s basically an optimization now. |
Order is important. The ordering generated by autolink-extract is nonsensical. I haven't had a chance to actually build the full symbol dependency graph so I'm definitely missing something, but this gets us closer to the actual library link order, so it might unblock some folks who are running into missing symbols.
etcwilde
force-pushed
the
ewilde/reorder-autolinks
branch
from
d59af2e to
0075695
Compare
| "-lFoundation", | ||
| "-lFoundationNetworking", | ||
| "-lFoundationXML", | ||
| // Foundation support libs | ||
| "-lcurl", | ||
| "-lxml2", | ||
| "-luuid", | ||
| // XCTest runtime libs (must be first due to http://github.com/apple/swift-corelibs-xctest/issues/432) |
There was a problem hiding this comment.
Might as well also fix my silly comment typo from way back in my original PR while we're at it:
| // XCTest runtime libs (must be first due to http://github.com/apple/swift-corelibs-xctest/issues/432) | |
| // XCTest runtime libs (must come after runtime libs due to https://github.com/apple/swift-corelibs-xctest/issues/432) |
FWIW, if the group had "fully" helped with ordering, I wouldn't have run into the XCTest issue that made me throw up my hands and add ordering to the deduplication pass in the first place, so a good choice of ordering is definitely a good thing. (Fixing the XCTest issue would also be good! 😆) It was my impression that #65795 solved the static linking issue; does that include the Radars mentioned by this PR? |
Order is important. The ordering generated by autolink-extract is nonsensical in main and 5.9. I haven't had a chance to actually build the full symbol dependency graph to generate the proper ordering so I expect that I'm missing something, but this gets us closer to the actual library link order, so it might hopefully unblock some folks who are running into the "missing" symbols.
rdar://107643395
rdar://107445812