[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

This revision was automatically updated to reflect the committed changes. Closed by commit rGfcf03e728007: [OpenMP] Add OpenMP offloading toolchain for AMDGPU (authored by Pushpinder Singh , committed by pdhaliwal). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. fine with me, assuming this doesn't disturb anything outside the AMD pipeline. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94961/new/ ht

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

JonChesterfield added inline comments. Comment at: clang/lib/Driver/Driver.cpp:755 + *this, TT, *HostTC, C.getInputArgs(), Action::OFK_OpenMP); +else if (TT.isAMDGCN()) + DeviceTC = pdhaliwal wrote: > JonChes

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

pdhaliwal added inline comments. Comment at: clang/lib/Driver/Driver.cpp:755 + *this, TT, *HostTC, C.getInputArgs(), Action::OFK_OpenMP); +else if (TT.isAMDGCN()) + DeviceTC = JonChesterfield wrote: > Minor s

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

pdhaliwal updated this revision to Diff 320740. pdhaliwal added a comment. - Use 0 for default -O option - Rename addOptLevelArgs to addLLCOptArg Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94961/new/ https://reviews.llvm.org/D94961 Files: cla

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

JonChesterfield added a comment. Looks much simpler, thanks! Comment at: clang/lib/Driver/Driver.cpp:755 + *this, TT, *HostTC, C.getInputArgs(), Action::OFK_OpenMP); +else if (TT.isAMDGCN()) + DeviceTC = Mi

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

JonChesterfield added inline comments. Comment at: clang/test/Driver/amdgpu-openmp-toolchain.c:2 +// REQUIRES: amdgpu-registered-target +// RUN: %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

pdhaliwal marked 3 inline comments as done. pdhaliwal added a comment. After addressing the review comments, I have internally verified changes on few simple test programs. They seem to be working fine. Comment at: clang/lib/Driver/Driver.cpp:3004 +} + } + //

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

pdhaliwal updated this revision to Diff 320722. pdhaliwal added a comment. Addressed review comments. - Combined the toolchain creation logic for nvptx and amdgcn - Replaced -Xopenmp-target with -emit-llvm-bc inside AMDGPUOpenMP.cpp - Removed opt from pipeline Repository: rG LLVM Github Monor

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

jdoerfert added a comment. Generally OK, the more I look at it the more things pop up. Other than my comments, I'm fine with this going in. Comment at: clang/lib/Driver/Driver.cpp:767 + } + TC = AMDGPUOpenMPTC.get(); } else --

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

JonChesterfield added a comment. I believe this is 'safe', in the sense that it can't break anything other than AMDGPU's openmp implementation, which doesn't work yet anyway. The change to non-amd code is to OpenMPActionBuilder, and while parsing `-march=gfx` isn't pretty, it won't fire on nvpt

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

JonChesterfield added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:65 + .Case("g", "1") + .Default("2"); +} JonChesterfield wrote: > jdoerfert wrote: > > To verify, O0 is not mapped to O2, correct?

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

JonChesterfield added a comment. In D94961#2533848 , @jdoerfert wrote: > Overall, looks reasonable. I'm not a fan of the way things are hooked up but > that is also true for the NVPTX toolchain and for now unavoidable I guess. This is loosely based on th

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

jdoerfert added a comment. Overall, looks reasonable. I'm not a fan of the way things are hooked up but that is also true for the NVPTX toolchain and for now unavoidable I guess. Some nits below. Comment at: clang/lib/Driver/Driver.cpp:2999 +if (VStr.startswith("-

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

saiislam added a comment. LGTM. But, please wait from someone outside AMD to accept it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94961/new/ https://reviews.llvm.org/D94961 ___ cfe-commits mailing li

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

pdhaliwal added a comment. Ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94961/new/ https://reviews.llvm.org/D94961 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

JonChesterfield added a comment. The deviceRTL is changing from cuda/hip to openmp at present. It would be good to be able to compile that as openmp for amdgpu, which needs a patch roughly like this and probably some follow up. It's plausible that the contents of this file are only really of in

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

pdhaliwal updated this revision to Diff 318120. pdhaliwal added a comment. - Moved common methods of HIP and OpenMP to base ROCMToolChain - Removed unnecessary asserts Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94961/new/ https://reviews.llvm.or

[PATCH] D94961: [OpenMP] Add OpenMP offloading toolchain for AMDGPU

pdhaliwal updated this revision to Diff 317831. pdhaliwal added a comment. Fixed failing debian tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94961/new/ https://reviews.llvm.org/D94961 Files: clang/lib/Driver/CMakeLists.txt clang/lib/Dri