[PATCH] D147666: [OPENMP] Adds /lib to rpath to avoid need to set LD_LIBRARY_PATH to find plugins.

gregrodgers abandoned this revision. gregrodgers added a comment. Its ugly, but to avoid requirement to set LD_LIBRARY_PATH for end-users who may need LD_LIBRARY_PATH for their own application, we will modify the compiler installation with these bash commands where _INSTALL_DIR is the installati

[PATCH] D147666: [OPENMP] Adds /lib to rpath to avoid need to set LD_LIBRARY_PATH to find plugins.

gregrodgers created this revision. Herald added subscribers: sunshaoce, guansong, yaxunl. Herald added a project: All. gregrodgers requested review of this revision. Herald added subscribers: cfe-commits, jplehr, sstefan1, MaskRay. Herald added a reviewer: jdoerfert. Herald added a project: clang.

[PATCH] D146075: [flang][driver][openmp] Write MLIR for -save-temps

gregrodgers accepted this revision. gregrodgers added a comment. This looks good. Please merge. I found it very useful especially in the context of other generated temp files generated after llvm llinking and optimization in the offload driver. For example just listing the temp files with ls

[PATCH] D114890: [OpenMP] Make the new device runtime the default

gregrodgers accepted this revision. gregrodgers added a comment. This revision is now accepted and ready to land. this is ok as is Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114890/new/ https://reviews.llvm.org/D114890 _

[PATCH] D114890: [OpenMP] Make the new device runtime the default

gregrodgers requested changes to this revision. gregrodgers added a comment. This revision now requires changes to proceed. I forgot to add the "request changes" action. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114890/new/ https://reviews.llvm

[PATCH] D114890: [OpenMP] Make the new device runtime the default

gregrodgers added a comment. We want amdgcn to remain on old deviceRTL till we have verified it . I made inline comments on how this could be done. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5905 // runtime. if (Args.hasFlag(options::OPT_fopenmp_target_n

[PATCH] D99235: [HIP] Change to code object v4

gregrodgers added inline comments. Comment at: clang/lib/Driver/ToolChains/HIP.cpp:116 + if (getOrCheckAMDGPUCodeObjectVersion(C.getDriver(), Args) >= 4) +OffloadKind = OffloadKind + "v4"; for (const auto &II : Inputs) { yaxunl wrote: > tra wrote: > > We

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

gregrodgers accepted this revision. gregrodgers added a comment. This revision is now accepted and ready to land. I am removing my objection with the understanding that we will either replace or enhance amdgpu-arch with the cross-architecture tool offload-arch as described in my comments above.

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

gregrodgers added inline comments. Comment at: clang/tools/amdgpu-arch/CMakeLists.txt:9 + +find_package(hsa-runtime64 QUIET 1.2.0 HINTS ${CMAKE_INSTALL_PREFIX} PATHS /opt/rocm) +if (NOT ${hsa-runtime64_FOUND}) What happens when /opt/rocm is not available? Agai

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

gregrodgers added a comment. Dependence on hsa is not necessary. The amdgpu and nvidia drivers both use PCI codes available in /sys . We should use architecture independent methods as much as possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

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

gregrodgers requested changes to this revision. gregrodgers added a comment. This revision now requires changes to proceed. I have two serious concerns with this tool . 1. It does not provide the infrastructure to identify runtime capabilities to satisfy requirements of a compiled image. 2.

[PATCH] D84743: [Clang][AMDGCN] Universal device offloading macros header

gregrodgers added a comment. This is all excellent feedback. Thank you. I don't understand what I see on the godbolt link. So far, we have only tested with clang. We will test with gcc to understand the fail. I will make the change to use numeric values for _DEVICE_ARCH and change "UNKNO

[PATCH] D76987: Rename options --cuda-gpu-arch and --no-cuda-gpu-arch

gregrodgers added a comment. This was discussed on llvm-dev three years ago. Here is the thread. http://lists.llvm.org/pipermail/llvm-dev/2017-February/109930.html The last name discussed was "-- offload-arch". I don't believe we need a list option anymore. So ignore the very old request f

[PATCH] D50845: [CUDA/OpenMP] Define only some host macros during device compilation

gregrodgers added a comment. I have a longer comment on header files, but let me first understand this patch. IIUC,the concept of this patch is to fake the macros to think it is seeing a host on the device patch. if ((LangOpts.CUDA || LangOpts.OpenMPIsDevice) && PP.getAuxTargetInfo()) Initi

[PATCH] D47849: [OpenMP][Clang][NVPTX] Enable math functions called in an OpenMP NVPTX target device region to be resolved as device-native function calls

gregrodgers added a comment. I like the idea of using an automatic include as a cc1 option (-include). However, I would prefer a more general automatic include for OpenMP, not just for math functions (__clang_cuda_device_functions.h). Clang cuda automatically includes __clang_cuda_runtime_wrap

[PATCH] D48455: Remove hip.amdgcn.bc hc.amdgcn.bc from HIP Toolchains

gregrodgers added a comment. Why not provide a specific list of --hip-device-lib= for VDI builds? I am not sure about defining functions inside headers instead of using a hip bc lib. Repository: rC Clang https://reviews.llvm.org/D48455 ___ cfe

[PATCH] D42800: Let CUDA toolchain support amdgpu target

gregrodgers added a comment. Here my replys to the inline comments. Everything should be fixed in the next revision. Comment at: include/clang/Basic/Cuda.h:79 COMPUTE_72, + COMPUTE_GCN, }; t-tye wrote: > Suggest using amdgcn which matches the architectu

[PATCH] D42800: Let CUDA toolchain support amdgpu target

gregrodgers added a comment. Sorry, all my great inline comments got lost somehow. I am a newbie to Phabricator. I will try to reconstruct my comments. https://reviews.llvm.org/D42800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D42800: Let CUDA toolchain support amdgpu target

gregrodgers requested changes to this revision. gregrodgers added a comment. This revision now requires changes to proceed. Thanks to everyone for the reviews. I hope I replied to all inline comments. Since I sent this to Sam to post, we discovered a major shortcoming. As tra points out, the