[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

pdhaliwal added a comment. Hi, Sorry for late reply. > If that is not feasible for some reason, I would lean towards your option > (2), but I think more is needed in this patch to ensure the generation script > is always run, right? I think you are right. As of now, just removing the dependen

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

pdhaliwal added a comment. > 3. Another way is to gracefully handle the file write error, for which I > don't think there is a portable way. (which @scott.linder also suggested) I have found the portable way to do this. So cmake provides a command-line tool `touch` (doc

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

pdhaliwal updated this revision to Diff 266172. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79400/new/ https://reviews.llvm.org/D79400 Files: llvm/cmake/modules/AddLLVM.cmake Index: llvm/cmake/modules/AddLLVM.cmake

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

pdhaliwal updated this revision to Diff 266794. pdhaliwal marked an inline comment as done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79400/new/ https://reviews.llvm.org/D79400 Files: llvm/cmake/modules/AddLLVM.cmake llvm/include/llvm/Suppor

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

pdhaliwal added inline comments. Comment at: llvm/cmake/modules/AddLLVM.cmake:1945 + if (NOT touch_head_result EQUAL 0) +return() + endif() scott.linder wrote: > This seems to implement the behavior that when the log is not present a

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

pdhaliwal updated this revision to Diff 267123. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79400/new/ https://reviews.llvm.org/D79400 Files: llvm/cmake/modules/AddLLVM.cmake llvm/include/llvm/Support/CMakeLists.txt Index: llvm/include/llvm/

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

pdhaliwal updated this revision to Diff 267153. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79400/new/ https://reviews.llvm.org/D79400 Files: llvm/cmake/modules/AddLLVM.cmake llvm/include/llvm/Support/CMakeLists.txt Index: llvm/include/llvm/

[PATCH] D80996: [AMDGPU][OpenMP] Fix duplicate copies of arguments in commands

pdhaliwal created this revision. pdhaliwal added reviewers: yaxunl, msearles, sameerds. Herald added subscribers: cfe-commits, sstefan1, guansong, t-tye, tpr, dstuttard, wdng, kzhuravl. Herald added a reviewer: jdoerfert. Herald added a project: clang. When offloading kind is OFK_OpenMP, the host

[PATCH] D80996: [AMDGPU][OpenMP] Fix duplicate copies of arguments in commands

pdhaliwal updated this revision to Diff 267870. pdhaliwal added a comment. Added lit test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80996/new/ https://reviews.llvm.org/D80996 Files: clang/lib/Driver/ToolChains/HIP.cpp clang/test/Drive

[PATCH] D80996: [AMDGPU][OpenMP] Fix duplicate copies of arguments in commands

pdhaliwal added inline comments. Comment at: clang/lib/Driver/ToolChains/HIP.cpp:389 - for (Arg *A : Args) { -DAL->append(A); + if (DeviceOffloadKind != Action::OFK_OpenMP) { +for (Arg *A : Args) { arsenm wrote: > Needs a comment? I don't understand w

[PATCH] D84221: [OpenMP] Add missing RUN lines for OpenMP 4.5

pdhaliwal created this revision. pdhaliwal added reviewers: saiislam, ABataev, jdoerfert. Herald added subscribers: cfe-commits, sstefan1, guansong, yaxunl. Herald added a project: clang. This was missed when default version was upgraded to 5.0 (part of D81098 )

[PATCH] D80996: [AMDGPU][OpenMP] Fix duplicate copies of arguments in commands

pdhaliwal abandoned this revision. pdhaliwal marked an inline comment as done. pdhaliwal added inline comments. Comment at: clang/lib/Driver/ToolChains/HIP.cpp:389 - for (Arg *A : Args) { -DAL->append(A); + if (DeviceOffloadKind != Action::OFK_OpenMP) { +for (Arg *A :

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

pdhaliwal added a comment. In D79400#2121685 , @vsapsai wrote: > Seems like this change causes `ninja clean` to fail with the error > > > ninja: error: remove(include/llvm/Support): Directory not empty > > Full repro steps are > > > ninja install >

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

pdhaliwal created this revision. pdhaliwal added reviewers: chandlerc, beanz, pcc. Herald added subscribers: llvm-commits, cfe-commits, mgorny. Herald added projects: clang, LLVM. The AddLLVM.cmake file exposes a function find_first_existing_vc_file which in case of git repository looks for .git/

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

pdhaliwal updated this revision to Diff 262299. pdhaliwal added a comment. Added more context to diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79400/new/ https://reviews.llvm.org/D79400 Files: clang/lib/Basic/CMakeLists.txt lld/Common/CM

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

pdhaliwal added a comment. In D79400#2021255 , @scott.linder wrote: > Can you provide more context in the diff? Looking at the current `master` > locally it seems like this patch changes the behavior of the > `llvm_vcsrevision_h` target when `logs/HEAD`

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

pdhaliwal added a comment. Removing `find_first_existing_vc_file` makes a lot of sense, as since llvm has moved from svn to git, there is no need to have logic for svn dependency. Even the generation script is dependent on git executable only. I will keep my patch ready just in case other revie

[PATCH] D79400: [CMAKE] Fix build failure when source directory is read only

pdhaliwal added a comment. I understand that `.git/logs/HEAD` acts as a dependency for `vcs_revision_h` target. However, problem here is that cmake fails when it tries to create `.git/logs/HEAD` in read-only filesystem. I had following ideas for solving above issue, 1. Skip creating `.git/logs

[PATCH] D105981: [AMDGPU][OpenMP] Support linking of math libraries

pdhaliwal updated this revision to Diff 361967. pdhaliwal added a comment. Extract the options from HIP/OpenMP to a common method in base class. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105981/new/ https://reviews.llvm.org/D105981 Files: cl

[PATCH] D105981: [AMDGPU][OpenMP] Support linking of math libraries

pdhaliwal added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:923-924 + bool CorrectSqrt = DriverArgs.hasFlag( + options::OPT_fhip_fp32_correctly_rounded_divide_sqrt, + options::OPT_fno_hip_fp32_correctly_rounded_divide_sqrt); + bool Wave64 = isW

[PATCH] D105981: [AMDGPU][OpenMP] Support linking of math libraries

pdhaliwal added a comment. Due to the current state of math headers, I was unable to test this patch without ockl. But last time when headers were working, I was actually required to link ockl for a symbol (I forgot the name). I will update once I am able to get the math headers work again. R

[PATCH] D105981: [AMDGPU][OpenMP] Support linking of math libraries

pdhaliwal updated this revision to Diff 362717. pdhaliwal added a comment. Rename method to getCommonDeviceLibNames Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105981/new/ https://reviews.llvm.org/D105981 Files: clang/lib/Driver/ToolChains/AMD

[PATCH] D105981: [AMDGPU][OpenMP] Support linking of math libraries

pdhaliwal updated this revision to Diff 362718. pdhaliwal added a comment. Missed comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105981/new/ https://reviews.llvm.org/D105981 Files: clang/lib/Driver/ToolChains/AMDGPU.cpp clang/lib/Drive

[PATCH] D102065: [AMDGPU][OpenMP] Emit textual IR for -emit-llvm -S

pdhaliwal updated this revision to Diff 344763. pdhaliwal added a comment. Fixed the if-else logic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102065/new/ https://reviews.llvm.org/D102065 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/te

[PATCH] D102065: [AMDGPU][OpenMP] Emit textual IR for -emit-llvm -S

pdhaliwal added a comment. Tested on gfx906. The libomptarget tests are working as expected. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102065/new/ https://reviews.llvm.org/D102065 ___ cfe-commits mai

[PATCH] D102065: [AMDGPU][OpenMP] Emit textual IR for -emit-llvm -S

This revision was automatically updated to reflect the committed changes. Closed by commit rG10c779d2065f: [AMDGPU][OpenMP] Emit textual IR for -emit-llvm -S (authored by pdhaliwal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102065/new/ https://

[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

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

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 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

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

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] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

pdhaliwal created this revision. pdhaliwal added reviewers: jdoerfert, JonChesterfield, ronlieb, saiislam, ABataev. Herald added subscribers: dang, kerbowa, guansong, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. Herald added a reviewer: jansvoboda11. pdhaliwal requested review of t

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

pdhaliwal updated this revision to Diff 322052. pdhaliwal added a comment. Accidently missed some changes, - Fix openmp-offload.c test failure - Fix amdgpu-openmp-toolchain.c test failure Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96248/new/ ht

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

pdhaliwal added a comment. It looks like from IR diff that this patch is adding use of kmpc_alloc_shared method. These methods likely won't work on AMDGPU as device malloc is not available. Not sure what could be done apart from marking those tests as XFAIL on amdgcn. :( Repository: rG LLVM

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

pdhaliwal added a comment. I got this after changing __kmpc_impl_malloc to return 0xdeadbeef. So, this confirms that missing malloc implementation is the root cause. > Memory access fault by GPU node-4 (Agent handle: 0x1bc5000) on address > 0xdeadb000. Reason: Page not present or supervisor pri

[PATCH] D105191: [Clang][OpenMP] Add support for Static Device Libraries

pdhaliwal added inline comments. Comment at: clang/test/Driver/fat_archive.cpp:9 +// could be resolved correctly. +// RUN: env LIBRARY_PATH=%T/../../../../../runtimes/runtimes-bins/openmp/libomptarget %clang -O2 -target x86_64-pc-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

pdhaliwal added a comment. Apologies for late reply. Most of the tests now do not try to call malloc, so no page fault errors. But all of them are producing wrong results. For e.g. declare_mapper_target.cpp produces Sum = 132608 with the patch applied. Similarly for other tests as well. So don'

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

pdhaliwal added a comment. I modified the declare_mapper_target to print the contents of array after target region and found the following output: 2 3 4 5 6 7 8 9 10 11 Sum = 65 Program: #include #include #define NUM 10 int main() { int *c= new int[NUM]; for (int i =

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

pdhaliwal updated this revision to Diff 322090. pdhaliwal added a comment. Addressed review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96248/new/ https://reviews.llvm.org/D96248 Files: clang/include/clang/Driver/Options.td clang/l

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

pdhaliwal updated this revision to Diff 322298. pdhaliwal added a comment. - Added check for nogpulib - Fixed diagnostic message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96248/new/ https://reviews.llvm.org/D96248 Files: clang/include/clang/

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

pdhaliwal updated this revision to Diff 322638. pdhaliwal marked an inline comment as done. pdhaliwal added a comment. I haave removed libomptarget-device-bc-path and have added amdgcn one. For diagnostic, instead of having one per architecture, I have used the same and added second parameter to

[PATCH] D96248: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG79401b43ce4e: [OpenMP][AMDGPU] Add support for linking libomptarget bitcode (authored by Pushpinder Singh , committe

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

pdhaliwal created this revision. pdhaliwal added reviewers: jdoerfert, JonChesterfield, ronlieb. Herald added subscribers: kerbowa, guansong, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. pdhaliwal requested review of this revision. Herald added subscribers: cfe-commits, sstefan1, wd

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

pdhaliwal added inline comments. Comment at: clang/lib/Driver/Driver.cpp:3057 + for (unsigned I = 0; I < ToolChains.size(); ++I) { +Action *&A = OpenMPDeviceActions[I]; +// AMDGPU does not support linking of object files, so we skip This logi

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

pdhaliwal added a comment. This does fixes the save-temps but only when -o is not specified. If -o is specified the name of host object file and host-wrapper object file (second last phase) is same, which fails the linker. This does not seem to be related to this patch. Repository: rG LLVM

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

pdhaliwal added a comment. emit-llvm-bc does not correctly solve the problem. It works because [input, compile, assemble, backend] actions collapse to a single action by driver. This single command handles emit-llvm-bc properly. But when save-temps is specified, this collapsing does not happen

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

pdhaliwal added a comment. It is because of how addClangTargetOptions is invoked. In case of save-temps, it is being invoked for all the actions resulting in target cc1 call. That's why all these invocations have -emit-llvm-bc. I guess we need Action as an argument to addClangTargetOptions. Al

[PATCH] D97273: OpenMP: Fix object clobbering issue when using save-temps

pdhaliwal created this revision. pdhaliwal added reviewers: jdoerfert, JonChesterfield, ronlieb, tianshilei1992. Herald added subscribers: guansong, yaxunl. pdhaliwal requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. There are two pr

[PATCH] D97273: OpenMP: Fix object clobbering issue when using save-temps

pdhaliwal added a comment. Here's a bit of background, OffloadingPrefix was not getting properly set in the dependent actions of OffloadWrapperJobAction (which are backend [11] and assemble [12]). Since backend [11] and assemble [12] host-wrapper actions have same logic to the other host action

[PATCH] D97273: OpenMP: Fix object clobbering issue when using save-temps

This revision was automatically updated to reflect the committed changes. Closed by commit rG99951aa68da3: OpenMP: Fix object clobbering issue when using save-temps (authored by pdhaliwal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97273/new/ ht

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

pdhaliwal added a comment. So, neither emit-llvm-bc or emit-llvm work well with save-temps. Therefore, I feel the current approach is still valid. This does not impact nvptx or any other target in any way. And I don't see how. I see valid concern regarding assembly output. This patch will surel

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

pdhaliwal updated this revision to Diff 326392. pdhaliwal added a comment. Add extra llc step to produce assembly in the linker. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96769/new/ https://reviews.llvm.org/D96769 Files: clang/lib/Driver/Dri

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

pdhaliwal updated this revision to Diff 362997. pdhaliwal added a comment. It required some work to fix the failing lit test case. And many thanks to @estewart for helping in that. The current status is that we are now following the nvptx openmp strategy for openmp math headers very closely. In t

[PATCH] D105981: [AMDGPU][OpenMP] Support linking of math libraries

This revision was automatically updated to reflect the committed changes. Closed by commit rG9830f902e4d0: [AMDGPU][OpenMP] Support linking of math libraries (authored by pdhaliwal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105981/new/ https://

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

pdhaliwal updated this revision to Diff 363090. pdhaliwal added a comment. Addressed review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104904/new/ https://reviews.llvm.org/D104904 Files: clang/lib/Driver/ToolChains/Clang.cpp clang

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG12da97ea10a9: [OpenMP][AMDGCN] Initial math headers support (authored by pdhaliwal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

pdhaliwal updated this revision to Diff 363387. pdhaliwal added a comment. Fixed compilation error for nvptx headers. Tested on both cuda and non-cuda systems. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104904/new/ https://reviews.llvm.org/D104

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

pdhaliwal added a comment. @ye-luo and @JonChesterfield can you please test the latest version of this patch? It should work now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104904/new/ https://reviews.llvm.org/D104904 _

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG713a5d12cde5: [OpenMP][AMDGCN] Initial math headers support (authored by pdhaliwal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

pdhaliwal added inline comments. Comment at: clang/lib/Headers/openmp_wrappers/__clang_openmp_device_functions.h:38 + +#pragma omp begin declare variant match(device = {arch(amdgcn)}) + JonChesterfield wrote: > Given that declare variant didn't work elsewhere, i

[PATCH] D107468: [AMDGPU][OpenMP] Wrap amdgcn declare variant inside ifdef

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf3eb5f900d2a: [AMDGPU][OpenMP] Wrap amdgcn declare variant inside ifdef (authored by pdhaliwal). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D107468: [AMDGPU][OpenMP] Wrap amdgcn declare variant inside ifdef

pdhaliwal created this revision. pdhaliwal added reviewers: JonChesterfield, ye-luo, ronlieb. Herald added subscribers: guansong, t-tye, tpr, dstuttard, yaxunl, kzhuravl. pdhaliwal requested review of this revision. Herald added subscribers: cfe-commits, sstefan1, wdng. Herald added a reviewer: jdo

[PATCH] D107952: [AMDGPU][OpenMP] Use llvm-link to link ocml libraries

pdhaliwal created this revision. pdhaliwal added reviewers: ronlieb, JonChesterfield. Herald added subscribers: kerbowa, guansong, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. pdhaliwal requested review of this revision. Herald added subscribers: cfe-commits, sstefan1, wdng. Herald

[PATCH] D107952: [AMDGPU][OpenMP] Use llvm-link to link ocml libraries

pdhaliwal updated this revision to Diff 365965. pdhaliwal added a comment. Remove redundant test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107952/new/ https://reviews.llvm.org/D107952 Files: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp cla

[PATCH] D107952: [AMDGPU][OpenMP] Use llvm-link to link ocml libraries

pdhaliwal added inline comments. Comment at: clang/test/Driver/amdgpu-openmp-toolchain.c:80 +// CHECK-LIB-DEVICE: {{.*}}llvm-link{{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

pdhaliwal created this revision. pdhaliwal added reviewers: JonChesterfield, ye-luo, ronlieb, gregrodgers, jdoerfert. Herald added subscribers: guansong, yaxunl, jvesely. pdhaliwal requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. W

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

pdhaliwal updated this revision to Diff 354471. pdhaliwal added a comment. Fix format errors Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104904/new/ https://reviews.llvm.org/D104904 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Head

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

pdhaliwal updated this revision to Diff 354801. pdhaliwal marked 2 inline comments as done. pdhaliwal added a comment. Addressed review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104904/new/ https://reviews.llvm.org/D104904 Files: c

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

pdhaliwal added inline comments. Comment at: clang/lib/Headers/__clang_hip_math.h:29 +#else #define __DEVICE__ static __device__ inline __attribute__((always_inline)) +#endif JonChesterfield wrote: > wonder if HIP would benefit from nothrow here Would like to ke

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

pdhaliwal updated this revision to Diff 354802. pdhaliwal added a comment. Typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104904/new/ https://reviews.llvm.org/D104904 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/lib/Headers/__clang_h

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

pdhaliwal updated this revision to Diff 354913. pdhaliwal marked 2 inline comments as done. pdhaliwal added a comment. - Move __constant__ to openmp_wrappers/cmath - Using push/pop_macro to avoid redefinition Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

pdhaliwal added inline comments. Comment at: clang/lib/Headers/__clang_hip_cmath.h:30 +#ifdef __OPENMP_AMDGCN__ +#define __DEVICE__ static __attribute__((always_inline, nothrow)) +#define __CONSTEXPR__ constexpr ashi1 wrote: > Does OpenMP not require `__device__`

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

pdhaliwal added inline comments. Comment at: clang/lib/Headers/__clang_hip_cmath.h:96 +__DEVICE__ __CONSTEXPR__ bool isnan(float __x) { return ::__isnanf(__x); } +__DEVICE__ __CONSTEXPR__ bool isnan(double __x) { return ::__isnan(__x); } jdoerfert wrote: > ^ Th

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

pdhaliwal added a comment. Should the name of file be changed as well? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105221/new/ https://reviews.llvm.org/D105221 ___ cfe-commits mailing list cfe-commits@

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

pdhaliwal added a comment. Looks ok to me. Regression tests and runtime tests went fine. Tested a simple cuda and openmp kernel with `sin` function on sm_61, didn't see any issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105221/new/ https://r

[PATCH] D105981: [AMDGPU][OpenMP] Support linking of math libraries

pdhaliwal created this revision. pdhaliwal added reviewers: JonChesterfield, ronlieb, jdoerfert. Herald added subscribers: kerbowa, guansong, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. pdhaliwal requested review of this revision. Herald added subscribers: cfe-commits, sstefan1, wd

[PATCH] D105981: [AMDGPU][OpenMP] Support linking of math libraries

pdhaliwal added inline comments. Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:252 +bool Wave64 = isWave64(DriverArgs, Kind); + +// TODO: There are way too many flags that change this. Do we need to check JonChesterfield wrote: > I recognise th

[PATCH] D105981: [AMDGPU][OpenMP] Support linking of math libraries

pdhaliwal updated this revision to Diff 358614. pdhaliwal added a comment. Move linking logic to a common method. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105981/new/ https://reviews.llvm.org/D105981 Files: clang/lib/Driver/ToolChains/AMDGP

[PATCH] D114865: [AMDGPU][OpenMP] Use -amdgpu-fixed-function-abi

pdhaliwal created this revision. pdhaliwal added reviewers: arsenm, JonChesterfield. Herald added subscribers: kerbowa, guansong, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. pdhaliwal requested review of this revision. Herald added subscribers: openmp-commits, cfe-commits, sstefan1

[PATCH] D101901: [AMDGPU][OpenMP] Fix clang driver crash when provided -c

pdhaliwal created this revision. pdhaliwal added reviewers: JonChesterfield, ronlieb, jdoerfert, ye-luo, tianshilei1992. Herald added subscribers: kerbowa, guansong, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. pdhaliwal requested review of this revision. Herald added subscribers:

[PATCH] D101901: [AMDGPU][OpenMP] Fix clang driver crash when provided -c

This revision was automatically updated to reflect the committed changes. Closed by commit rG1f5cacfcb845: [AMDGPU][OpenMP] Fix clang driver crash when provided -c (authored by pdhaliwal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101901/new/ ht

[PATCH] D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed

pdhaliwal added a comment. I am investigating the find_package issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99949/new/ https://reviews.llvm.org/D99949 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D101926: [amdgpu-arch] Fix rpath to run from build dir

pdhaliwal accepted this revision. pdhaliwal added a comment. This revision is now accepted and ready to land. Looks good to me. Comment at: clang/tools/amdgpu-arch/CMakeLists.txt:17 +set_target_properties(amdgpu-arch PROPERTIES INSTALL_RPATH_USE_LINK_PATH ON) + --

[PATCH] D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed

pdhaliwal added a comment. I could not find anything in the cmake files which could point to the issue mentioned here. @davezarzycki, are you on fedora/redhat? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99949/new/ https://reviews.llvm.org/D9994

[PATCH] D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed

pdhaliwal added a comment. Greg was also interested in having pci ids table in amdgpu-arch. And, keeping this table inside the target/amdgpu directory sounds like a good idea. Overall, I agree with not having dependency on hsa as it has caused many issues. Repository: rG LLVM Github Monorepo

[PATCH] D102065: [AMDGPU][OpenMP] Emit textual IR for -emit-llvm -S

pdhaliwal created this revision. pdhaliwal added reviewers: jdoerfert, JonChesterfield, ronlieb, gregrodgers. Herald added subscribers: kerbowa, guansong, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. pdhaliwal requested review of this revision. Herald added subscribers: cfe-commits,

[PATCH] D102065: [AMDGPU][OpenMP] Emit textual IR for -emit-llvm -S

pdhaliwal added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4397 JA.getType() == types::TY_LTO_BC) { - CmdArgs.push_back("-emit-llvm-bc"); + // Emit textual llvm IR for AMDGPU offloading for -emit-llvm -S + if (Triple.isAMDGCN

[PATCH] D102067: [amdgpu-arch] Guard hsa.h with __has_include

pdhaliwal created this revision. pdhaliwal added reviewers: jdoerfert, JonChesterfield, ronlieb, davezarzycki. Herald added subscribers: kerbowa, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. pdhaliwal requested review of this revision. Herald added subscribers: cfe-commits, wdng. He

[PATCH] D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed

pdhaliwal added a comment. I have put up a patch D102067 which uses __has_include as a workaround for header not found issue. @davezarzycki can you check if this resolves the issue? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D102067: [amdgpu-arch] Guard hsa.h with __has_include

pdhaliwal updated this revision to Diff 343668. pdhaliwal added a comment. Added fallback in case __has_include is not defined or header is not found anywhere. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102067/new/ https://reviews.llvm.org/D102

[PATCH] D102067: [amdgpu-arch] Guard hsa.h with __has_include

This revision was automatically updated to reflect the committed changes. Closed by commit rGc711aa0f6f9d: [amdgpu-arch] Guard hsa.h with __has_include (authored by pdhaliwal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102067/new/ https://review

[PATCH] D102065: [AMDGPU][OpenMP] Emit textual IR for -emit-llvm -S

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7f78e409d028: [AMDGPU][OpenMP] Emit textual IR for -emit-llvm -S (authored by pdhaliwal). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D102065: [AMDGPU][OpenMP] Emit textual IR for -emit-llvm -S

pdhaliwal added a comment. This is not working as expected. And has resulted in broken libomptarget tests. Reverting this until I find a different fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102065/new/ https://reviews.llvm.org/D102065 ___

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

pdhaliwal added a comment. And, I am seeing a lot of failures on nvptx machine (sm_70, cuda11.4) with this patch, libomptarget :: nvptx64-nvidia-cuda :: offloading/bug49021.cpp libomptarget :: nvptx64-nvidia-cuda :: offloading/bug49334.cpp libomptarget :: nvptx64-nvidia-cuda :: offloading/

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfc12a64ecc71: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn (authored by pdhaliwal). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D99402: [AMDGPU][OpenMP] Add /include to the search path

pdhaliwal created this revision. pdhaliwal added reviewers: ronlieb, JonChesterfield, jdoerfert. Herald added subscribers: kerbowa, guansong, t-tye, tpr, dstuttard, yaxunl, nhaehnle, jvesely, kzhuravl. pdhaliwal requested review of this revision. Herald added subscribers: cfe-commits, sstefan1, wd

[PATCH] D99402: [AMDGPU][OpenMP] Add /include to the search path

pdhaliwal added a comment. I was not aware that it was a general problem. I will check if I could get a general solution. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99402/new/ https://reviews.llvm.org/D99402 ___

[PATCH] D99402: [AMDGPU][OpenMP] Add /include to the search path

pdhaliwal added a comment. The general problem seems bit more involved. I am not that familiar with how other architectures/systems handle the library/include path. Simplest solution that I can propose right now is to generalise my revision to other architectures for header lookup and similarl

[PATCH] D99949: [AMDGPU][OpenMP] Add amdgpu-arch tool to list AMD GPUs installed

pdhaliwal created this revision. pdhaliwal added reviewers: JonChesterfield, ronlieb, jdoerfert, ABataev, gregrodgers. Herald added subscribers: kerbowa, guansong, t-tye, tpr, dstuttard, yaxunl, mgorny, nhaehnle, jvesely, kzhuravl. pdhaliwal requested review of this revision. Herald added subscri

  1   2   >