GCC 9 OpenMP issues

[Orion Poplawski]

Looks like GCC 9 is finally enforcing an OpenMP change:

 From https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00628.html

- change data sharing for readonly variables without mutable members, 
they are
   no longer predetermined shared (this actually changed in earlier 
OpenMP standard
   releases, but was considered a mistake; for 5.0 it was decided it 
isn't going to
   be reverted; this makes a difference mainly when using default(none))


Which is now leading to some (expected) compilation failures:

BUILDSTDERR: nco_omp.c: In function 'nco_openmp_ini':
BUILDSTDERR: nco_omp.c:210:38: error: 'fp_stderr' not specified in 
enclosing 'parallel'
BUILDSTDERR:   210 |  if(nco_dbg_lvl_get() >= nco_dbg_fl) 
(void)fprintf(fp_stderr,"%s: INFO Small parallel test region spawned 
team of %d thread(s)\n",nco_prg_nm_get(),thr_nbr_act);
BUILDSTDERR:       | 
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
BUILDSTDERR: nco_omp.c:205:10: error: enclosing 'parallel'
BUILDSTDERR:   205 | # pragma omp parallel default(none) shared(thr_nbr_act)
BUILDSTDERR:       |          ^~~


The problem is that it's proving difficult to come up with a solution 
that compiles under both GCC 9 and GCC 8.

GCC 8 complains about just adding fp_stderr to shared():

nco_omp.c: In function 'nco_openmp_ini':
nco_omp.c:205:43: error: 'fp_stderr' is predetermined 'shared' for
'shared'
  # pragma omp parallel default(none) shared(fp_stderr,thr_nbr_act)

And apparently complains if you drop default(none) as well (from another 
project with the same problem):

[  9%] Building CXX object CMakeFiles/_CuraEngine.dir/src/layerPart.cpp.o
/home/ruben/Projects/CuraEngine/src/layerPart.cpp: In function ‘void 
cura::createLayerParts(cura::SliceMeshStorage&, cura::Slicer*)’:
/home/ruben/Projects/CuraEngine/src/layerPart.cpp:52:78: error: 
‘total_layers’ is predetermined ‘shared’ for ‘shared’
  #pragma omp parallel for shared(mesh, slicer, total_layers) 
schedule(dynamic)

https://github.com/nco/nco/issues/114
https://github.com/Ultimaker/CuraEngine/issues/984

Is there any hope of a common solution?

Thanks!

- Orion

-- 
Orion Poplawski
Manager of NWRA Technical Systems          720-772-5637
NWRA, Boulder/CoRA Office             FAX: [phone]
3380 Mitchell Lane                       orion(a)nwra.com
Boulder, CO 80301                 https://www.nwra.com/

[Sérgio Basto]

I also have OpenMP issues in ufraw [1] and can help me too ? 

Thanks

[1]
https://bugzilla.redhat.com/show_bug.cgi?id=1671080
https://koji.fedoraproject.org/koji/taskinfo?taskID=32347242



On Mon, 2019-02-11 at 19:17 -0700, Orion Poplawski wrote:
>  Looks like GCC 9 is finally enforcing an OpenMP change:
>  
>   From https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00628.html
>  
>  - change data sharing for readonly variables without mutable
>  members, 
>  they are
>     no longer predetermined shared (this actually changed in earlier 
>  OpenMP standard
>     releases, but was considered a mistake; for 5.0 it was decided it 
>  isn't going to
>     be reverted; this makes a difference mainly when using
>  default(none))
>  
>  
>  Which is now leading to some (expected) compilation failures:
>  
>  BUILDSTDERR: nco_omp.c: In function 'nco_openmp_ini':
>  BUILDSTDERR: nco_omp.c:210:38: error: 'fp_stderr' not specified in 
>  enclosing 'parallel'
>  BUILDSTDERR:   210 |  if(nco_dbg_lvl_get() >= nco_dbg_fl) 
>  (void)fprintf(fp_stderr,"%s: INFO Small parallel test region spawned 
>  team of %d thread(s)\n",nco_prg_nm_get(),thr_nbr_act);
>  BUILDSTDERR:       | 
>  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  BUILDSTDERR: nco_omp.c:205:10: error: enclosing 'parallel'
>  BUILDSTDERR:   205 | # pragma omp parallel default(none)
>  shared(thr_nbr_act)
>  BUILDSTDERR:       |          ^~~
>  
>  
>  The problem is that it's proving difficult to come up with a
>  solution 
>  that compiles under both GCC 9 and GCC 8.
>  
>  GCC 8 complains about just adding fp_stderr to shared():
>  
>  nco_omp.c: In function 'nco_openmp_ini':
>  nco_omp.c:205:43: error: 'fp_stderr' is predetermined 'shared' for
>  'shared'
>    # pragma omp parallel default(none) shared(fp_stderr,thr_nbr_act)
>  
>  And apparently complains if you drop default(none) as well (from
>  another 
>  project with the same problem):
>  
>  [  9%] Building CXX object
>  CMakeFiles/_CuraEngine.dir/src/layerPart.cpp.o
>  /home/ruben/Projects/CuraEngine/src/layerPart.cpp: In function ‘void 
>  cura::createLayerParts(cura::SliceMeshStorage&, cura::Slicer*)’:
>  /home/ruben/Projects/CuraEngine/src/layerPart.cpp:52:78: error: 
>  ‘total_layers’ is predetermined ‘shared’ for ‘shared’
>    #pragma omp parallel for shared(mesh, slicer, total_layers) 
>  schedule(dynamic)
>  
>  https://github.com/nco/nco/issues/114
>  https://github.com/Ultimaker/CuraEngine/issues/984
>  
>  Is there any hope of a common solution?
>  
>  Thanks!
>  
>  - Orion
>  
>  -- 
>  Orion Poplawski
>  Manager of NWRA Technical Systems          720-772-5637
>  NWRA, Boulder/CoRA Office             FAX: [phone]
>  3380 Mitchell Lane                       orion(a)nwra.com
>  Boulder, CO 80301                 https://www.nwra.com/
>  _______________________________________________
>  devel mailing list -- devel(a)lists.fedoraproject.org
>  To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>  List Guidelines: 
>  https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives: 
>  https://lists.fedoraproject.org/archives/list/[e-mail] 
-- 
Sérgio M. B.

[Jakub Jelinek]

On Mon, Feb 11, 2019 at 07:17:25PM -0700, Orion Poplawski wrote:
>  Looks like GCC 9 is finally enforcing an OpenMP change:
>  
>  From https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00628.html 
Please see https://gcc.gnu.org/gcc-9/porting_to.html#ompdatasharing
which documents what you can do and what works with both compilers and what
doesn't.

>  GCC 8 complains about just adding fp_stderr to shared():
>  
>  nco_omp.c: In function 'nco_openmp_ini':
>  nco_omp.c:205:43: error: 'fp_stderr' is predetermined 'shared' for
> 'shared'
>   # pragma omp parallel default(none) shared(fp_stderr,thr_nbr_act) 
Yes, either you want firstprivate(fp_stderr), or drop default(none) if you
want to make it work with both compilers.

>  And apparently complains if you drop default(none) as well (from
> another
>  project with the same problem):
>  
>  [  9%] Building CXX object CMakeFiles/_CuraEngine.dir/src/layerPart.cpp.o
>  /home/ruben/Projects/CuraEngine/src/layerPart.cpp: In function ‘void
>  cura::createLayerParts(cura::SliceMeshStorage&, cura::Slicer*)’:
>  /home/ruben/Projects/CuraEngine/src/layerPart.cpp:52:78: error:
>  ‘total_layers’ is predetermined ‘shared’ for ‘shared’
>   #pragma omp parallel for shared(mesh, slicer, total_layers)
>  schedule(dynamic) 
That is with GCC 8 or earlier, right?  Just leave total_layers out
of the shared clause.

	Jakub

[Orion Poplawski]

On 2/12/19 1:02 AM, Jakub Jelinek wrote:
>  On Mon, Feb 11, 2019 at 07:17:25PM -0700, Orion Poplawski wrote:
> > Looks like GCC 9 is finally enforcing an OpenMP change:
> >
> >  From https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00628.html
>  
>  Please see https://gcc.gnu.org/gcc-9/porting_to.html#ompdatasharing
>  which documents what you can do and what works with both compilers and what
>  doesn't. 
Thanks for the reference.

> > GCC 8 complains about just adding fp_stderr to shared():
> >
> > nco_omp.c: In function 'nco_openmp_ini':
> > nco_omp.c:205:43: error: 'fp_stderr' is predetermined 'shared' for
> 'shared'
> >   # pragma omp parallel default(none) shared(fp_stderr,thr_nbr_act)
>  
>  Yes, either you want firstprivate(fp_stderr), or drop default(none) if you
>  want to make it work with both compilers. 
Okay.  In this case we'll probably drop default(none).  Many people have 
gotten into the habit of using default(none) though in order to force 
declaring the form of all variables.

> > And apparently complains if you drop default(none) as well (from
> another
> > project with the same problem):
> >
> > [  9%] Building CXX object CMakeFiles/_CuraEngine.dir/src/layerPart.cpp.o
> > /home/ruben/Projects/CuraEngine/src/layerPart.cpp: In function ‘void
> > cura::createLayerParts(cura::SliceMeshStorage&, cura::Slicer*)’:
> > /home/ruben/Projects/CuraEngine/src/layerPart.cpp:52:78: error:
> > ‘total_layers’ is predetermined ‘shared’ for ‘shared’
> >   #pragma omp parallel for shared(mesh, slicer, total_layers)
> > schedule(dynamic)
>  
>  That is with GCC 8 or earlier, right?  Just leave total_layers out
>  of the shared clause. 
Right, because the default is to be shared.  Thanks.

-- 
Orion Poplawski
Manager of NWRA Technical Systems          720-772-5637
NWRA, Boulder/CoRA Office             FAX: [phone]
3380 Mitchell Lane                       orion(a)nwra.com
Boulder, CO 80301                 https://www.nwra.com/

[Christoph Junghans]

On Tue, Feb 12, 2019 at 5:39 PM Orion Poplawski <orion(a)nwra.com&gt; wrote:
>  On 2/12/19 1:02 AM, Jakub Jelinek wrote:
>  > On Mon, Feb 11, 2019 at 07:17:25PM -0700, Orion Poplawski wrote:
>  >> Looks like GCC 9 is finally enforcing an OpenMP change:
>  >>
>  >>  From https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00628.html
>  >
>  > Please see https://gcc.gnu.org/gcc-9/porting_to.html#ompdatasharing
>  > which documents what you can do and what works with both compilers and what
>  > doesn't.
>
>  Thanks for the reference.
>
>  >> GCC 8 complains about just adding fp_stderr to shared():
>  >>
>  >> nco_omp.c: In function 'nco_openmp_ini':
>  >> nco_omp.c:205:43: error: 'fp_stderr' is predetermined 'shared'
> for 'shared'
>  >>   # pragma omp parallel default(none) shared(fp_stderr,thr_nbr_act)
>  >
>  > Yes, either you want firstprivate(fp_stderr), or drop default(none) if you
>  > want to make it work with both compilers.
>
>  Okay.  In this case we'll probably drop default(none).  Many people have
>  gotten into the habit of using default(none) though in order to force
>  declaring the form of all variables.
>
>  >> And apparently complains if you drop default(none) as well (from another
>  >> project with the same problem):
>  >>
>  >> [  9%] Building CXX object CMakeFiles/_CuraEngine.dir/src/layerPart.cpp.o
>  >> /home/ruben/Projects/CuraEngine/src/layerPart.cpp: In function ‘void
>  >> cura::createLayerParts(cura::SliceMeshStorage&, cura::Slicer*)’:
>  >> /home/ruben/Projects/CuraEngine/src/layerPart.cpp:52:78: error:
>  >> ‘total_layers’ is predetermined ‘shared’ for ‘shared’
>  >>   #pragma omp parallel for shared(mesh, slicer, total_layers)
>  >> schedule(dynamic)
>  >
>  > That is with GCC 8 or earlier, right?  Just leave total_layers out
>  > of the shared clause.
>
>  Right, because the default is to be shared.  Thanks. 
For the lammps package I
chatted with upstream:
https://github.com/lammps/lammps/issues/1326
and they already had a hack_openmp script in their source for the PGI
compiler, which basically boils down to something like:
find . -type f \( -name "*.cpp" -or -name "*.h" \) -exec sed -e
'/#pragma omp/s/default(none)/default(shared)/' -e '/#pragma
omp/s/shared([^)]\+)//' -i {} +
and that did the trick for now, while upstream is fixing it for good:
https://github.com/lammps/lammps/pull/1330

Cheers,

Christoph
>  --
>  Orion Poplawski
>  Manager of NWRA Technical Systems          720-772-5637
>  NWRA, Boulder/CoRA Office             FAX: [phone]
>  3380 Mitchell Lane                       orion(a)nwra.com
>  Boulder, CO 80301                 https://www.nwra.com/
>  _______________________________________________
>  devel mailing list -- devel(a)lists.fedoraproject.org
>  To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>  List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives:
> https://lists.fedoraproject.org/archives/list/[e-mail] 


-- 
Christoph Junghans
Web: http://www.compphys.de

[Sérgio Basto]

On Wed, 2019-02-13 at 13:42 -0700, Christoph Junghans wrote:
>  On Tue, Feb 12, 2019 at 5:39 PM Orion Poplawski
> <orion(a)nwra.com&gt;
>  wrote:
>  > 
>  > On 2/12/19 1:02 AM, Jakub Jelinek wrote:
>  > > On Mon, Feb 11, 2019 at 07:17:25PM -0700, Orion Poplawski wrote:
>  > > > Looks like GCC 9 is finally enforcing an OpenMP change:
>  > > > 
>  > > >  From https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00628.html
>  > > 
>  > > Please see 
>  > > https://gcc.gnu.org/gcc-9/porting_to.html#ompdatasharing
>  > > which documents what you can do and what works with both
>  > > compilers and what
>  > > doesn't.
>  > 
>  > Thanks for the reference.
>  > 
>  > > > GCC 8 complains about just adding fp_stderr to shared():
>  > > > 
>  > > > nco_omp.c: In function 'nco_openmp_ini':
>  > > > nco_omp.c:205:43: error: 'fp_stderr' is predetermined
> 'shared'
>  > > > for 'shared'
>  > > >   # pragma omp parallel default(none)
>  > > > shared(fp_stderr,thr_nbr_act)
>  > > 
>  > > Yes, either you want firstprivate(fp_stderr), or drop
>  > > default(none) if you
>  > > want to make it work with both compilers.
>  > 
>  > Okay.  In this case we'll probably drop default(none).  Many people
>  > have
>  > gotten into the habit of using default(none) though in order to
>  > force
>  > declaring the form of all variables.
>  > 
>  > > > And apparently complains if you drop default(none) as well
>  > > > (from another
>  > > > project with the same problem):
>  > > > 
>  > > > [  9%] Building CXX object
>  > > > CMakeFiles/_CuraEngine.dir/src/layerPart.cpp.o
>  > > > /home/ruben/Projects/CuraEngine/src/layerPart.cpp: In function
>  > > > ‘void
>  > > > cura::createLayerParts(cura::SliceMeshStorage&,
>  > > > cura::Slicer*)’:
>  > > > /home/ruben/Projects/CuraEngine/src/layerPart.cpp:52:78: error:
>  > > > ‘total_layers’ is predetermined ‘shared’ for ‘shared’
>  > > >   #pragma omp parallel for shared(mesh, slicer, total_layers)
>  > > > schedule(dynamic)
>  > > 
>  > > That is with GCC 8 or earlier, right?  Just leave total_layers
>  > > out
>  > > of the shared clause.
>  > 
>  > Right, because the default is to be shared.  Thanks.
>  
>  For the lammps package I chatted with upstream:
>  https://github.com/lammps/lammps/issues/1326
>  and they already had a hack_openmp script in their source for the PGI
>  compiler, which basically boils down to something like:
>  find . -type f \( -name "*.cpp" -or -name "*.h" \) -exec sed -e
>  '/#pragma omp/s/default(none)/default(shared)/' -e '/#pragma
>  omp/s/shared([^)]\+)//' -i {} +
>  and that did the trick for now, while upstream is fixing it for good:
>  https://github.com/lammps/lammps/pull/1330 
Thanks for sharing your solution, but seems to me more simpler the
Orion solution than what I saw in lammps [1] .
The Orion solution is just drop "default(none)" [2] , hope that is
useful information for you ... 

