This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] libomp: implemented task priorities.
ClosedPublic

Authored by AndreyChurbanov on Feb 13 2022, 12:44 PM.

Details

Summary

Implemented shared list of task deques one per task priority value.
Tasks execution changed to first check if priority tasks ready for execution exist,
and these tasks executed before others; otherwise usual tasks execution mechanics work.

Diff Detail

Event Timeline

AndreyChurbanov requested review of this revision.Feb 13 2022, 12:44 PM
cchen added a subscriber: cchen.Mar 2 2022, 7:33 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2022, 7:33 AM

Few comments below, and just for clarification, is it correct that priority==0 means "use the regular per-thread deques" and are assumed to be lowest (and default) priority?

openmp/runtime/src/kmp_tasking.cpp
336

I know this function returns a structure called kmp_thread_data_t here, but it would be easier to read if it was named __kmp_get_priority_deque() or similar since that is the really the semantics. I thought the priority queues were per-thread until looking closely.

344

"Existed queues keep" -> "All current priority queues contain"

Maybe just outright mention the list of priority queues are sorted from high -> low.

For the test I suggest signalling like used for OMPT tests (see openmp/runtime/tests/ompt/ompt_signal.h) to enforce the execution ordering rather than sleep "synchronization".

openmp/runtime/test/tasking/omp_task_priority2.c
61

Since you already have the shared atomic variable passed, you can wait for the expected value in this variable.

102–104

Similar for the second parallel region

... just for clarification, is it correct that priority==0 means "use the regular per-thread deques" and are assumed to be lowest (and default) priority?

Yes. The description of the priority clause says: "The default priority-value when no priority clause is specified is zero (the lowest priority)."

Addressed comments:

  • renamed auxiliary function and added comment with its description;
  • fixed test to use signaling instead of potentially unsafe delays.
AndreyChurbanov marked 4 inline comments as done.Mar 5 2022, 12:38 PM
jlpeyton accepted this revision.Mar 7 2022, 8:21 AM

LGTM

This revision is now accepted and ready to land.Mar 7 2022, 8:21 AM
This revision was automatically updated to reflect the committed changes.