[M68k] Fix ODR violation in GISel code

[petrochenkov]

It prevents LLVM from being linked with LLD at least on Windows, with errors like this:

  = 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)

Binutils linker also complains about this, but only with warnings.

FormalArgHandler has a base class M68kIncomingValueHandler which doesn't have a virtual method markPhysRegUsed like IncomingValueHandlers for all other targets including PPC, so it results in a conflict.
The simplest fix is to rename the FormalArgHandler structure (rather than to add virtual methods for compatibility).

cc rust-lang/rust#107668

[llvmbot]

@llvm/pr-subscribers-backend-m68k

Author: Vadim Petrochenkov (petrochenkov)

Changes

It prevents LLVM from being linked with LLD at least on Windows, with errors like this:

  = 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)

Binutils linker also complains about this, but only with warnings.

FormalArgHandler has a base class M68kIncomingValueHandler which doesn't have a virtual method markPhysRegUsed like IncomingValueHandlers for all other targets including PPC, so it results in a conflict.
The simplest fix is to rename the FormalArgHandler structure (rather than to add virtual methods for compatibility).

cc rust-lang/rust#107668

---

Full diff: https://github.com/llvm/llvm-project/pull/72797.diff

2 Files Affected:

• (modified) llvm/lib/Target/M68k/GISel/M68kCallLowering.cpp (+1-1)
• (modified) llvm/lib/Target/M68k/GISel/M68kCallLowering.h (+2-2)

diff --git a/llvm/lib/Target/M68k/GISel/M68kCallLowering.cpp b/llvm/lib/Target/M68k/GISel/M68kCallLowering.cpp
index bb63516e957fd5e..8aa8a48c12d501c 100644
--- a/llvm/lib/Target/M68k/GISel/M68kCallLowering.cpp
+++ b/llvm/lib/Target/M68k/GISel/M68kCallLowering.cpp
@@ -119,7 +119,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 7644e6cffbb1bc1..82b5cee9e03bcc3 100644
--- a/llvm/lib/Target/M68k/GISel/M68kCallLowering.h
+++ b/llvm/lib/Target/M68k/GISel/M68kCallLowering.h
@@ -64,8 +64,8 @@ struct M68kIncomingValueHandler : public CallLowering::IncomingValueHandler {
                            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) {}
 };
 

[0x59616e]

Defining FormatArgHandler in the anonymous namespace might be better, but still LGTM.

Just wondering why doesn't this happen on linux ?

[petrochenkov]

Perhaps linker implementations are more permissive there.
I can check tomorrow whether lld or ld report any warnings for this on Linux.

[petrochenkov]

Perhaps linker implementations are more permissive there. I can check tomorrow whether lld or ld report any warnings for this on Linux.

Yep, no errors or warnings with ELF linkers (both lld and binutils).

[petrochenkov]

The CI failure doesn't look relevant to me.

[petrochenkov]

ping @mshockwave

[mshockwave]

LGTM. Thank you!

Side note: please don't force push unless it's necessary (e.g. to resolve failing tests). Please append with new commits and squash them upon merging the PR.