Page MenuHomePhabricator

[libomptarget] Refactor activemask macro to inline function
ClosedPublic

Authored by JonChesterfield on Aug 27 2019, 5:55 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptAug 27 2019, 5:55 PM
jdoerfert accepted this revision.Aug 28 2019, 6:58 AM

THX! LGTM.

This revision is now accepted and ready to land.Aug 28 2019, 6:58 AM
ABataev added inline comments.Aug 28 2019, 7:13 AM
openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
46 ↗(On Diff #217546)

I would suggest to change the types of all variables used as mask to __kmpc_impl_lanemask_t, just like for this function.

JonChesterfield marked an inline comment as done.Aug 28 2019, 7:26 AM
JonChesterfield added inline comments.
openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h
46 ↗(On Diff #217546)

That sounds good. I'll rebase this on master (that'll catch syncwarp) and look through the codebase for int/int32_t/uint32_t that are obviously lane masks.

This means the lanemask type will be in the extern API - nothing will break as it's still an i32, and that'll be helpful when targeting a 64 bit warp from clang.

  • uint32 -> lanemask_t
JonChesterfield marked an inline comment as done.
  • keep ULL type in IsWarpMasterActiveThread

Replace types with __kmpc_impl_lanemask_t wherever they are used within a function.

We probably want to change some of the function prototypes to take this type as well but I'd like to make that change separately.

jdoerfert accepted this revision.Sep 3 2019, 8:33 AM

This still looks good to me :)

Thanks guys. It's nice to see the last function style macros disappear from omptarget-nvptx. This means all the CUDA_VERSION tests are now localised to target_impl.h,

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 3 2019, 9:30 AM