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