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

Switch pc-windows-gnu targets from GCC to Clang #92502

Closed

Conversation

dennisameling
Copy link

@dennisameling dennisameling commented Jan 2, 2022

My main goal is to add a new target for aarch64-pc-windows-gnu. However, GCC doesn't support Windows arm64 so we'll first need to switch to Clang. Luckily, the MSYS2 team has been providing clang64 builds of Rust since a few months.

This PR changes the toolchain for *-pc-windows-gnu from GCC to Clang. This uses the great work that the MSYS2 team has done to get Rust to build on Clang for Windows GNU targets.

The challenge is that the bootstrap compiler still uses GCC to build the target, so if I understand things correctly, we'll need to have a temporary situation in which things like libgcc_eh.a and libgcc_s.a are available until a new version of the x86_64-pc-windows-gnu toolchain has been published. Since those dependencies are not available in Clang, I applied the same hacks that the MSYS2 team did to get things to work. I'll be more than happy to create a follow-up PR to get rid of those hacks once a new version of the CI/bootstrap compiler has been published.

I was able to build a full distribution locally with MSYS2 clang64 - artifacts can be found here: https://github.com/dennisameling/rust/releases/tag/windows-gnu-clang

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 2, 2022
@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 2, 2022
cp "/clang64/lib/libunwind.a" "${GCC_LIBS_HACK}/libgcc_eh.a"
cp "/clang64/lib/libunwind.dll.a" "${GCC_LIBS_HACK}/libgcc_s.a"
export RUSTFLAGS_BOOTSTRAP="-C link-arg=-L$(cygpath -am missing-libs-hack)"
pacman -S --noconfirm --needed mingw-w64-clang-$arch-toolchain mingw-w64-clang-$arch-cmake \
Copy link
Author

@dennisameling dennisameling Jan 2, 2022

Choose a reason for hiding this comment

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

Note the change from mingw-w64 to mingw-w64-clang packages here - these are the key enablers for Clang support. I'm not sure how your custom mingw toolchain should be updated so I switched CI to CUSTOM_MINGW: 0 for now

