Driver: enable -force-autolink-symbol on Darwin as well
#26635
Conversation
|
@swift-ci please test |
|
CC: @rjmccall |
|
Build failed |
|
Build failed |
|
CC: @jrose-apple - bleh, seems that I cannot use |
|
Hmm, functions cannot have common linkage. I wonder what we were doing previously. |
|
I had a global int previously, I think. |
|
Yeah, the difference is that it is a function now, and that cannot be common, it has to be weak ( |
|
@swift-ci please test |
|
You could make it not a function on non-COMDAT platforms. |
|
I really would rather that we kept the behaviour the same on all the platforms. The symbol needs to be a function since you cannot do cross-module data symbol references, but you can do cross-module function references (as that can be trampolined). I believe that the symbol should be preserved here with the |
|
Build failed |
|
I don't think anyone else is doing linkonce_odr + external visibility, so that feels like a risky thing to rely on. If you want to keep the behavior the same on all platforms, we should go back to not using comdat. |
…in COFF |
|
Um, I would be really surprised if linkonce_odr+external visibility would be unused in practice. This is safe, the only concern is whether the symbol is dead stripped or not by the linker. |
This further generalises the support to Darwin where MachO is the object file format. This environment does not support COMDAT, so use `weak_odr` to ensure that the symbol is coalesced and preserved. Thanks to John McCall for reminding me of the weak odr on MachO!
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
We don't want to use linkonce_odr + default visibility because that creates a non-hidden weak symbol, which is heavily frowned upon in the Darwin world. The easiest solution is to just pick a file from the module that's required to emit this. |
|
Oh, I didn't realize that |
|
Yes, Jordan might know better what the problem with incremental builds is. Maybe it's hard to agree on which file to emit the symbol in. Adding or deleting a file to a build might cause issues with that except that they likely force recompilation of other files anyway. It's certainly better to use a single code-generation pattern, but we already can't necessarily do so because of COMDATs, so if we need target-specific workarounds, so be it. |
Yeah, that's the problem. Specifically, adding a file does not force recompilation (and we got vehement requests for that during the Swift 2 days), so you could get duplicate symbols in that one rare case. |
|
How is it valid for adding a file to not force recompilation? Existing lookups are allowed to resolve to new declarations. If we can do proper incremental negative-dependency checking with a new file, I don't see why we can't do proper negative-dependency checking that triggers a rebuild only when the primary file changes. Or was the vehement request that we not rebuild a file for a reason like that under any circumstances? |
|
We do do that. We only rebuild the files that change, then we check their dependency info to see if they could have affected other files. But "is the first file" isn't tracked in any way at the moment. |
|
Okay. So we could probably fix this that way if we had a need to, like if there was something more expensive to emit at the module level. |
|
Yeah. I think it'd even be as simple as adding a dummy "I am first" symbol to both the "provides" and "depends" sets of the first source file. That way, if anyone new "provides" it, the formerly-first file would get rebuilt, and no longer provide it itself. But that said, the idea of special-casing the first file is also a hack. COMDAT / Common linkage is actually closer to the intent of this thing, and yet another option would be some kind of bonus object file. (But that only works if swiftc is also doing the linking.) |
|
At least in the case of building with CMake (3.15+) - the linker driver is swiftc! |
|
Hmm, sounds like we should try to serialise an indicator for the primary file in the module to the swiftmodule? |
Yeah, but that's not the only interesting case. If you're in any build system that compiles and links in separate steps, you need to know there's an extra object file. (Though I guess there's already an extra object file for the wrapped module on non-Darwin.) |
5 participants
This further generalises the support to Darwin where MachO is the object
file format. This environment does not support COMDAT, so use
weak_odrto ensure that the symbol is coalesced and preserved.Thanks to John McCall for reminding me of the weak odr on MachO!
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves SR-NNNN.