PR for llvm/llvm-project#70458 #752
Conversation
This adds a new pass to add an Any comdat to each linkonce and linkonce_odr function in the LLVM dialect. These comdats are necessary on Windows to allow the default system linker to link binaries containing these functions. (cherry picked from commit a6857156df9a73720fbd9633067d1a61c32dd74e)
This fixes a bug where functions generated by the MLIR Math dialect, for example ipowi, would fail to link with link.exe on Windows due to having linkonce linkage but no associated comdat. Adding the comdat on ELF also allows linkers to perform better garbage collection in the binary. Simply adding comdats to all functions with this linkage type should also cover future cases where linkonce or linkonce_odr functions might be necessary. (cherry picked from commit 5f476b80e3d472f672f5f6a719eebe2c0aadf52c)
llvmbot
force-pushed
the
llvm-issue70458
branch
from
2c9592f to
9fe6c30
Compare
|
@tru can we merge this into 17.x. This fixes Flang MSVC compilation for 17.x |
|
@omjavaid Looking at the commit this seems a little like a new feature? or is this a regression from 16? |
|
Hi Tobias, this is a regression we found when compiling OpenBLAS with LLVM-17, and latest cmake, that now supports flang-new with MSVC ABI. LLVM nightly compiles it cleanly. See more details in llvm/llvm-project#70458. |
|
Roping in @joker-eph as well since this touches MLIR - does this seem safe to merge this late in the release cycle? |
|
On the MLIR side this adds a pass, which won't affect anyone not using it (so by definition it can't affect any existing users of the release). It seems like needed for fixing Flang thought, so fine for me from the MLIR point of view, I don't know the assessment on Flang. |
|
Ok sounds like it pretty safe to merge. |
resolves llvm/llvm-project#70458