@@ -177,7 +177,7 @@ impl Step for Llvm {
.define("LLVM_ENABLE_ASSERTIONS", assertions)
.define("LLVM_ENABLE_PLUGINS", plugins)
.define("LLVM_TARGETS_TO_BUILD", llvm_targets)
.define("LLVM_EXPERIMENTAL_TARGETS_TO_BUILD", llvm_exp_targets)
// .define("LLVM_EXPERIMENTAL_TARGETS_TO_BUILD", llvm_exp_targets)
Copy link
Author

Choose a reason for hiding this comment

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

This was causing the LLVM build to fail due to duplicate symbols - I'm not sure how to fix that so I've disabled the experimental targets for now

[2451/2844] Building CXX object tools/llvm-config/CMakeFiles/llvm-config.dir/llvm-config.cpp.obj
[2452/2844] Linking CXX executable bin\llvm-config.exe
[2453/2844] Linking CXX executable bin\llvm-ar.exe
[2454/2844] Generating ../../bin/llvm-ranlib.exe
[2455/2844] Generating ../../bin/llvm-lib.exe
[2456/2844] Generating ../../bin/llvm-dlltool.exe
[2457/2844] Building CXX object tools/lto/CMakeFiles/LTO.dir/lto.cpp.obj
[2458/2844] Building CXX object tools/bugpoint/CMakeFiles/bugpoint.dir/FindBugs.cpp.obj
[2459/2844] Building CXX object tools/llvm-lto/CMakeFiles/llvm-lto.dir/llvm-lto.cpp.obj
[2460/2844] Building CXX object tools/bugpoint/CMakeFiles/bugpoint.dir/Miscompilation.cpp.obj
[2461/2844] Building CXX object tools/bugpoint/CMakeFiles/bugpoint.dir/ExtractFunction.cpp.obj
[2462/2844] Building CXX object tools/bugpoint/CMakeFiles/bugpoint.dir/CrashDebugger.cpp.obj
[2463/2844] Building Options.inc...
[2464/2844] Building CXX object tools/bugpoint/CMakeFiles/bugpoint.dir/bugpoint.cpp.obj
[2465/2844] Building CXX object tools/bugpoint/CMakeFiles/bugpoint.dir/BugDriver.cpp.obj
[2466/2844] Building CXX object tools/bugpoint/CMakeFiles/bugpoint.dir/OptimizerDriver.cpp.obj
[2467/2844] Building CXX object tools/bugpoint/CMakeFiles/bugpoint.dir/ToolRunner.cpp.obj
[2468/2844] Building CXX object tools/llvm-profdata/CMakeFiles/llvm-profdata.dir/llvm-profdata.cpp.obj
[2469/2844] Linking CXX executable bin\llvm-profdata.exe
[2470/2844] Building CXX object tools/dsymutil/CMakeFiles/dsymutil.dir/CFBundle.cpp.obj
[2471/2844] Building CXX object tools/bugpoint/CMakeFiles/bugpoint.dir/ExecutionDriver.cpp.obj
[2472/2844] Linking CXX executable bin\bugpoint.exe
FAILED: bin/bugpoint.exe
cmd.exe /C "cd . && C:\msys64\clang64\bin\g++.exe -ffunction-sections -fdata-sections -m64 -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -ffunction-sections -fdata-sections -O3 -DNDEBUG -Wl,--stack,16777216 @CMakeFiles\bugpoint.rsp -o bin\bugpoint.exe -Wl,--out-implib,lib\libbugpoint.dll.a -Wl,--major-image-version,0,--minor-image-version,0  && cd ."
ld.lld: error: duplicate symbol: vtable for llvm::FormalArgHandler
>>> defined at libLLVMPowerPCCodeGen.a(PPCCallLowering.cpp.obj)
>>> defined at libLLVMM68kCodeGen.a(M68kCallLowering.cpp.obj)
g++: error: linker command failed with exit code 1 (use -v to see invocation)
[2473/2844] Building CXX object tools/lli/CMakeFiles/lli.dir/ExecutionUtils.cpp.obj
[2474/2844] Building CXX object tools/dsymutil/CMakeFiles/dsymutil.dir/BinaryHolder.cpp.obj
[2475/2844] Building CXX object tools/dsymutil/CMakeFiles/dsymutil.dir/MachODebugMapParser.cpp.obj
[2476/2844] Building CXX object tools/llc/CMakeFiles/llc.dir/llc.cpp.obj
[2477/2844] Building CXX object tools/dsymutil/CMakeFiles/dsymutil.dir/dsymutil.cpp.obj
[2478/2844] Building CXX object tools/lli/CMakeFiles/lli.dir/lli.cpp.obj
[2479/2844] Building CXX object lib/Passes/CMakeFiles/LLVMPasses.dir/PassBuilder.cpp.obj
ninja: build stopped: subcommand failed.
thread 'main' panicked at '
command did not execute successfully, got: exit code: 1

build script failed, must exit now', C:\Users\Dennis\.cargo\registry\src\github.com-1ecc6299db9ec823\cmake-0.1.44\src\lib.rs:885:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
        finished in 1092.867 seconds
Build completed unsuccessfully in 0:18:14

@bjorn3
Copy link
Member

bjorn3 commented Jan 3, 2022

I applied the same hacks that the MSYS2 team did to get things to work. I'll be more than happy to create a follow-up PR to get rid of those hacks once a new version of the CI/bootstrap compiler has been published.

Would it be possible to use #[cfg(bootstrap)] and #[cfg(not(bootstrap))] in the compiler instead? #[cfg(bootstrap)] is only enabled when compiling using the bootstrap compiler and #[cfg(not(bootstrap))] is only enabled when not compiling using the bootstrap compiler. Using no_default_libraries: false may cause some crates to accidentally start depending on it, in which case it is going to be hard to enable no_default_libraries again.

@mati865
Copy link
Contributor

mati865 commented Jan 3, 2022

Unfortunately this breaks a lot of things:

  • GCC is still the most popular compiler for mingw-w64 based toolchains, it cannot be changed like that
  • it's no longer possible to use GCC after this change:
    • GCC+mingw-w64 toolchains do not provide libunwind
    • if adds libunwind themselves using GCC will still link libgcc_s* so they end up with with executables linked to both libunwind and libgcc (assuming it doesn't fail)
  • ld.bfd still doesn't properly support import libraries created by LLVM (or MSVC)
  • more things I cannot think of right now

@dennisameling
Copy link
Author

dennisameling commented Jan 3, 2022

Unfortunately this breaks a lot of things

Ok, sounds like it's a bit too early for this PR then. Thanks for the context! 👍🏼

@mati865
Copy link
Contributor

mati865 commented Jan 3, 2022

I'm not sure if windows-gnu targets can start using LLVM based mingw-w64 toolchains ever.
Creating new targets is the easiest (possibly the only non-breaking) way to achieve that.

@dennisameling
Copy link
Author

Makes sense. Do you have suggestions for a target name? x86_64-pc-windows-clang maybe?

@mati865
Copy link
Contributor

mati865 commented Jan 3, 2022

TBH I had an idea how to call it I'd have already added it.
LLVM can target both MSVC and mignw-w64 so anything like *-pc-windows-clang or *-pc-windows-llvm is out of consideration.

@mati865
Copy link
Contributor

mati865 commented Mar 16, 2022

There is PR adding new targets: #94872

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants