[PATCH] D112639: [openmp][amdgpu] Add comment warning that libm may be broken

Jon Chesterfield via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 27 10:57:56 PDT 2021


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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D112639.382721.patch
Type: text/x-patch
Size: 1622 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211027/4cbd18fe/attachment.bin>


More information about the cfe-commits mailing list