Skip to content

[compiler-rt] Don't link builtins against the CRT on Windows #70675

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

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

DavidTruby
Copy link
Member

compiler-rt/builtins doesn't depend on anything from the CRT but
currently links against it and embeds a /defaultlib:msvcrt in the
.lib file, forcing anyone linking against it to also link against that
specific CRT. This isn't necessary as the end user can just choose which
CRT they want to use independently.

Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks mostly reasonable to me, just a couple minor points. (But it might be good to have a few extra opinions.)

@@ -761,6 +761,12 @@ else ()

append_list_if(COMPILER_RT_HAS_STD_C11_FLAG -std=c11 BUILTIN_CFLAGS)

# Don't link the the CRT on Windows
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to clarify the terminology here a bit; the builtins is a static library, so we're not linking anything anyway. Perhaps # Don't embed directives for picking any specific CRT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change in my rebase before merging

@mstorsjo mstorsjo requested review from rnk, zmodem and strega-nil October 30, 2023 21:53
Copy link
Collaborator

@zmodem zmodem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me, thanks!

compiler-rt/builtins doesn't depend on anything from the CRT but
currently links against it and embeds a `/defaultlib:msvcrt` in the
`.lib` file, forcing anyone linking against it to also link against that
specific CRT. This isn't necessary as the end user can just choose which
CRT they want to use independently.
@DavidTruby DavidTruby merged commit dbb4f90 into llvm:main Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants