Hi Paul-Antoine!
On 2026-07-01T16:57:43+0200, Paul-Antoine Arras <[email protected]> wrote:
> While working on a follow-up patch, I realised that I did not include
> target-side versions of GOMP_distribute_static_worksharing_start et al.
> The attached fixup patch does so and adds testcases.
ACK.
> On 30/06/2026 14:56, Paul-Antoine Arras wrote:
>> Add two new compiler flags, -fopenmp-ompt and -fopenmp-ompt-detailed, that
>> cause the compiler to emit OMPT-aware runtime entry points around OpenMP
>> worksharing constructs. With -fopenmp-ompt, the _start and _end variants
>> are called instead of the plain worksharing functions; with
>> -fopenmp-ompt-detailed, a _dispatch stub is additionally called after the
>> chunk bounds are computed. New stubs are added to libgomp as placeholders
>> for future OMPT callback invocations.
Thanks! I've not verified your implementation vs. what OpenMP/OMPT
requires -- I assume you and Tobias have sorted that out.
>> --- gcc/common.opt
>> +++ gcc/common.opt
>> @@ -2503,6 +2503,13 @@ fomit-frame-pointer
>> Common Var(flag_omit_frame_pointer) Optimization
>> When possible do not generate stack frames.
>>
>> +fopenmp-ompt
>> +Common Var(flag_openmp_ompt) EnabledBy(fopenmp-ompt-detailed)
>> +
>> +
>> +fopenmp-ompt-detailed
>> +Common Var(flag_openmp_ompt_detailed)
>> +
>> fopenmp-target-simd-clone
>> Common Alias(fopenmp-target-simd-clone=,any,none)
Some short "help text" for the new options, please.
Also, need documenting in 'gcc/doc/invoke.texi', in the "Option Summary"
as well as "OpenMP and OpenACC Options". Probably don't need anything
additional in 'gcc/fortran/gfortran.texi'.
We have '-fopenmp', '-fopenmp-simd' defined in 'gcc/c-family/c.opt',
'gcc/fortran/lang.opt', as, from the user perspective, they only apply to
those respective front ends. I did wonder whether we shouldn't handle
'-fopenmp-ompt', '-fopenmp-ompt-detailed' similarly? But on the other
head, those are 'Common' infrastructure, in the sense that OpenMP/OMPT
functionality also applies if OpenMP is used "as an implementation
detail" in GCC, such as when 'Common' '-ftree-parallelize-loops'
synthesizes OpenMP constructs (and therefore implicitly enables OpenMP
support) to parallelize Algol 68 code, for example. In that case, it's
then certainly valid to expect to see OpenMP/OMPT events reported, and
'-fopenmp-ompt', '-fopenmp-ompt-detailed' than likewise apply.
Therefore, it appears correct to have those 'Common' options. (We shall
add a corresponding Algol 68 test case (or other non-C, C++, Fortran, of
course), once that's feasible to do.)
>> --- gcc/omp-builtins.def
>> +++ gcc/omp-builtins.def
>> @@ -445,6 +445,11 @@ DEF_GOMP_BUILTIN (BUILT_IN_GOMP_SECTIONS_END_NOWAIT,
>> BT_FN_VOID, ATTR_NOTHROW_LEAF_LIST)
>> DEF_GOMP_BUILTIN (BUILT_IN_GOMP_SINGLE_START, "GOMP_single_start",
>> BT_FN_BOOL, ATTR_NOTHROW_LEAF_LIST)
>> +DEF_GOMP_BUILTIN (BUILT_IN_GOMP_SINGLE_START_WITH_END,
>> + "GOMP_single_start_with_end", BT_FN_BOOL,
>> + ATTR_NOTHROW_LEAF_LIST)
>> +DEF_GOMP_BUILTIN (BUILT_IN_GOMP_SINGLE_END, "GOMP_single_end", BT_FN_VOID,
>> + ATTR_NOTHROW_LEAF_LIST)
>> DEF_GOMP_BUILTIN (BUILT_IN_GOMP_SINGLE_COPY_START,
>> "GOMP_single_copy_start",
>> BT_FN_PTR, ATTR_NOTHROW_LEAF_LIST)
>> DEF_GOMP_BUILTIN (BUILT_IN_GOMP_SINGLE_COPY_END, "GOMP_single_copy_end",
>> @@ -503,9 +508,29 @@ DEF_GOMP_BUILTIN (BUILT_IN_GOMP_ERROR, "GOMP_error",
>> DEF_GOMP_BUILTIN (BUILT_IN_GOMP_HAS_MASKED_THREAD_NUM,
>> "GOMP_has_masked_thread_num", BT_FN_BOOL_INT,
>> ATTR_CONST_NOTHROW_LEAF_LIST)
>> +DEF_GOMP_BUILTIN (BUILT_IN_GOMP_MASKED_END, "GOMP_masked_end", BT_FN_VOID,
>> + ATTR_NOTHROW_LEAF_LIST)
>> DEF_GOMP_BUILTIN (BUILT_IN_GOMP_LOOP_STATIC_WORKSHARING,
>> "GOMP_loop_static_worksharing", BT_FN_COMPLEX_INT,
>> ATTR_CONST_NOTHROW_LEAF_LIST)
>> +DEF_GOMP_BUILTIN (BUILT_IN_GOMP_LOOP_STATIC_WORKSHARING_START,
>> + "GOMP_loop_static_worksharing_start", BT_FN_COMPLEX_INT,
>> + ATTR_CONST_NOTHROW_LEAF_LIST)
>> +DEF_GOMP_BUILTIN (BUILT_IN_GOMP_LOOP_STATIC_WORKSHARING_END,
>> + "GOMP_loop_static_worksharing_end", BT_FN_VOID,
>> + ATTR_NOTHROW_LEAF_LIST)
>> +DEF_GOMP_BUILTIN (BUILT_IN_GOMP_LOOP_STATIC_WORKSHARING_DISPATCH,
>> + "GOMP_loop_static_worksharing_dispatch", BT_FN_VOID,
>> + ATTR_NOTHROW_LEAF_LIST)
>> DEF_GOMP_BUILTIN (BUILT_IN_GOMP_DISTRIBUTE_STATIC_WORKSHARING,
>> "GOMP_distribute_static_worksharing", BT_FN_COMPLEX_INT,
>> ATTR_CONST_NOTHROW_LEAF_LIST)
>> +DEF_GOMP_BUILTIN (BUILT_IN_GOMP_DISTRIBUTE_STATIC_WORKSHARING_START,
>> + "GOMP_distribute_static_worksharing_start", BT_FN_COMPLEX_INT,
>> + ATTR_CONST_NOTHROW_LEAF_LIST)
>> +DEF_GOMP_BUILTIN (BUILT_IN_GOMP_DISTRIBUTE_STATIC_WORKSHARING_END,
>> + "GOMP_distribute_static_worksharing_end", BT_FN_VOID,
>> + ATTR_NOTHROW_LEAF_LIST)
>> +DEF_GOMP_BUILTIN (BUILT_IN_GOMP_DISTRIBUTE_STATIC_WORKSHARING_DISPATCH,
>> + "GOMP_distribute_static_worksharing_dispatch", BT_FN_VOID,
>> + ATTR_NOTHROW_LEAF_LIST)
All these need prototypes in 'libgomp/libgomp_g.h', too. (This is
especially important for my libgomp/C++ work.) In 'libgomp/libgomp_g.h',
please sort them in the right place: in sections corresponding to their
libgomp implementation file, in order of definition therein.
'GOMP_loop_static_worksharing_dispatch',
'GOMP_distribute_static_worksharing_dispatch' are really just a signal,
but don't transmit any further data into libgomp/OMPT?
>> --- gcc/omp-expand.cc
>> +++ gcc/omp-expand.cc
>> @@ -5201,10 +5201,14 @@ expand_omp_for_static_nochunk (struct omp_region
>> *region,
>> @@ -5969,10 +6019,14 @@ expand_omp_for_static_chunk (struct omp_region
>> *region,
I've not verified the positions of the individual 'GOMP_[...]' calls that
you're synthesizing.
Let me know if anything needs more in-depth review.
>> --- gcc/omp-low.cc
>> +++ gcc/omp-low.cc
>> @@ -8786,7 +8786,9 @@ lower_omp_single_simple (gomp_single *single_stmt,
>> gimple_seq *pre_p)
>> @@ -8923,6 +8925,15 @@ lower_omp_single (gimple_stmt_iterator *gsi_p,
>> omp_context *ctx)
>> @@ -9099,6 +9110,14 @@ lower_omp_master (gimple_stmt_iterator *gsi_p,
>> omp_context *ctx)
Likewise.
>> --- gcc/opts.cc
>> +++ gcc/opts.cc
>> @@ -1538,6 +1538,12 @@ finish_options (struct gcc_options *opts, struct
>> gcc_options *opts_set,
>> " %<-fstrict-flex-arrays%> is not present");
>> }
>>
>> + if ((opts->x_flag_openmp_ompt || opts->x_flag_openmp_ompt_detailed)
>> + && !opts->x_flag_openmp)
>> + error_at (
>> + loc,
>> + "%<-fopenmp-ompt%> and %<-fopenmp-ompt-detailed%> require
>> %<-fopenmp%>");
>> +
>> diagnose_options (opts, opts_set, loc);
>> }
Looks like the appropriate place for these diagnostics.
>> --- libgomp/loop.c
>> +++ libgomp/loop.c
>> @@ -1186,3 +1186,53 @@ GOMP_loop_ordered_guided_next (long *istart, long
>> *iend)
>> return gomp_loop_ordered_guided_next (istart, iend);
>> }
>> #endif
>> +
>> +/* For a worksharing-loop construct with static schedule, return the thread
>> ID
>> + and number of threads packed into a single complex value. */
>> +
>> +_Complex int
>> +GOMP_loop_static_worksharing (void)
>> +{
>> + struct gomp_team *team = gomp_thread ()->ts.team;
>> + unsigned tid = gomp_thread ()->ts.team_id;
>> + unsigned nthreads = team ? team->nthreads : 1;
>> + return nthreads + tid * 1I;
>> +}
>> +
>> +/* OMPT variant enabled by -fopenmp-ompt. */
>> +
>> +_Complex int
>> +GOMP_loop_static_worksharing_start (void)
>> +{
>> + struct gomp_team *team = gomp_thread ()->ts.team;
>> + unsigned tid = gomp_thread ()->ts.team_id;
>> + unsigned nthreads = team ? team->nthreads : 1;
>> + return nthreads + tid * 1I;
>> +}
>> +
>> +/* Stub for OMPT callback enabled by -fopenmp-ompt-detailed. */
>> +
>> +void
>> +GOMP_loop_static_worksharing_dispatch (void)
>> +{}
>> +
>> +/* Stub for OMPT callback enabled by -fopenmp-ompt. */
>> +
>> +void
>> +GOMP_loop_static_worksharing_end (void)
>> +{}
>> +
>> +/* Return true if the current thread number equals TID.
>> + Used to implement the masked construct's filter clause. */
>> +
>> +bool
>> +GOMP_has_masked_thread_num (int tid)
>> +{
>> + return tid == gomp_thread ()->ts.team_id;
>> +}
>> +
>> +/* Stub for OMPT callback enabled by -fopenmp-ompt. */
>> +
>> +void
>> +GOMP_masked_end (void)
>> +{}
Please update 'libgomp/libgomp_g.h' also for the functions you're moving
into 'libgomp/loop.c' from:
>> --- libgomp/parallel.c
>> +++ libgomp/parallel.c
>> @@ -271,27 +271,6 @@ GOMP_cancel (int which, bool do_cancel)
>> return true;
>> }
>>
>> -/* For a worksharing-loop construct with static schedule, return the thread
>> ID
>> - and number of threads packed into a single complex value. */
>> -
>> -_Complex int
>> -GOMP_loop_static_worksharing (void)
>> -{
>> - struct gomp_team *team = gomp_thread ()->ts.team;
>> - unsigned tid = gomp_thread ()->ts.team_id;
>> - unsigned nthreads = team ? team->nthreads : 1;
>> - return nthreads + tid * 1I;
>> -}
>> -
>> -/* Return true if the current thread number equals TID.
>> - Used to implement the masked construct's filter clause. */
>> -
>> -bool
>> -GOMP_has_masked_thread_num (int tid)
>> -{
>> - return tid == gomp_thread ()->ts.team_id;
>> -}
... here.
(I'm not disagreeing -- but was there any specific reason for moving
them?)
>> -
Keep the page break, I guess?
>> /* The public OpenMP API for thread and team related inquiries. */
>>
>> int
> Subject: [PATCH] libgomp: Add target-side versions of
> GOMP_distribute_static_worksharing
ACK.
Grüße
Thomas
