Re: [PATCH] openmp: Do not emit OMPT loop-end call with inscan

Paul-Antoine Arras <[email protected]>
Newsgroups gmane.comp.gcc.patches
Message-ID <[email protected]> 
Hi Tobias,

Thanks for the review.

On 08/08/2026 19:45, Tobias Burnus wrote:
> Paul-Antoine Arras wrote:
>> When the inscan modifier is present on a for construct, a call to GOMP_loop_end
>> (or one of its variants) is already emitted. The extra call to one of the
>> *_worksharing_end functions, enabled by -fopenmp-ompt, is therefore redundant.
> 
> Looking at the code and the dump, I was confused - and I know now why :-)
> 
> 
> With -fopenmp-ompt, the code was (before this patch):
> 
>    D.3286 = __builtin_GOMP_loop_static_worksharing_start (100);
>      ...
>    __builtin_GOMP_loop_end_nowait ();
>    __builtin_GOMP_loop_static_worksharing_end ();
> 
> The 'GOMP_loop_static_worksharing_start' dispatches the ompt_work_loop_static
> callback with ompt_scope_begin
> 
> And both 'GOMP_loop_end_nowait' and 'GOMP_loop_static_worksharing_end'
> dispatch: 'ompt_work_loop_static' with ompt_scope_end.
> 
> [Side remark: the nowait function knows the schedule type, i.e. it
> might also dispatch other callbacks; but for static both are identical.]
> 
> Thus, the patch now removes the call to the latter.
> This now LGTM.
> 
> 
> However, without -fopenmp-ompt, the code looks as follows:
> 
>    D.3282 = __builtin_GOMP_loop_static_worksharing (100);
>    __builtin_GOMP_loop_end_nowait ();
> 
> Both invoke the ompt_work_loop_static callback - the latter, see
> above, with ompt_scope_end.
> 
> However, the former with ompt_scope_beginend. - That's inconsistent!
> If there is an 'end', there needs to be a 'begin' not a 'beginend'!
> 
> Hence: If there is latere a call to GOMP_loop_end_nowait, the first
> call must be to GOMP_loop_static_worksharing_start.
> 
> * * *
> 
>> gcc/ChangeLog:
>>
>> 	* omp-expand.cc (expand_omp_for_static_nochunk): Do not emit
>> 	*_static_worksharing_end call with inscan modifier.
> 
> Spurious '*' at the front of the second line.
> 
> * * *
> 
>> --- a/gcc/omp-expand.cc
>> +++ b/gcc/omp-expand.cc
>> @@ -5610,7 +5610,10 @@ expand_omp_for_static_nochunk (struct omp_region *region,
>>       }
>>   
>>     gsi = gsi_last_nondebug_bb (exit_bb);
>> -  if (flag_openmp_ompt)
>> +  if (flag_openmp_ompt
>> +      && !(fd->have_reductemp
>> +	   || ((fd->have_pointer_condtemp || fd->have_scantemp)
>> +	       && !fd->have_nonctrl_scantemp)))
>>       {
>>         /* Insert call to GOMP_*_static_worksharing_end at the end of exit_bb.
>>          */
>> @@ -6367,7 +6370,7 @@ expand_omp_for_static_chunk (struct omp_region *region,
>>       }
>>   
>>     gsi = gsi_last_nondebug_bb (exit_bb);
>> -  if (flag_openmp_ompt)
>> +  if (flag_openmp_ompt && !(fd->have_reductemp || fd->have_pointer_condtemp))
>>       {
>>         /* Insert call to GOMP_*_static_worksharing_end at the end of exit_bb.
>>          */
> 
> In both functions:
> 
> * If there will be a GOMP_loop_end_nowait call, the *static*_start call has
>    to be called.  [Also without -fopenmp-ompt], otherwise only with -fopenmp-ompt.
> 
> Any maybe also add a comment that - as the call to GOMP_loop_end_nowait implies
> the end of the scope, '_start' has to be used and '_end' can be skipped.
> 
> Maybe, it looks cleaner with some common boolean to handle the check both for the
> adding of _start and (with a !boolean) for (not) adding the _end call?
> 
> * * *
> 
> In libgomp/loop.c, can you add a comment to the non-"_start" function
> that the "_start" one is called if there is a "_end_nowait" call - and note
> at the "_start" function that no _end" is expected if there is an _end_nowait"
> call? – Or something like that?

Please find attached an updated patch.

Cheers,
-- 
PA
v2-0001-openmp-Adjust-calls-to-OMPT-variants-depending-on.patch (text/x-patch, 7.6 KB) 
From 42238c860458c7355cddab45b730ceee78c51d73 Mon Sep 17 00:00:00 2001
From: Paul-Antoine Arras <[email protected]>
Date: Fri, 7 Aug 2026 15:33:37 +0200
Subject: [PATCH v2] openmp: Adjust calls to OMPT variants depending on
 loop-end call

When the inscan modifier is present on a for construct, a call to GOMP_loop_end
(or one of its variants) is already emitted. The extra call to one of the
*_worksharing_end functions, enabled by -fopenmp-ompt, is therefore redundant.

Furthermore, even without -fopenmp-ompt, the inscan modifier implies a call to
GOMP_loop_end which requires the start variant of GOMP_loop_static_worksharing.

gcc/ChangeLog:

	* omp-expand.cc (expand_omp_for_static_nochunk,
	expand_omp_for_static_chunk): Adjust calls to OMPT variants with
	GOMP_loop_end.

gcc/testsuite/ChangeLog:

	* c-c++-common/gomp/scan-8.c: New test.
	* c-c++-common/gomp/scan-9.c: New test.

libgomp/ChangeLog:

	* loop.c (GOMP_loop_static_worksharing_start,
	GOMP_loop_static_worksharing_end): Update comments.
---
 gcc/omp-expand.cc                        | 34 +++++++++++++++++-------
 gcc/testsuite/c-c++-common/gomp/scan-8.c | 22 +++++++++++++++
 gcc/testsuite/c-c++-common/gomp/scan-9.c | 21 +++++++++++++++
 libgomp/loop.c                           |  6 +++--
 4 files changed, 71 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/gomp/scan-8.c
 create mode 100644 gcc/testsuite/c-c++-common/gomp/scan-9.c

diff --git a/gcc/omp-expand.cc b/gcc/omp-expand.cc
index 0d801e0d68a..65f65da4b07 100644
--- a/gcc/omp-expand.cc
+++ b/gcc/omp-expand.cc
@@ -5225,6 +5225,12 @@ expand_omp_for_static_nochunk (struct omp_region *region,
   t = fold_convert (itype, t);
   n = force_gimple_operand_gsi (&gsi, t, true, NULL_TREE, true, GSI_SAME_STMT);
 
+  /* When GOMP_loop_end (or one of its variants) is emitted (e.g. with the
+     inscan modifier), which already implies the end of the scope, _start
+     variants of GOMP builtin calls have to be used and _end can be skipped.  */
+  bool has_gomp_loop_end = fd->have_reductemp
+			   || ((fd->have_pointer_condtemp || fd->have_scantemp)
+			       && !fd->have_nonctrl_scantemp);
   {
     /* Fetch the thread/team id and the number of threads/teams in a single
        call to GOMP_loop_static_worksharing or
@@ -5239,13 +5245,15 @@ expand_omp_for_static_nochunk (struct omp_region *region,
       {
       case GF_OMP_FOR_KIND_FOR:
 	decl = builtin_decl_explicit (
-	  flag_openmp_ompt ? BUILT_IN_GOMP_LOOP_STATIC_WORKSHARING_START
-			   : BUILT_IN_GOMP_LOOP_STATIC_WORKSHARING);
+	  flag_openmp_ompt || has_gomp_loop_end
+	    ? BUILT_IN_GOMP_LOOP_STATIC_WORKSHARING_START
+	    : BUILT_IN_GOMP_LOOP_STATIC_WORKSHARING);
 	break;
       case GF_OMP_FOR_KIND_DISTRIBUTE:
 	decl = builtin_decl_explicit (
-	  flag_openmp_ompt ? BUILT_IN_GOMP_DISTRIBUTE_STATIC_WORKSHARING_START
-			   : BUILT_IN_GOMP_DISTRIBUTE_STATIC_WORKSHARING);
+	  flag_openmp_ompt || has_gomp_loop_end
+	    ? BUILT_IN_GOMP_DISTRIBUTE_STATIC_WORKSHARING_START
+	    : BUILT_IN_GOMP_DISTRIBUTE_STATIC_WORKSHARING);
 	break;
       default:
 	gcc_unreachable ();
@@ -5610,7 +5618,7 @@ expand_omp_for_static_nochunk (struct omp_region *region,
     }
 
   gsi = gsi_last_nondebug_bb (exit_bb);
-  if (flag_openmp_ompt)
+  if (flag_openmp_ompt && !has_gomp_loop_end)
     {
       /* Insert call to GOMP_*_static_worksharing_end at the end of exit_bb.
        */
@@ -6056,6 +6064,10 @@ expand_omp_for_static_chunk (struct omp_region *region,
   n = force_gimple_operand_gsi (&gsi, t, true, NULL_TREE,
 				true, GSI_SAME_STMT);
 
+  /* When GOMP_loop_end (or one of its variants) is emitted (e.g. with the
+     inscan modifier), which already implies the end of the scope, _start
+     variants of GOMP builtin calls have to be used and _end can be skipped.  */
+  bool has_gomp_loop_end = fd->have_reductemp || fd->have_pointer_condtemp;
   {
     /* Fetch the thread/team id and the number of threads/teams in a single
        call to GOMP_loop_static_worksharing or
@@ -6070,13 +6082,15 @@ expand_omp_for_static_chunk (struct omp_region *region,
       {
       case GF_OMP_FOR_KIND_FOR:
 	decl = builtin_decl_explicit (
-	  flag_openmp_ompt ? BUILT_IN_GOMP_LOOP_STATIC_WORKSHARING_START
-			   : BUILT_IN_GOMP_LOOP_STATIC_WORKSHARING);
+	  flag_openmp_ompt || has_gomp_loop_end
+	    ? BUILT_IN_GOMP_LOOP_STATIC_WORKSHARING_START
+	    : BUILT_IN_GOMP_LOOP_STATIC_WORKSHARING);
 	break;
       case GF_OMP_FOR_KIND_DISTRIBUTE:
 	decl = builtin_decl_explicit (
-	  flag_openmp_ompt ? BUILT_IN_GOMP_DISTRIBUTE_STATIC_WORKSHARING_START
-			   : BUILT_IN_GOMP_DISTRIBUTE_STATIC_WORKSHARING);
+	  flag_openmp_ompt || has_gomp_loop_end
+	    ? BUILT_IN_GOMP_DISTRIBUTE_STATIC_WORKSHARING_START
+	    : BUILT_IN_GOMP_DISTRIBUTE_STATIC_WORKSHARING);
 	break;
       default:
 	gcc_unreachable ();
@@ -6367,7 +6381,7 @@ expand_omp_for_static_chunk (struct omp_region *region,
     }
 
   gsi = gsi_last_nondebug_bb (exit_bb);
-  if (flag_openmp_ompt)
+  if (flag_openmp_ompt && !has_gomp_loop_end)
     {
       /* Insert call to GOMP_*_static_worksharing_end at the end of exit_bb.
        */
diff --git a/gcc/testsuite/c-c++-common/gomp/scan-8.c b/gcc/testsuite/c-c++-common/gomp/scan-8.c
new file mode 100644
index 00000000000..cff8c03b349
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/gomp/scan-8.c
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-fopenmp-ompt -fdump-tree-ompexp" } */
+
+/* Check that an extra, OMPT variant of GOMP_loop_end is not emitted when the
+   inscan modifier is present on the for construct.  */
+
+#define N 100
+
+void f(void) {
+    int a[N], b[N];
+    int x = 0;
+
+#pragma omp parallel for simd reduction(inscan, +: x)
+    for (int k = 0; k < N; k++) {
+        x += a[k];
+#pragma omp scan inclusive(x)
+        b[k] = x;
+    }
+}
+
+/* { dg-final { scan-tree-dump "__builtin_GOMP_loop_end_nowait \\(" "ompexp" } } */
+/* { dg-final { scan-tree-dump-not "__builtin_GOMP_loop_end_nowait \\(\\);\[\t\n \]*__builtin_GOMP_loop_static_worksharing_end \\(\\);" "ompexp" } } */
diff --git a/gcc/testsuite/c-c++-common/gomp/scan-9.c b/gcc/testsuite/c-c++-common/gomp/scan-9.c
new file mode 100644
index 00000000000..f0335536339
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/gomp/scan-9.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-fdump-tree-ompexp" } */
+
+/* Check that the start variant of GOMP_loop_static_worksharing is emitted
+   when the inscan modifier is present on the for construct.  */
+
+#define N 100
+
+void f(void) {
+    int a[N], b[N];
+    int x = 0;
+
+#pragma omp parallel for simd reduction(inscan, +: x)
+    for (int k = 0; k < N; k++) {
+        x += a[k];
+#pragma omp scan inclusive(x)
+        b[k] = x;
+    }
+}
+
+/* { dg-final { scan-tree-dump "__builtin_GOMP_loop_static_worksharing_start \\(" "ompexp" } } */
diff --git a/libgomp/loop.c b/libgomp/loop.c
index 0692663215f..7474279fcd6 100644
--- a/libgomp/loop.c
+++ b/libgomp/loop.c
@@ -1201,7 +1201,8 @@ GOMP_loop_static_worksharing (unsigned long long niter
   return nthreads + tid * 1I;
 }
 
-/* OMPT variant enabled by -fopenmp-ompt.  */
+/* OMPT variant enabled by -fopenmp-ompt and when GOMP_loop_end is called
+   (e.g. with the inscan modifier).  */
 
 _Complex int
 GOMP_loop_static_worksharing_start (unsigned long long niter
@@ -1224,7 +1225,8 @@ GOMP_loop_static_worksharing_dispatch (unsigned long long start
 				       __attribute__ ((unused)))
 {}
 
-/* Stub for OMPT callback enabled by -fopenmp-ompt.  */
+/* Stub for OMPT callback enabled by -fopenmp-ompt, except when GOMP_loop_end is
+   already called (e.g. with the inscan modifier).  */
 
 void
 GOMP_loop_static_worksharing_end (void)
-- 
2.53.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.