jhuber6 added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:8195
+  // Get the AMDGPU math libraries.
+  // FIXME: This method is bad, remove once AMDGPU has a proper math library.
+  for (auto &I : llvm::make_range(OpenMPTCRange.first, OpenMPTCRange.second)) {
----------------
jdoerfert wrote:
> Can you elaborate on this comment, what's bad, how would the better version 
> look
It's explained in more detail where this is done for the AMDGPU ToolChain, e.g.
```
      // 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
```
Should I copy the gist here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119841/new/

https://reviews.llvm.org/D119841

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to