JonChesterfield created this revision.
JonChesterfield added reviewers: arsenm, jdoerfert, gregrodgers, pdhaliwal,
ye-luo, tianshilei1992, b-sumner.
Herald added subscribers: kerbowa, guansong, t-tye, tpr, dstuttard, yaxunl,
nhaehnle, jvesely, kzhuravl.
JonChesterfield requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, wdng.
Herald added a project: clang.
Using llvm-link to add rocm device-libs probably doesn't work
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D112639
Files:
clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===================================================================
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -106,6 +106,22 @@
}
if (HasLibm) {
+ // This is not certain to work. The device libs added here, and passed to
+ // llvm-link, are missing attributes that they expect to be inserted when
+ // passed to mlink-builtin-bitcode. The amdgpu backend does not generate
+ // conservatively correct code when attributes are missing, so this may
+ // be the root cause of miscompilations. Passing via
mlink-builtin-bitcode
+ // ultimately hits CodeGenModule::addDefaultFunctionDefinitionAttributes
+ // on each function, see D28538 for context.
+ // Potential workarounds:
+ // - unconditionally link all of the device libs to every translation
+ // unit in clang via mlink-builtin-bitcode
+ // - build a libm bitcode file as part of the DeviceRTL and explictly
+ // mlink-builtin-bitcode the rocm device libs components at build time
+ // - drop this llvm-link fork in favour or some calls into LLVM, chosen
+ // to do basically the same work as llvm-link but with that call first
+ // - write an opt pass that sets that on every function it sees and pipe
+ // the device-libs bitcode through that on the way to this llvm-link
SmallVector<std::string, 12> BCLibs =
AMDGPUOpenMPTC.getCommonDeviceLibNames(Args, SubArchName.str());
llvm::for_each(BCLibs, [&](StringRef BCFile) {
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===================================================================
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -106,6 +106,22 @@
}
if (HasLibm) {
+ // This is not certain to work. The device libs added here, and passed to
+ // llvm-link, are missing attributes that they expect to be inserted when
+ // passed to mlink-builtin-bitcode. The amdgpu backend does not generate
+ // conservatively correct code when attributes are missing, so this may
+ // be the root cause of miscompilations. Passing via mlink-builtin-bitcode
+ // ultimately hits CodeGenModule::addDefaultFunctionDefinitionAttributes
+ // on each function, see D28538 for context.
+ // Potential workarounds:
+ // - unconditionally link all of the device libs to every translation
+ // unit in clang via mlink-builtin-bitcode
+ // - build a libm bitcode file as part of the DeviceRTL and explictly
+ // mlink-builtin-bitcode the rocm device libs components at build time
+ // - drop this llvm-link fork in favour or some calls into LLVM, chosen
+ // to do basically the same work as llvm-link but with that call first
+ // - write an opt pass that sets that on every function it sees and pipe
+ // the device-libs bitcode through that on the way to this llvm-link
SmallVector<std::string, 12> BCLibs =
AMDGPUOpenMPTC.getCommonDeviceLibNames(Args, SubArchName.str());
llvm::for_each(BCLibs, [&](StringRef BCFile) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
