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

[JonChesterfield]

Summary

Using llvm-link to add rocm device-libs probably doesn't work

[JonChesterfield]

Using phab as a bug tracker here, but am also happy to check in that comment as-is if someone hits the green button.

The problem is that rocm device-libs doesn't have architecture specific attributes set because it uses one IR file for multiple GPU architectures. This relies on -mlink-builtin-bitcode setting appropriate attributes for the target presently being compiled when the files are pulled in.

Since we're passing the device libs at link time (to fit the '-lm' link time mode, and so we don't have N copies of the same code in every application) we can't use mlink-builtin-bitcode and presently use llvm-link.

That means we splice in bitcode that doesn't have the right attributes set, and our backend assumes that the correct attributes will be set. Outlined a few workarounds in the comment, not personally certain which way to go here.

clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp

if (Args.hasArg(options::OPT_l)) {
     for (auto &Lib : Lm) {
       if (Lib == "m") {
         HasLibm = true;
         break;
       }
     }
 
     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) {
         CmdArgs.push_back(Args.MakeArgString(BCFile));
       });
     }
   }