This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] add 4 custom APIs supporting MSVC OMP codegen
ClosedPublic

Authored by vadikp-intel on Jun 22 2022, 5:34 PM.

Details

Summary

This check-in adds 4 APIs to support MSVC, specifically:

  1. 3 APIs (kmpc_sections_init, kmpc_next_section, __kmpc_end_sections) to support the dynamic scheduling of OMP sections.
  2. 1 API (kmpc_copyprivate_light, a light-weight version of kmpc_copyrprivate) to support the OMP single copyprivate clause.

Diff Detail

Event Timeline

vadikp-intel created this revision.Jun 22 2022, 5:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 22 2022, 5:34 PM
vadikp-intel requested review of this revision.Jun 22 2022, 5:34 PM
Herald added a project: Restricted Project. · View Herald Transcript

Hi Vadim!

Can you add maximum amount of context to patches: https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

To get a full diff, use one of the following commands

  • git show HEAD -U999999 > mypatch.patch
  • git diff -U999999 u > mypatch.patch
  • git diff HEAD~1 -U999999 > mypatch.patch
openmp/runtime/src/kmp_dispatch.cpp
2288

Need this to be /*!

2400

This comes up as unused because it is only in KMP_DEBUG_ASSERT() macros. Can you either change the team usages to th->th.th_team in the KMP_DEBUG_ASSERT() or have

#ifdef KMP_DEBUG
kmp_team_t *team = th->th.th_team;
#endif
protze.joachim added inline comments.
openmp/runtime/src/kmp_csupport.cpp
2279–2293

This should all go away with removing the second barrier, I think.

openmp/runtime/src/kmp_dispatch.cpp
2312–2314

The value is only used locally, so rather use OMPT_GET_RETURN_ADDRESS(0) instead of OMPT_LOAD_RETURN_ADDRESS(gtid)) below and remove this block.

2457–2465

This should be an ompt_callback_dispatch. The callback for end of sections is already in below function.

2485–2487

The value is only used locally, so rather use OMPT_GET_RETURN_ADDRESS(0) instead of OMPT_LOAD_RETURN_ADDRESS(gtid)) below and remove this block.

vadikp-intel marked 6 inline comments as done.

Addressed review comments

This comment was removed by vadikp-intel.

fixed a build problem

clang-formatted

jlpeyton accepted this revision.Jul 1 2022, 12:22 PM

LGTM, @protze.joachim , are you satisfied with the changes?

This revision is now accepted and ready to land.Jul 1 2022, 12:22 PM
protze.joachim accepted this revision.Jul 2 2022, 1:13 AM

Just one more change. Then the ompt part looks good to me.

openmp/runtime/src/kmp_dispatch.cpp
2390

This block needs to go away, because the value is not consumed anywhere.

Removed dead code in __kmpc_next_section.

vadikp-intel marked an inline comment as done.Jul 2 2022, 11:05 AM

Johnny,

I have removed the dead code as Joachim requested, and the builds have passed. Could you complete the check-in (I have no write access) If everything looks good?

Thanks,

Vadim

This revision was automatically updated to reflect the committed changes.