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

clang Use llvm-ar linker when LTO flag is set. #1554

Merged
merged 1 commit into from
Nov 9, 2020

Conversation

JoelLinn
Copy link
Contributor

@JoelLinn JoelLinn commented Nov 4, 2020

What does this PR do?

Changes the static linker from ar to llvm-ar when the clang toolset is used and the LinkTimeOptimization flag is set.
Although many times GNU ar will also work, for some clang features like -flto (link-time optimisation, I only came across this one but could be more) llvm-ar is needed.

How does this PR change Premake's behavior?

It should not break or change build-file output for existing projects (that may not have llvm-ar installed) since llvm-ar is only conditionally used and still defaults to ar.

Anything else we should know?

Before this change, it was not possible to use the LinkTimeOptimization flag with the clang toolset unless using the "AR=llvm-ar" environment variable.

Did you check all the boxes?

  • Focus on a single fix or feature; remove any unrelated formatting or code changes
  • Add unit tests showing fix or feature works; all tests pass
  • Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • Follow our coding conventions
  • Minimize the number of commits

You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!

It is required for some clang features like `-flto`. Still defaults to `ar` if the `LinkTimeOptimization` flag is not set.
@samsinsane
Copy link
Member

Please fill in the template, it's there for a reason.

@samsinsane samsinsane closed this Nov 8, 2020
@JoelLinn
Copy link
Contributor Author

JoelLinn commented Nov 8, 2020

Please fill in the template, it's there for a reason.

please reopen

@samsinsane samsinsane reopened this Nov 8, 2020
@tritao
Copy link
Contributor

tritao commented Nov 8, 2020

It seems a bit inconsistent that the tool will be different depending on if LTO is enabled or not.

Is there any reason not to just always use llvm-ar when using the clang toolset?

@JoelLinn
Copy link
Contributor Author

JoelLinn commented Nov 8, 2020

Is there any reason not to just always use llvm-ar when using the clang toolset?

  • It may break existing projects since usually llvm packages are not required by clang-packages. (llvm-ar is not installed)
  • Afaik (if that counts anything here) cmake also uses plain ar first in its makefiles unless certain features are enabled, only then llvm-ar, llvm-ranlib and friends are emitted. I never looked into when exactly they switched, just noticed it in the makefiles

@tritao
Copy link
Contributor

tritao commented Nov 8, 2020

Makes sense, thanks.

@samsinsane samsinsane merged commit f51aad1 into premake:master Nov 9, 2020
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.

3 participants