[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

This revision was automatically updated to reflect the committed changes. Closed by commit rG194ec844f5c6: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU (authored by jhuber6). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133726/new/ ht

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

JonChesterfield accepted this revision. JonChesterfield added a comment. This revision is now accepted and ready to land. I don't like this but will concede it's quicker than changing device libs to contain IR that doesn't have to be patched on the fly. Repository: rG LLVM Github Monorepo CH

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 updated this revision to Diff 459911. jhuber6 added a comment. Changing to `getDeviceLibs`. I suppose in the future we could make this work for CUDA, but for now it won't be defined for that toolchain so it's fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

yaxunl added inline comments. Comment at: clang/include/clang/Driver/ToolChain.h:719 virtual llvm::SmallVector - getHIPDeviceLibs(const llvm::opt::ArgList &Args) const; + getAMDGPUDeviceLibs(const llvm::opt::ArgList &Args) const; jhuber6 wrote: > yaxunl wr

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 added inline comments. Comment at: clang/include/clang/Driver/ToolChain.h:719 virtual llvm::SmallVector - getHIPDeviceLibs(const llvm::opt::ArgList &Args) const; + getAMDGPUDeviceLibs(const llvm::opt::ArgList &Args) const; yaxunl wrote: > well, HIP

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

yaxunl added inline comments. Comment at: clang/include/clang/Driver/ToolChain.h:719 virtual llvm::SmallVector - getHIPDeviceLibs(const llvm::opt::ArgList &Args) const; + getAMDGPUDeviceLibs(const llvm::opt::ArgList &Args) const; well, HIPSPV toolchain is

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 updated this revision to Diff 459874. jhuber6 added a comment. Changing interface to `getAMDGPUDeviceLibs`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133726/new/ https://reviews.llvm.org/D133726 Files: clang/include/clang/Driver/Tool

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

yaxunl added inline comments. Comment at: clang/include/clang/Driver/ToolChain.h:719 virtual llvm::SmallVector - getHIPDeviceLibs(const llvm::opt::ArgList &Args) const; + getROCmDeviceLibs(const llvm::opt::ArgList &Args) const; HIPSPV toolchain is not impl

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 updated this revision to Diff 459848. jhuber6 added a comment. Removing old function update for 'mcpu' Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133726/new/ https://reviews.llvm.org/D133726 Files: clang/include/clang/Driver/ToolChain

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:720-722 + if (DriverArgs.hasArg(options::OPT_march_EQ)) +return getProcessorFromTargetID( +getTriple(), DriverArgs.getLastArgValue(options::OPT_march_EQ)); yaxunl wrote

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

yaxunl added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:720-722 + if (DriverArgs.hasArg(options::OPT_march_EQ)) +return getProcessorFromTargetID( +getTriple(), DriverArgs.getLastArgValue(options::OPT_march_EQ)); It seems the

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 updated this revision to Diff 459786. jhuber6 added a comment. Removing use of `getGPUArch` and just using `-march` directly for OpenMP Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133726/new/ https://reviews.llvm.org/D133726 Files: cla

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

JonChesterfield added a comment. march for openmp, mcpu for hip seems ok. Notably llc needs to be told this as well, using mcpu, which may be an issue for save-temps Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133726/new/ https://reviews.llvm.or

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

yaxunl added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:717 AMDGPUToolChain::getGPUArch(const llvm::opt::ArgList &DriverArgs) const { - return getProcessorFromTargetID( - getTriple(), DriverArgs.getLastArgValue(options::OPT_mcpu_EQ)); + if (Driver

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:717 AMDGPUToolChain::getGPUArch(const llvm::opt::ArgList &DriverArgs) const { - return getProcessorFromTargetID( - getTriple(), DriverArgs.getLastArgValue(options::OPT_mcpu_EQ)); + if (Drive

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

yaxunl added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:717 AMDGPUToolChain::getGPUArch(const llvm::opt::ArgList &DriverArgs) const { - return getProcessorFromTargetID( - getTriple(), DriverArgs.getLastArgValue(options::OPT_mcpu_EQ)); + if (Driver

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 added a comment. In D133726#3786607 , @arsenm wrote: > Does this fix the weird behavior where you needed to use -lm to use anything > in the device libraries? I don't see that being removed That was removed earlier when these files were just sen

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

arsenm added a comment. Does this fix the weird behavior where you needed to use -lm to use anything in the device libraries? I don't see that being removed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133726/new/ https://reviews.llvm.org/D133726

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:717 AMDGPUToolChain::getGPUArch(const llvm::opt::ArgList &DriverArgs) const { - return getProcessorFromTargetID( - getTriple(), DriverArgs.getLastArgValue(options::OPT_mcpu_EQ)); + if (Drive

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

yaxunl added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:717 AMDGPUToolChain::getGPUArch(const llvm::opt::ArgList &DriverArgs) const { - return getProcessorFromTargetID( - getTriple(), DriverArgs.getLastArgValue(options::OPT_mcpu_EQ)); + if (Driver

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 updated this revision to Diff 459721. jhuber6 added a comment. Adding a test for using `-nogpulib`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133726/new/ https://reviews.llvm.org/D133726 Files: clang/include/clang/Driver/ToolChain.h

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 updated this revision to Diff 459716. jhuber6 added a comment. Renaming virtual function to make it more generic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133726/new/ https://reviews.llvm.org/D133726 Files: clang/include/clang/Drive

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

JonChesterfield added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.h:58 + llvm::SmallVector + getHIPDeviceLibs(const llvm::opt::ArgList &Args) const override; + jhuber6 wrote: > JonChesterfield wrote: > > Why hip device libs? There's a

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 added a comment. In D133726#3785040 , @JonChesterfield wrote: > We can do this but should expect an increase in code size from having > multiple internalised copies of the same function. There may be an incidental > benefit if we can specialise

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.h:58 + llvm::SmallVector + getHIPDeviceLibs(const llvm::opt::ArgList &Args) const override; + JonChesterfield wrote: > Why hip device libs? There's a common set, plus a hip.bc p

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

JonChesterfield added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.h:58 + llvm::SmallVector + getHIPDeviceLibs(const llvm::opt::ArgList &Args) const override; + Why hip device libs? There's a common set, plus a hip.bc plus a opencl.bc.

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

JonChesterfield added a comment. We can do this but should expect an increase in code size from having multiple internalised copies of the same function. There may be an incidental benefit if we can specialise some functions to the call site without additional cloning. Address of the same funct

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

jhuber6 created this revision. jhuber6 added reviewers: jdoerfert, JonChesterfield, ronlieb, arsenm, yaxunl, tianshilei1992, ye-luo. Herald added subscribers: kosarev, kerbowa, guansong, t-tye, tpr, dstuttard, jvesely, kzhuravl. Herald added a project: All. jhuber6 requested review of this revisi