This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP][WIP] `spir64` device runtime library template
Needs ReviewPublic

Authored by spoutn1k on Feb 27 2023, 10:34 AM.

Details

Reviewers
jdoerfert

Diff Detail

Event Timeline

spoutn1k created this revision.Feb 27 2023, 10:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 27 2023, 10:34 AM
spoutn1k requested review of this revision.Feb 27 2023, 10:34 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 27 2023, 10:34 AM
spoutn1k added inline comments.Feb 27 2023, 10:43 AM
openmp/libomptarget/DeviceRTL/src/State.cpp
69–86

This is only defined for AMDGCN with a TODO, in a way that seems to overwrite the default malloc. Should I do the same for this implementation ?

openmp/libomptarget/DeviceRTL/src/Utils.cpp
87–91

Found shuffle below in the spirv interface but not pack. Should I start using bitwise operation the way it is done for AMD ?

Can you update the patch with context?

jdoerfert added inline comments.Feb 27 2023, 11:20 AM
openmp/libomptarget/DeviceRTL/src/State.cpp
69–86

On AMDGPU there is no "default malloc". On NVPTX there is, on Intel we need to check. You can probably go with the AMD route for now.

openmp/libomptarget/DeviceRTL/src/Utils.cpp
87–91

There is a default implementation for Pack and Unpack, you can just not overwrite them. I think that is what we do for AMD.

spoutn1k updated this revision to Diff 500863.Feb 27 2023, 11:42 AM

Added context

spoutn1k updated this revision to Diff 500943.Feb 27 2023, 3:30 PM

Add pack/unpack and malloc/free for spir64 target runtime implementation.

tianshilei1992 added inline comments.Mar 1 2023, 4:54 PM
llvm/lib/Frontend/OpenMP/OMPContext.cpp
55

SPIR-V can be for host as well right? OpenCL CPU runtime can also take SPIR-V.

openmp/libomptarget/DeviceRTL/src/LibC.cpp
37

I'm thinking potentially we can have a generic implementation of omp_vprintf that just returns -1 and having the NVPTX version calls vprintf. In this way we don't need to do it for every target.

spoutn1k marked 2 inline comments as done.Mar 6 2023, 9:52 AM
spoutn1k added inline comments.
llvm/lib/Frontend/OpenMP/OMPContext.cpp
55

It can ! The objective of this work is to use it with intel gpus. I put it there in order to have it understand the variants in DeviceRTL. Is there a way to signify both host and device ?

openmp/libomptarget/DeviceRTL/src/LibC.cpp
37

How do I go about doing this ? Declare omp_vprintf as weak ?

spoutn1k updated this revision to Diff 503534.Mar 8 2023, 3:02 PM
spoutn1k marked an inline comment as not done.

Added contents of DeviceRTL/src/Mapping.cpp; ignored activemask for the time being.

spoutn1k updated this revision to Diff 503991.Mar 9 2023, 5:39 PM
spoutn1k edited the summary of this revision. (Show Details)
spoutn1k edited the summary of this revision. (Show Details)
tianshilei1992 added inline comments.Mar 9 2023, 5:45 PM
openmp/libomptarget/DeviceRTL/src/LibC.cpp
37

just something like:

int32_t omp_vprintf(const char *Format, void *Arguments, uint32_t) {
  return -1;
}
#pragma omp begin declare variant match(device = {arch(nvptx64)})
int32_t omp_vprintf(const char *Format, void *Arguments, uint32_t) {
  return vprintf(...);
}
#pragma omp end declare variant

In this way, when the arch is NVPTX64, that variant will replace the default one.

openmp/libomptarget/DeviceRTL/src/Mapping.cpp
162

Do we have these builtins in LLVM right now?

spoutn1k added inline comments.Mar 9 2023, 5:49 PM
openmp/libomptarget/DeviceRTL/src/LibC.cpp
37

Oh I see. I was under the impression this would create a symbol conflict. Will do !

openmp/libomptarget/DeviceRTL/src/Mapping.cpp
162

That was my question but Johannes told me to go ahead and use them.

spoutn1k updated this revision to Diff 509478.Mar 29 2023, 2:23 PM

Defined spirv methods as extern and defined spirv enums.

Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2023, 2:23 PM