public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug other/108749] New: [OpenMP][C/C++/Fortran] inscan reduction modifier rejected for combined/composite constructs of simd/for/do
@ 2023-02-10 10:11 burnus at gcc dot gnu.org
  2023-02-10 10:18 ` [Bug other/108749] " jakub at gcc dot gnu.org
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: burnus at gcc dot gnu.org @ 2023-02-10 10:11 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108749

            Bug ID: 108749
           Summary: [OpenMP][C/C++/Fortran] inscan reduction modifier
                    rejected for combined/composite constructs of
                    simd/for/do
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: openmp, rejects-valid
          Severity: normal
          Priority: P3
         Component: other
          Assignee: unassigned at gcc dot gnu.org
          Reporter: burnus at gcc dot gnu.org
                CC: jakub at gcc dot gnu.org
  Target Milestone: ---

This applies to C, C++ and Fortran likewise.

test.c:6:37: error: ‘inscan’ ‘reduction’ clause on construct other than ‘for’,
‘simd’, ‘for simd’, ‘parallel for’, ‘parallel for simd’

    6 |   #pragma omp target simd reduction (inscan, *:r)
      |                                     ^


OpenMP 5.0 had the following:

"A reduction clause with the *inscan* reduction-modifier may only appear on a
worksharing-loop construct, a worksharing-loop SIMD construct, a simd
construct, a parallel worksharing-loop construct or a parallel worksharing-loop
SIMD construct."

But, a bit confusingly, it also had:

"If a construct to which the *inscan* reduction-modifier is
applied is combined with the *target* construct, the effect is as if the same
list item also appears in a map clause with a map-type of tofrom."

The latter implying that a combined construct is also permitted - while the
former rules it out.

 * * *

OpenMP 5.1 seemingly fixed this while 5.2 removed constructs with 'distribute'.
In any case, OpenMP 5.2 reads as follows:

"A reduction clause with the *inscan* reduction-modifier may only appear on a
worksharing-loop construct, a simd construct or a combined or composite
construct for which any of the aforementioned constructs is a constituent
construct and distribute is not a constituent construct." — ["5.5.8 reduction
Clause" under "Restrictions to the reduction clause are as follows:" (7th
bullet), [136:1-4]]

 * * *

I think this currently implies that the following ones should be supported
besides the exsisting '(parallel) {do|for} (simd)' and 'simd'.

OMP_MASKED_TASKLOOP_SIMD
OMP_MASTER_TASKLOOP_SIMD
OMP_PARALLEL_MASKED_TASKLOOP_SIMD
OMP_PARALLEL_MASTER_TASKLOOP_SIMD
OMP_TARGET_PARALLEL_DO
OMP_TARGET_PARALLEL_DO_SIMD
OMP_TARGET_SIMD
OMP_TASKLOOP_SIMD

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug other/108749] [OpenMP][C/C++/Fortran] inscan reduction modifier rejected for combined/composite constructs of simd/for/do
  2023-02-10 10:11 [Bug other/108749] New: [OpenMP][C/C++/Fortran] inscan reduction modifier rejected for combined/composite constructs of simd/for/do burnus at gcc dot gnu.org
@ 2023-02-10 10:18 ` jakub at gcc dot gnu.org
  2023-02-10 10:27 ` jakub at gcc dot gnu.org
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-02-10 10:18 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108749

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
We implement the 5.0 wording which was quite clear that only the selected
combined/composite constructs are allowed for it.  The clause handling wording
was added without considering the former (it wasn't initially there I believe).

Next step would be implement the 5.2 wording, which would be support those also
on
   #pragma omp masked taskloop simd
   #pragma omp master taskloop simd
   #pragma omp parallel masked taskloop simd
   #pragma omp parallel master taskloop simd
   #pragma omp target parallel for
   #pragma omp target parallel for simd
   #pragma omp target simd
   #pragma omp taskloop simd
I guess we could even do it for GCC 13, it is a simple change.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug other/108749] [OpenMP][C/C++/Fortran] inscan reduction modifier rejected for combined/composite constructs of simd/for/do
  2023-02-10 10:11 [Bug other/108749] New: [OpenMP][C/C++/Fortran] inscan reduction modifier rejected for combined/composite constructs of simd/for/do burnus at gcc dot gnu.org
  2023-02-10 10:18 ` [Bug other/108749] " jakub at gcc dot gnu.org
@ 2023-02-10 10:27 ` jakub at gcc dot gnu.org
  2023-02-10 11:00 ` burnus at gcc dot gnu.org
  2023-02-10 11:11 ` jakub at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-02-10 10:27 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108749

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Actually, I don't see how inscan be implemented on taskloop, so I'd say both
5.1 and 5.2 are wrong and it should be neither distribute nor taskloop are
constituent.
   #pragma omp target parallel for
   #pragma omp target parallel for simd
   #pragma omp target simd
should work fine, those just map(tofrom:) the reduction variable.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug other/108749] [OpenMP][C/C++/Fortran] inscan reduction modifier rejected for combined/composite constructs of simd/for/do
  2023-02-10 10:11 [Bug other/108749] New: [OpenMP][C/C++/Fortran] inscan reduction modifier rejected for combined/composite constructs of simd/for/do burnus at gcc dot gnu.org
  2023-02-10 10:18 ` [Bug other/108749] " jakub at gcc dot gnu.org
  2023-02-10 10:27 ` jakub at gcc dot gnu.org
@ 2023-02-10 11:00 ` burnus at gcc dot gnu.org
  2023-02-10 11:11 ` jakub at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: burnus at gcc dot gnu.org @ 2023-02-10 11:00 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108749

--- Comment #3 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #2)
> Actually, I don't see how inscan be implemented on taskloop

The proposed extension of the restriction is now tracked in the OpenMP
specification Issue 3489.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Bug other/108749] [OpenMP][C/C++/Fortran] inscan reduction modifier rejected for combined/composite constructs of simd/for/do
  2023-02-10 10:11 [Bug other/108749] New: [OpenMP][C/C++/Fortran] inscan reduction modifier rejected for combined/composite constructs of simd/for/do burnus at gcc dot gnu.org
                   ` (2 preceding siblings ...)
  2023-02-10 11:00 ` burnus at gcc dot gnu.org
@ 2023-02-10 11:11 ` jakub at gcc dot gnu.org
  3 siblings, 0 replies; 5+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-02-10 11:11 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108749

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Perhaps it is implementable also for taskloop, but with a lot of work.
The way how e.g. for/do works with inscan is that the two parts of the loop are
split up, and one essentially gets two worksharing loops with the same number
of iterations, one doing one part, then some single (or in parallel)
middle-part and then another doing the other part.
With tasks, perhaps we could create separate tasks for the two halves, spawn a
taskloop that does one part, then a task that depends on all those tasks and
does the merging in the middle and finally another taskloop that does the other
part.
But I think this is something that hasn't been even considered when just
tweaking the wording.  After all, if we want to allow inscan on taskloop simd
and constructs combined with that, we'd first want to allow it on taskloop
itself.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-02-10 11:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-10 10:11 [Bug other/108749] New: [OpenMP][C/C++/Fortran] inscan reduction modifier rejected for combined/composite constructs of simd/for/do burnus at gcc dot gnu.org
2023-02-10 10:18 ` [Bug other/108749] " jakub at gcc dot gnu.org
2023-02-10 10:27 ` jakub at gcc dot gnu.org
2023-02-10 11:00 ` burnus at gcc dot gnu.org
2023-02-10 11:11 ` jakub at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).