Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[embedded] Emit weak_odr instead of linkonce_odr symbols under -Xfrontend -mergeable-symbols #79892

Merged
merged 3 commits into from
Mar 20, 2025

Conversation

kubamracek
Copy link
Contributor

We added a -Xfrontend -mergeable-symbols flag in #79242, see the description for the motivation behind that. We used LinkOnceODRLinkage for such mergeable symbols, but those are viewed as "removable if unused" by LLVM, which makes e.g. the entrypoint ("main") get removed from object files. Let's instead use WeakODRLinkage.

@kubamracek
Copy link
Contributor Author

@swift-ci please test

@compnerd
Copy link
Member

I think that this might be a problem for PE/COFF - there is no concept of a weak symbol. The way to handle this is to mark the symbol as link once odr and add a llvm.used for it to treat is as a GC root.

@rjmccall
Copy link
Contributor

rjmccall commented Mar 11, 2025

The distinction between weak_odr and linkonce_odr is purely a compile-time property; it shouldn't vary based on target. We do need to use a COMDAT on ELF and PE/COFF, though, if we want the linker to actually merge symbols.

@kubamracek
Copy link
Contributor Author

ApplyIRLinkage already seems to apply a COMDAT for both weak_odr and linkonce_odr?

      if (IRL.Linkage == llvm::GlobalValue::LinkOnceODRLinkage ||
          IRL.Linkage == llvm::GlobalValue::WeakODRLinkage)
        if (Triple.supportsCOMDAT())
          if (llvm::GlobalObject *GO = dyn_cast<llvm::GlobalObject>(GV))
            GO->setComdat(M->getOrInsertComdat(GV->getName()));

@kubamracek
Copy link
Contributor Author

@swift-ci please test

@kubamracek
Copy link
Contributor Author

@swift-ci please test

@kubamracek
Copy link
Contributor Author

Given this is all under a hidden flag and meant for experimentations/explorations, I'm going to merge; will figure out about how to support PE/COFF if this behavior turns out to be something we want to actually use in practice for Embedded Swift.

@kubamracek kubamracek merged commit 605f1c9 into swiftlang:main Mar 20, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants