This reverts commit 83d59e05b201760e3f364ff6316301d347cbad95.
D108850 is being reverted due to breaking multiple aspects of how the
LLVM toolchain interacts with HIP, both in MLIR and for compilation on
Windows.
Paths
| Differential D119257
Revert "Re-land [LLD] Remove global state in lldCommon" AbandonedPublic Authored by krzysz00 on Feb 8 2022, 9:32 AM.
Details
Summary This reverts commit 83d59e05b201760e3f364ff6316301d347cbad95. D108850 is being reverted due to breaking multiple aspects of how the
Diff Detail
Unit TestsFailed Event TimelineHerald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald Transcript Herald added subscribers: llvm-commits, cfe-commits, stephenneuendorffer and 2 others. · View Herald Transcript Comment Actions @aganea Can we expect fixes for both the issues with this revision (the HIP-on-Windows problems noted last night and the MLIR breakage) shortly or should we revert while y'all work out fixes? Comment Actions Thanks to @aganea for working on this. I agree that lld/ELF is not ready for library usage. So far the arguments have always been "the library usage is easy since lld headers happen to be exposed", but do not justify that it should be done this way. Most use cases can be replaced with spawning a new process. For lld/ELF, the library usage is unsupported and may happen to work for some use cases. But please move away from the library usage, or expect that it may break at any time, and don't use "it worked for some time for us" as an argument for reverts like https://reviews.llvm.org/D119257 If anything needs to be reverted, the original change adding lld::elf::link for AMDGPU usage should be reverted instead. This revision now requires changes to proceed.Feb 8 2022, 1:23 PM Comment Actions (The discussion seems to be happening in two places: https://github.com/llvm/llvm-project/issues/53475 ) You can break downstream users, the relationship inside the monorepo is different as far as I know: I believe we will have to live with revert like the one above until you collaborate actively to find a supported way and move there though. Comment Actions @mehdi_amini Just to unblock users, I've posted https://reviews.llvm.org/D119277 which fixes the issue originally raised by @krzysz00 Comment Actions (Since it looks like the discussion moved here) @MaskRay We can't spawn lld as a subprocess due to customer requirements around having the kernel generator/compiler delivered as a static library in some cases. (And there's LLVM version compatibility concerns, but those might be less of an issue in this case) Comment Actions Having checked with other folks more familiar with the context of our project, it looks like we might actually be fine using lld as a subprocess, since we'll always be sitting on top of a ROCm installation, which includes lld, and since "take this object file and wrap it into a shared library" is extremely unlikely to be broken by version changes. That being said, I've also heard that when this code was initially landing (as part of mlir-rocm-runner, and before my time), the feedback was to use lld as a library and not a subprocess. Comment Actions
Thanks for the flexibility :)
I guess they did not realize how tricky to make lld/ELF library usage more reliable ...
Revision Contents
Diff 406872 clang/lib/Driver/ToolChains/MSVC.cpp
lld/COFF/COFFLinkerContext.h
lld/COFF/Chunks.cpp
lld/COFF/DLL.cpp
lld/COFF/Driver.cpp
lld/COFF/DriverUtils.cpp
lld/COFF/InputFiles.cpp
lld/COFF/LTO.cpp
lld/COFF/MinGW.cpp
lld/COFF/PDB.cpp
lld/COFF/SymbolTable.cpp
lld/COFF/Writer.cpp
lld/Common/CMakeLists.txt
lld/Common/CommonLinkerContext.cpp
lld/Common/ErrorHandler.cpp
lld/Common/Memory.cpp
lld/Common/TargetOptionsCommandFlags.cpp
lld/ELF/AArch64ErrataFix.cpp
lld/ELF/ARMErrataFix.cpp
lld/ELF/Arch/PPC64.cpp
lld/ELF/Driver.cpp
lld/ELF/DriverUtils.cpp
lld/ELF/InputFiles.cpp
lld/ELF/InputSection.cpp
lld/ELF/LinkerScript.cpp
lld/ELF/MarkLive.cpp
lld/ELF/ScriptParser.cpp
lld/ELF/SyntheticSections.cpp
lld/ELF/Thunks.cpp
lld/ELF/Writer.cpp
lld/MachO/ConcatOutputSection.cpp
lld/MachO/Driver.cpp
lld/MachO/DriverUtils.cpp
lld/MachO/InputFiles.cpp
lld/MachO/LTO.cpp
lld/MachO/SectionPriorities.cpp
lld/MachO/SyntheticSections.cpp
lld/MachO/Writer.cpp
lld/MinGW/Driver.cpp
lld/include/lld/Common/CommonLinkerContext.h
lld/include/lld/Common/Driver.h
lld/include/lld/Common/ErrorHandler.h
lld/include/lld/Common/Memory.h
lld/include/lld/Core/LinkingContext.h
lld/tools/lld/lld.cpp
lld/wasm/Driver.cpp
lld/wasm/InputFiles.cpp
lld/wasm/SymbolTable.cpp
lld/wasm/Writer.cpp
llvm/include/llvm/DebugInfo/PDB/DIA/DIASupport.h
mlir/lib/Dialect/GPU/Transforms/SerializeToHsaco.cpp
|