[OpenMP] OpenMP ThreadSet clause - basic runtime #144409

Ritanya-B-Bharadwaj · 2025-06-16T18:31:37Z

Initial runtime support for threadset clause in task and taskloop directives [Section 14.8 in in OpenMP 6.0 spec]

Frontend PR- #135807

github-actions · 2025-06-16T18:35:15Z

✅ With the latest revision this PR passed the C/C++ code formatter.

alexey-bataev · 2025-06-17T14:27:50Z

openmp/runtime/src/kmp.h

@@ -2740,7 +2740,8 @@ typedef struct kmp_tasking_flags { /* Total struct must be exactly 32 bits */
  unsigned tasking_ser : 1;
  unsigned task_serial : 1;
  unsigned tasktype : 1;
-  unsigned reserved : 8;
+  unsigned reserved : 7;
+  unsigned free_agent_eligible : 1;


Put free_agent_eligible before reserved

It looks like you need to use reversed order for big endian here

#if OMPX_TASKGRAPH unsigned reserved31 : 4; unsigned onced : 1; #else unsigned reserved31 : 5; #endif unsigned hidden_helper : 1; unsigned target : 1; unsigned native : 1; unsigned freed : 1; unsigned complete : 1; unsigned executing : 1; unsigned started : 1; unsigned team_serial : 1; unsigned tasking_ser : 1; unsigned task_serial : 1; unsigned tasktype : 1; unsigned reserved : 8; unsigned free_agent_eligible : 1; unsigned detachable : 1; unsigned priority_specified : 1; unsigned proxy : 1; unsigned destructors_thunk : 1; unsigned merged_if0 : 1; unsigned final : 1; unsigned tiedness : 1;

jpeyton52 · 2025-06-17T15:54:45Z

openmp/runtime/src/kmp.h

+  unsigned reserved : 7; /* reserved for compiler use */
+  unsigned free_agent_eligible : 1; /* set if task can be executed by a


This needs to be reversed as well.

Shouldn't it be the reverse of the above?

If you take one bit out of a reserved bits, you need to make it before the remaining bits.

If you take one bit out of a reserved bits, you need to make it before the remaining bits.

If you look at the other bits, they are consumed in reverse order for this endianess. I'm still confused by inverting the bit order and not the byte order for endianess, but this is how they seem to be implemented.

oh, that reminds me of the big endian thingy that IBM folks fixed in the past.

/* Compiler flags */ /* Total compiler flags must be 16 bits */ unsigned tiedness : 1; /* 0: task is either tied (1) or untied (0) */ unsigned final : 1; /* 1: task is final(1) so execute immediately */ unsigned merged_if0 : 1; /* 2: no __kmpc_task_{begin/complete}_if0 calls in if0 code path */ unsigned destructors_thunk : 1; /* 3: set if the compiler creates a thunk to invoke destructors from the runtime */ unsigned proxy : 1; /* 4:task is a proxy task (it will be executed outside the context of the RTL) */ unsigned priority_specified : 1; /* 5: set if the compiler provides priority setting for the task */ unsigned detachable : 1; /* 6: 1 == can detach */ unsigned free_agent_eligible : 1; /* 7: set if task can be executed by a free-agent thread */ unsigned reserved : 8; /* reserved for compiler use */ /* Library flags */ /* Total library flags must be 16 bits */ unsigned tasktype : 1; /* task is either explicit(1) or implicit (0) */ unsigned task_serial : 1; // task is executed immediately (1) or deferred (0) unsigned tasking_ser : 1; // all tasks in team are either executed immediately // (1) or may be deferred (0) unsigned team_serial : 1; // entire team is serial (1) [1 thread] or parallel // (0) [>= 2 threads] /* If either team_serial or tasking_ser is set, task team may be NULL */ /* Task State Flags: */ unsigned started : 1; /* 1==started, 0==not started */ unsigned executing : 1; /* 1==executing, 0==not executing */ unsigned complete : 1; /* 1==complete, 0==not complete */ unsigned freed : 1; /* 1==freed, 0==allocated */ unsigned native : 1; /* 1==gcc-compiled task, 0==intel */ unsigned target : 1; unsigned hidden_helper : 1; /* 1 == hidden helper task */ #if OMPX_TASKGRAPH unsigned onced : 1; /* 1==ran once already, 0==never ran, record & replay purposes */ unsigned reserved31 : 4; /* reserved for library use */ #else unsigned reserved31 : 5; /* reserved for library use */ #endif```

shiltian

I don't see any meaningful changes out of this PR at this moment. This should definitely not be merged into the repo given its current shape.

jprotze · 2025-06-17T16:55:20Z

I don't see any meaningful changes out of this PR at this moment. This should definitely not be merged into the repo given its current shape.

This is a compliant stub implementation of the runtime to complement the codegen. There are other openmp features with a runtime stub implementation at a similar QoI level on runtime side (taskwait nowait, target nowait with depend, ...)

shiltian · 2025-06-17T17:05:56Z

I'd assume the runtime support is orthogonal to the front end support. For the runtime support, this is far from what is needed. Just to add one bit w/o anything else is not sufficient. Even if the front end emits code that uses the bit, it will be fine because it is reserved anyway. On the other hand, to add runtime support doesn't require front end support as well.

alexey-bataev · 2025-06-17T17:08:26Z

I'd assume the runtime support is orthogonal to the front end support. For the runtime support, this is far from what is needed. Just to add one bit w/o anything else is not sufficient. Even if the front end emits code that uses the bit, it will be fine because it is reserved anyway. On the other hand, to add runtime support doesn't require front end support as well.

Not quite. If the compiler claims some bits used for some particular purposes, we cannot keep this bit "reserved" in the runtime, it maybe a potential source of bugs in future

shiltian · 2025-06-17T17:14:02Z

I'm not saying we are gonna have that for a long run. Just speaking from the series of patch's perspective. Even if we can't, we can always merge runtime support first, and then do the front end work "later".

jprotze · 2025-06-17T17:46:29Z

The whole bitset got messed up. It used to be 16 bits for compiler use, 16 bits for runtime use:

/* Compiler flags */ /* Total compiler flags must be 16 bits */
  unsigned tiedness : 1; /* task is either tied (1) or untied (0) */
  unsigned final : 1; /* task is final(1) so execute immediately */
  unsigned merged_if0 : 1; /* no __kmpc_task_{begin/complete}_if0 calls in if0
                              code path */
  unsigned destructors_thunk : 1; /* set if the compiler creates a thunk to
                                     invoke destructors from the runtime */
  unsigned proxy : 1; /* task is a proxy task (it will be executed outside the
                         context of the RTL) */
  unsigned priority_specified : 1; /* set if the compiler provides priority
                                      setting for the task */
  unsigned detachable : 1; /* 1 == can detach */
  unsigned reserved : 9; /* reserved for compiler use */

Then someone added hidden_helper to the compiler flags. Is this bit ever set by the compiler, or should it be a runtime flag?

The new flag should definitely be added to the compiler flags partition. From my perspective it should replace the hidden_helper flag and the hidden_helper flag should move to the runtime partition.

jprotze

After reordering the bits, the PR lgtm.

jprotze · 2025-06-17T17:58:12Z

openmp/runtime/src/kmp.h

@@ -2740,7 +2740,8 @@ typedef struct kmp_tasking_flags { /* Total struct must be exactly 32 bits */
  unsigned tasking_ser : 1;
  unsigned task_serial : 1;
  unsigned tasktype : 1;
-  unsigned reserved : 8;
+  unsigned reserved : 7;
+  unsigned free_agent_eligible : 1;


#if OMPX_TASKGRAPH unsigned reserved31 : 4; unsigned onced : 1; #else unsigned reserved31 : 5; #endif unsigned hidden_helper : 1; unsigned target : 1; unsigned native : 1; unsigned freed : 1; unsigned complete : 1; unsigned executing : 1; unsigned started : 1; unsigned team_serial : 1; unsigned tasking_ser : 1; unsigned task_serial : 1; unsigned tasktype : 1; unsigned reserved : 8; unsigned free_agent_eligible : 1; unsigned detachable : 1; unsigned priority_specified : 1; unsigned proxy : 1; unsigned destructors_thunk : 1; unsigned merged_if0 : 1; unsigned final : 1; unsigned tiedness : 1;

jprotze · 2025-06-17T18:00:40Z

openmp/runtime/src/kmp.h

+  unsigned reserved : 7; /* reserved for compiler use */
+  unsigned free_agent_eligible : 1; /* set if task can be executed by a


/* Compiler flags */ /* Total compiler flags must be 16 bits */ unsigned tiedness : 1; /* 0: task is either tied (1) or untied (0) */ unsigned final : 1; /* 1: task is final(1) so execute immediately */ unsigned merged_if0 : 1; /* 2: no __kmpc_task_{begin/complete}_if0 calls in if0 code path */ unsigned destructors_thunk : 1; /* 3: set if the compiler creates a thunk to invoke destructors from the runtime */ unsigned proxy : 1; /* 4:task is a proxy task (it will be executed outside the context of the RTL) */ unsigned priority_specified : 1; /* 5: set if the compiler provides priority setting for the task */ unsigned detachable : 1; /* 6: 1 == can detach */ unsigned free_agent_eligible : 1; /* 7: set if task can be executed by a free-agent thread */ unsigned reserved : 8; /* reserved for compiler use */ /* Library flags */ /* Total library flags must be 16 bits */ unsigned tasktype : 1; /* task is either explicit(1) or implicit (0) */ unsigned task_serial : 1; // task is executed immediately (1) or deferred (0) unsigned tasking_ser : 1; // all tasks in team are either executed immediately // (1) or may be deferred (0) unsigned team_serial : 1; // entire team is serial (1) [1 thread] or parallel // (0) [>= 2 threads] /* If either team_serial or tasking_ser is set, task team may be NULL */ /* Task State Flags: */ unsigned started : 1; /* 1==started, 0==not started */ unsigned executing : 1; /* 1==executing, 0==not executing */ unsigned complete : 1; /* 1==complete, 0==not complete */ unsigned freed : 1; /* 1==freed, 0==allocated */ unsigned native : 1; /* 1==gcc-compiled task, 0==intel */ unsigned target : 1; unsigned hidden_helper : 1; /* 1 == hidden helper task */ #if OMPX_TASKGRAPH unsigned onced : 1; /* 1==ran once already, 0==never ran, record & replay purposes */ unsigned reserved31 : 4; /* reserved for library use */ #else unsigned reserved31 : 5; /* reserved for library use */ #endif```

llvmbot added the openmp:libomp OpenMP host runtime label Jun 16, 2025

Ritanya-B-Bharadwaj mentioned this pull request Jun 16, 2025

[clang][OpenMP] New OpenMP 6.0 threadset clause #135807

Open

Ritanya-B-Bharadwaj requested a review from alexey-bataev June 16, 2025 18:32

Ritanya-B-Bharadwaj force-pushed the threadset-runtime branch from 54f3860 to ca3f29a Compare June 16, 2025 18:39

alexey-bataev reviewed Jun 17, 2025

View reviewed changes

[clang][OpenMP] OpenMP ThreadSet clause - basic runtime

4553bd2

Ritanya-B-Bharadwaj force-pushed the threadset-runtime branch from ca3f29a to 4553bd2 Compare June 17, 2025 15:43

alexey-bataev requested review from shiltian and jdoerfert June 17, 2025 15:44

jpeyton52 reviewed Jun 17, 2025

View reviewed changes

shiltian changed the title ~~[clang][OpenMP] OpenMP ThreadSet clause - basic runtime~~ [OpenMP] OpenMP ThreadSet clause - basic runtime Jun 17, 2025

shiltian requested changes Jun 17, 2025

View reviewed changes

jprotze reviewed Jun 17, 2025

View reviewed changes

		unsigned reserved : 7; /* reserved for compiler use */
		unsigned free_agent_eligible : 1; /* set if task can be executed by a

[OpenMP] OpenMP ThreadSet clause - basic runtime #144409

Are you sure you want to change the base?

[OpenMP] OpenMP ThreadSet clause - basic runtime #144409

Conversation

Ritanya-B-Bharadwaj commented Jun 16, 2025

Uh oh!

github-actions bot commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shiltian left a comment

Choose a reason for hiding this comment

Uh oh!

jprotze commented Jun 17, 2025

Uh oh!

shiltian commented Jun 17, 2025

Uh oh!

alexey-bataev commented Jun 17, 2025

Uh oh!

shiltian commented Jun 17, 2025

Uh oh!

jprotze commented Jun 17, 2025

Uh oh!

jprotze left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jun 16, 2025 •

edited

Loading