Repository navigation
Use a portable popcount so MSVC can build the LLVM backend - #31
Merged
Merged
Conversation
__builtin_popcountll is a GCC/Clang builtin with no MSVC equivalent, so an MSVC build of the LLVM backend fails outright: abi_policy.cpp(15): error C3861: '__builtin_popcountll': identifier not found CI does not catch this because no job builds that combination. The windows-latest MSVC job configures without DOLRECOMP_ENABLE_LLVM, so this file is never compiled; the "LLVM backend (windows-latest)" job does enable it but builds under MSYS2/UCRT64 with mingw GCC, which has the builtin. MSVC plus the LLVM backend is the configuration a local Windows toolchain actually uses. std::bitset<64>::count() is C++17, needs no feature test, and lowers to popcnt on every compiler that has the instruction. This was the only __builtin_ in src/.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
__builtin_popcountllis a GCC/Clang builtin with no MSVC equivalent, so an MSVC build of the LLVM backend fails outright onmain:Why CI does not catch it
No job builds that combination:
build (windows-latest, Release, cl, cl)src/backend/llvm/abi_policy.cppis never compiledLLVM backend (windows-latest)MSVC plus the LLVM backend is the configuration a local Windows toolchain actually uses (
vcvars64+ an LLVM release build), and it is the one combination CI never exercises.The change
std::bitset<64>::count()is C++17, needs no feature test or#ifdef, and lowers topopcnton every compiler that has the instruction. This was the only__builtin_insrc/.Verification
Built and tested on three hosts, all with
-DDOLRECOMP_ENABLE_LLVM=ON:The three macOS failures (
modern_codegen,llvm_codegen,llvm_pipeline) are pre-existing and unrelated to this change — I confirmed by reverting only this hunk on the same tree and re-running serially, which produced the identical failure set. They are also not what blocks the macOS build; that is the Mach-O section specifier issue fixed by #26.Note for anyone reproducing the macOS numbers: run
ctest -j1. Under-j10those tests contend over shared artifacts in the build directory and report an inconsistent subset of failures between runs.Suggested follow-up
Adding
-DDOLRECOMP_ENABLE_LLVM=ONto the MSVC CI job would close the gap that let this land.