dreachem added subscribers: dhruvachak, dreachem.
dreachem added a comment.
Herald added a subscriber: MaskRay.
Herald added a project: All.
@jdoerfert @tianshilei1992 @atmnpatel @dhruvachak
Is the target to get this merged in for LLVM 16? Does the VGPU implementation
provide a way to support OM
jdoerfert added a comment.
We can merge runtime first, build it in isolation, then libomptarget host
runtime, then clang.
Also make sure to adjust the commit messages
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113359/new/
https://reviews.llvm.
tianshilei1992 added a comment.
Not sure if it's good to merge such a large patch. We could potentially split
the patch to three independent patches: tool chain, device runtime, and the
OpenMPOpt pass to support expansion of shared variable (which for some reason
is not included in this patch.
jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.
LG, with some things to address before the merge though.
Didn't we have a pass to expand shared memory (and such)?
Comment at: clang/lib/Basic/TargetInfo.cpp:155
+
+
atmnpatel added inline comments.
Comment at: openmp/libomptarget/test/CMakeLists.txt:23
+continue()
+ ENDIF()
string(STRIP "${CURRENT_TARGET}" CURRENT_TARGET)
jdoerfert wrote:
> This is to disable the tests? Not sure this is a good way though. For one,
>
atmnpatel updated this revision to Diff 405407.
atmnpatel marked 7 inline comments as done.
atmnpatel added a comment.
updates
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113359/new/
https://reviews.llvm.org/D113359
Files:
clang/lib/Basic/Targ
jdoerfert added inline comments.
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:3082
+ if (getTriple().getVendor() == llvm::Triple::OpenMP_VGPU) {
+std::string BitcodeSuffix = "x86_64-vgpu";
+clang::driver::tools::addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args,
---
atmnpatel added inline comments.
Comment at: openmp/libomptarget/DeviceRTL/CMakeLists.txt:231
+
+compileDeviceRTLLibrary(x86_64 vgpu -target x86_64-vgpu -std=c++20
-stdlib=libc++ -I${devicertl_base_directory}/../plugins/vgpu/src)
tianshilei1992 wrote:
> It's not
atmnpatel updated this revision to Diff 401112.
atmnpatel marked 9 inline comments as done.
atmnpatel added a comment.
Addressed comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113359/new/
https://reviews.llvm.org/D113359
Files:
clang/lib
jdoerfert added inline comments.
Comment at: openmp/libomptarget/DeviceRTL/src/Kernel.cpp:127
+#pragma omp begin declare variant match(
\
+device = {kind(cpu)}, implementation = {extension(match_any)})
tianshilei1992 w
tianshilei1992 added inline comments.
Comment at: openmp/libomptarget/DeviceRTL/CMakeLists.txt:231
+
+compileDeviceRTLLibrary(x86_64 vgpu -target x86_64-vgpu -std=c++20
-stdlib=libc++ -I${devicertl_base_directory}/../plugins/vgpu/src)
It's not a good practice to
jdoerfert added inline comments.
Comment at: llvm/lib/Support/Triple.cpp:512
+ .Case("oe", Triple::OpenEmbedded)
+ .Case("vgpu", Triple::OpenMP_VGPU)
+ .Default(Triple::UnknownVendor);
Comment at: openmp/libomptarget/DeviceRTL/s
atmnpatel updated this revision to Diff 398370.
atmnpatel added a comment.
- Fixed lifetime issue around ffi_call
- Addressed comments
The existing x86 plugin uses ffi, so this does as well, no explicit benefit in
doing so. Is it worth keeping?
Repository:
rG LLVM Github Monorepo
CHANGES SI
JonChesterfield added a comment.
I can't see it in the diff - does the cmake somewhere enable the existing tests
on this new target?
A bit surprised to see ffi involved, are we thinking of spawning a separate
process for the target?
Comment at: clang/lib/Basic/Targets/X86.h:
tianshilei1992 added inline comments.
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:3082
+ if (getTriple().getVendor() == llvm::Triple::OpenMP_VGPU) {
+std::string BitcodeSuffix = "x86_64-vgpu";
+clang::driver::tools::addOpenMPDeviceRTL(getDriver(), DriverArgs, CC1Args
atmnpatel updated this revision to Diff 386426.
atmnpatel added a comment.
small nit fix
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113359/new/
https://reviews.llvm.org/D113359
Files:
clang/lib/Basic/Targets/X86.h
clang/lib/CodeGen/CGOpenMP
atmnpatel updated this revision to Diff 386425.
atmnpatel added a comment.
I removed the shared var opt - might be best to keep this in a separate patch
@tianshilei1992. Also addressed comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113359/
jdoerfert added inline comments.
Comment at: clang/lib/CodeGen/CGOpenMPRuntimeVirtualGPU.cpp:54
+ CGOpenMPRuntime::createOffloadEntry(ID, Addr, Size, Flags, Linkage);
+}
We should be able to get rid of this file (and the cuda/hip) version. Might be
the right ti
atmnpatel created this revision.
atmnpatel added reviewers: jdoerfert, tianshilei1992, JonChesterfield.
Herald added subscribers: ormris, dexonsmith, pengfei, hiraditya, mgorny.
atmnpatel requested review of this revision.
Herald added subscribers: llvm-commits, openmp-commits, cfe-commits, sstefan
19 matches
Mail list logo