Best regards, 

[1] 
https://src.fedoraproject.org/rpms/lammps/blob/master/f/lammps.spec#_129

[2]
https://github.com/sergiomb2/ufraw/commit/ac8c74699cba65d65f918b3f4cba2de...

>  Cheers,
>  
>  Christoph
>  > 
>  > --
>  > Orion Poplawski
>  > Manager of NWRA Technical Systems          720-772-5637
>  > NWRA, Boulder/CoRA Office             FAX: [phone]
>  > 3380 Mitchell Lane                       orion(a)nwra.com
>  > Boulder, CO 80301                 https://www.nwra.com/
>  > _______________________________________________
>  > devel mailing list -- devel(a)lists.fedoraproject.org
>  > To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  > Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>  > List Guidelines: 
>  > https://fedoraproject.org/wiki/Mailing_list_guidelines
>  > List Archives: 
>  > https://lists.fedoraproject.org/archives/list/[e-mail]
>  
>  
>  
>  -- 
>  Christoph Junghans
>  Web: http://www.compphys.de
>  _______________________________________________
>  devel mailing list -- devel(a)lists.fedoraproject.org
>  To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>  List Guidelines: 
>  https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives: 
>  https://lists.fedoraproject.org/archives/list/[e-mail] 
-- 
Sérgio M. B.

