Page MenuHomePhabricator

[OpenMP][Offloading] Use bitset to indicate execution mode instead of value
ClosedPublic

Authored by tianshilei1992 on Sep 18 2021, 5:28 PM.

Details

Summary

The execution mode of a kernel is stored in a global variable, whose value means:

  • 0 - SPMD mode
  • 1 - indicates generic mode
  • 2 - SPMD mode execution with generic mode semantics

We are going to add support for SIMD execution mode. It will be come with another
execution mode, such as SIMD-generic mode. As a result, this value-based indicator
is not flexible.

This patch changes to bitset based solution to encode execution mode. Each
position is:
[0] - generic mode
[1] - SPMD mode
[2] - SIMD mode (will be added later)

In this way, 0x1 is generic mode, 0x2 is SPMD mode, and 0x3 is SPMD mode
execution with generic mode semantics. In the future after we add the support for
SIMD mode, 0b1xx will be in SIMD mode.

Diff Detail

Event Timeline

tianshilei1992 created this revision.Sep 18 2021, 5:28 PM
tianshilei1992 requested review of this revision.Sep 18 2021, 5:28 PM
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptSep 18 2021, 5:28 PM
jdoerfert added inline comments.Sep 19 2021, 10:24 PM
clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
1118

The magic constants need to go into OpenMPConstans.h as enum values.

llvm/lib/Transforms/IPO/OpenMPOpt.cpp
3296

Same here.

openmp/libomptarget/plugins/cuda/src/rtl.cpp
75–76

I think you can include OpenMPConstants.h here.

tianshilei1992 marked 3 inline comments as done.Mon, Sep 20, 12:45 PM

Just more style things, I think the rest is fine.

llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
134

If you copy the LLVM_MARK_AS_BITMASK_ENUM stuff you can actually use the enum class as bitmask w/o the static casts all the time.
I'd also suggest to create the combinations here, so GenericSPMD = Generic | SPMD;.

openmp/libomptarget/plugins/cuda/src/rtl.cpp
861
tianshilei1992 marked 2 inline comments as done.Tue, Sep 21, 9:10 AM
tianshilei1992 added inline comments.
llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
134

That's very nice to know.

tianshilei1992 marked an inline comment as done.Tue, Sep 21, 9:11 AM
tianshilei1992 added inline comments.
llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
132

Do we want to set a NONE = 0x0 here?

It looks better to use enum in this case

This revision is now accepted and ready to land.Wed, Sep 22, 7:15 AM
This revision was landed with ongoing or failed builds.Wed, Sep 22, 8:41 AM
This revision was automatically updated to reflect the committed changes.

This seems to be related to some things failing on amdgpu at present:

Target AMDGPU RTL --> Error wrong exec_mode value specified in HSA code object file: 3

The semantics for this mode seem to have been introduced in D106460, which updated the amdgpu plugin to match. I think applying roughly this patch to the amdgpu plugin will bring it back to working.