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

Add support for clang-cl. #950

Merged
merged 1 commit into from
Jul 3, 2024
Merged

Conversation

yangbowen
Copy link
Contributor

Previously clang-cl would not be detected as MSVC despite using MSVC command line syntax. This PR adds such support.

In addition, with this PR, on Clang ThinLTO will be used, which hopefully offers some performance benefit.
It appears that clang-cl would ignore the MSVC option for LTO /GL & /LTCG. ThinLTO will also be used on clang-cl.

Additionally, x86_sse option is modified to support SSE4.1, SSE4.2, AVX, AVX2, AVX512.
SSE2 is not unavailable on x86-64. Instead it's always available so compilers enable SSE2 support on x86-64 implicitly.

Various refactorings are made to realize the above changes while keeping it cleanly organized.

@LBPHacker
Copy link
Member

I'll go through this a bit later more thoroughly, but yeah, as I think even comments in meson.build say, these sorts of upgrades are much needed and appreciated.

One thing I'm not certain about right away is your removal of SSE explicitly configured but unavailable in msvc targeting 64-bit machines. I'm very much aware that x86-64 mandates at least SSE2, but I vaguely remember the compiler not accepting that switch (read: throwing warnings about it) for the same reason.

@LBPHacker LBPHacker added the Enhancement implemented but could be better label May 24, 2024
@yangbowen
Copy link
Contributor Author

yangbowen commented May 25, 2024

Well, I tweaked the part where x86_sse_level is translated into compiler flags, so that while the level is 20, the /arch flag will only be added when supported :)

elif x86_sse_level >= 20 and host_machine.cpu_family() == 'x86'

Perhaps comment should be added that specifically explains this, in case it's not clear?

@yangbowen
Copy link
Contributor Author

The rationale was that, x86_sse_level would always reflect whatever instruction set that's in use, instead of what compiler flags to specify (since the latter needs separate treatment anyway)

@LBPHacker
Copy link
Member

Yep, lgtm, I'll sort out a few problems we have with some pipelines so we don't immediately start looking at this PR when eventually something fails horribly (our ghactions setup doesn't run all jobs all the time, only the ones we consider more important, and the less important ones are only run when releasing something that users will see).

@LBPHacker
Copy link
Member

I've tested this with the clang-cl I have access to and it works fine, though I did have to disable resources because my clang-cl package didn't come with a resource compiler. I considered adding a pipeline that tests clang-cl builds but I'll put that off for now so as to not delay merging this further.

@LBPHacker LBPHacker merged commit 24628db into The-Powder-Toy:master Jul 3, 2024
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement implemented but could be better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants