bootstrap: "download-ci-llvm = true" doesn't work on x86_64-pc-windows-gnu with lld

[petrochenkov]

You get an error like this:

   Compiling rustc_driver v0.0.0 (C:\msys64\home\we\rust\compiler\rustc_driver)
error: linking with `x86_64-w64-mingw32-gcc` failed: exit code: 1
  |
...
  = note: ld.lld: error: duplicate symbol: vtable for llvm::FormalArgHandler
          >>> defined at librustc_llvm-a81737dd65a7c126.rlib(M68kCallLowering.cpp.obj)
          >>> defined at librustc_llvm-a81737dd65a7c126.rlib(PPCCallLowering.cpp.obj)
          collect2.exe: error: ld returned 1 exit status

I didn't investigate it further.

It never worked in the past and I assumed it's due to the old mingw version on CI, but it still doesn't work after #100178 and the actual error doesn't look like it's caused by a toolchain version.

[mati865]

It happens only with LLD but i haven't investigated.
At MSYS2 we had to remove M68k to maskę it build with LLD but since 1.67 we no longer got this issue: https://github.com/msys2/MINGW-packages/pull/15256/files#diff-9d9b9deaf0bc40ce778f7bbeae62dd4b0c7a3269b7cf080ab80acfb8b5d561d9L8

[jyn514]

cc #39915

do either of you know what LLD is doing differently compared to LD? is this something rustc controls? It would help to have a minimization smaller than "all of rustc and llvm" ...

[mati865]

I don't know the answer to any of those questions but this issue stopped happening at MSYS2 so it could have been some LLVM 16 change (MSYS2 uses LLVM 15 but backports important patches from LLVM 16) that fixed this problem there.

[jyn514]

Looks like we're currently using LLVM 15: https://github.com/rust-lang/llvm-project/tree/rustc/15.0-2022-12-07

@rustbot label +S-blocked

[mati865]

Yup, I plan to try once #107224 is merged.

[petrochenkov]

This is still an issue with LLVM 17.0.5.

[petrochenkov]

There's an ODR violation in M68k's FormalArgHandler, its base class M68kIncomingValueHandler doesn't have a virtual method markPhysRegUsed while IncomingValueHandler for all other targets including PPC do.

So it's not a linker's problem, ld actually complains about this too, it just reports it as a warning instead of an error.

The simplest fix is to rename the structure (instead of adding virtual methods).

diff --git a/llvm/lib/Target/M68k/GISel/M68kCallLowering.cpp b/llvm/lib/Target/M68k/GISel/M68kCallLowering.cpp
index b0ada29d1cea..93b250656cd2 100644
--- a/llvm/lib/Target/M68k/GISel/M68kCallLowering.cpp
+++ b/llvm/lib/Target/M68k/GISel/M68kCallLowering.cpp
@@ -118,7 +118,7 @@ bool M68kCallLowering::lowerFormalArguments(MachineIRBuilder &MIRBuilder,
   CCAssignFn *AssignFn =
       TLI.getCCAssignFn(F.getCallingConv(), false, F.isVarArg());
   IncomingValueAssigner ArgAssigner(AssignFn);
-  FormalArgHandler ArgHandler(MIRBuilder, MRI);
+  M68kFormalArgHandler ArgHandler(MIRBuilder, MRI);
   return determineAndHandleAssignments(ArgHandler, ArgAssigner, SplitArgs,
                                        MIRBuilder, F.getCallingConv(),
                                        F.isVarArg());
diff --git a/llvm/lib/Target/M68k/GISel/M68kCallLowering.h b/llvm/lib/Target/M68k/GISel/M68kCallLowering.h
index a1589e96aa3d..5e05e46f4ac4 100644
--- a/llvm/lib/Target/M68k/GISel/M68kCallLowering.h
+++ b/llvm/lib/Target/M68k/GISel/M68kCallLowering.h
@@ -63,8 +63,8 @@ private:
                            ISD::ArgFlagsTy Flags) override;
 };

-struct FormalArgHandler : public M68kIncomingValueHandler {
-  FormalArgHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI)
+struct M68kFormalArgHandler : public M68kIncomingValueHandler {
+  M68kFormalArgHandler(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI)
       : M68kIncomingValueHandler(MIRBuilder, MRI) {}
 };

I wish I tried to debug this a few years eariler.

[petrochenkov]

Could someone submit this to LLVM?
I'm not familiar with their processes.
cc @nikic

[mati865]

You can simply open pull request to https://github.com/llvm/llvm-project/
Otherwise I can do it this or next weekend.

[petrochenkov]

Opened a pull request with the fix - llvm/llvm-project#72797

[petrochenkov]

Fixed in #119159.