[Mattia Verga]

Il 2/12/19 3:17 AM, Orion Poplawski ha scritto:
>  Which is now leading to some (expected) compilation failures:
>
>  BUILDSTDERR: nco_omp.c: In function 'nco_openmp_ini':
>  BUILDSTDERR: nco_omp.c:210:38: error: 'fp_stderr' not specified in
>  enclosing 'parallel'
>  BUILDSTDERR:   210 |  if(nco_dbg_lvl_get() >= nco_dbg_fl)
>  (void)fprintf(fp_stderr,"%s: INFO Small parallel test region spawned
>  team of %d thread(s)\n",nco_prg_nm_get(),thr_nbr_act);
>  BUILDSTDERR:       |
>
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  BUILDSTDERR: nco_omp.c:205:10: error: enclosing 'parallel'
>  BUILDSTDERR:   205 | # pragma omp parallel default(none) shared(thr_nbr_act)
>  BUILDSTDERR:       |          ^~~
>
>  
Same problem in Rawtherapee [1], I've reported upstream asking for a 
solution [2]

Mattia

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1675874

[2] https://github.com/Beep6581/RawTherapee/issues/5177

[Sérgio Basto]

On Fri, 2019-02-15 at 07:32 +0000, Mattia Verga wrote:
>  Il 2/12/19 3:17 AM, Orion Poplawski ha scritto:
>  > Which is now leading to some (expected) compilation failures:
>  > 
>  > BUILDSTDERR: nco_omp.c: In function 'nco_openmp_ini':
>  > BUILDSTDERR: nco_omp.c:210:38: error: 'fp_stderr' not specified in
>  > enclosing 'parallel'
>  > BUILDSTDERR:   210 |  if(nco_dbg_lvl_get() >= nco_dbg_fl)
>  > (void)fprintf(fp_stderr,"%s: INFO Small parallel test region
>  > spawned
>  > team of %d thread(s)\n",nco_prg_nm_get(),thr_nbr_act);
>  > BUILDSTDERR:       |
>  > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  > BUILDSTDERR: nco_omp.c:205:10: error: enclosing 'parallel'
>  > BUILDSTDERR:   205 | # pragma omp parallel default(none)
>  > shared(thr_nbr_act)
>  > BUILDSTDERR:       |          ^~~ 

Just remove "default(none)" from "# pragma omp parallel default(none)
shared(thr_nbr_act)" doesn't fix the build for you ? 


>  > 
>  
>  Same problem in Rawtherapee [1], I've reported upstream asking for a 
>  solution [2]
>  
>  Mattia
>  
>  [1] https://bugzilla.redhat.com/show_bug.cgi?id=1675874
>  
>  [2] https://github.com/Beep6581/RawTherapee/issues/5177
>  
>  _______________________________________________
>  devel mailing list -- devel(a)lists.fedoraproject.org
>  To unsubscribe send an email to devel-leave(a)lists.fedoraproject.org
>  Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
>  List Guidelines: 
>  https://fedoraproject.org/wiki/Mailing_list_guidelines
>  List Archives: 
>  https://lists.fedoraproject.org/archives/list/[e-mail] 
-- 
Sérgio M. B.

[Mattia Verga]

Il 2/16/19 9:55 PM, Sérgio Basto ha scritto:
>  Just remove "default(none)" from "# pragma omp parallel default(none)
>  shared(thr_nbr_act)" doesn't fix the build for you ?
>
>  
Yes, thanks, upstream had replied and gave me a patch that removes 
"default(none)" from some places in the code.

Mattia