Bug 236582 - Enable LLVM openmp on i386
Summary: Enable LLVM openmp on i386
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: i386 Any
: --- Affects Some People
Assignee: Dimitry Andric
URL:
Keywords: feature
Depends on:
Blocks: 236062
  Show dependency treegraph
 
Reported: 2019-03-16 21:13 UTC by Jan Beich
Modified: 2020-08-24 02:40 UTC (History)
4 users (show)

See Also:
koobs: mfc-stable12+
koobs: mfc-stable11+


Attachments
v0 (1.35 KB, patch)
2019-03-16 21:13 UTC, Jan Beich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer freebsd_triage 2019-03-16 21:13:19 UTC
Created attachment 202923 [details]
v0

Clang -fopenmp works fine on i386, see ports r447281. As long as i386 remains Tier1 having -fopenmp excluded on it will accelerate the sunset of the architecture due to increased effort required to maintain ports on it.
Comment 1 Dimitry Andric freebsd_committer freebsd_triage 2019-03-17 00:30:48 UTC
Ah, this patch should really be upstreamed...
Comment 2 Dimitry Andric freebsd_committer freebsd_triage 2019-03-17 11:37:26 UTC
Unfortunately it does not compile for our old default i486 CPU:

--- kmp_csupport.pico ---
/usr/src/contrib/openmp/runtime/src/kmp_csupport.cpp:565:7: error: '_mm_setcsr' needs target feature sse
      __kmp_load_mxcsr(&serial_team->t.t_mxcsr);
      ^
/usr/src/contrib/openmp/runtime/src/kmp.h:3669:29: note: expanded from macro '__kmp_load_mxcsr'
#define __kmp_load_mxcsr(p) _mm_setcsr(*(p))
                            ^
1 error generated.

So we can't unconditionally apply this to the i386 arch. Personally I'd be in favor of raising the requirements for i386 to having at least SSE2, but that is going to get lots of pushback from people that like to run old hardware. :)

Maybe there is some way to detect the actual CPU or CPUTYPE setting at build time, and trigger on this.

Note also that the patch from ports r447281 is maybe not entirely correct, in the sense that it enables code also for arm, aarch and mips.  I have no idea at all whether those are supported in any way.
Comment 3 Konstantin Belousov freebsd_committer freebsd_triage 2019-03-17 11:46:48 UTC
(In reply to Dimitry Andric from comment #2)
You can certainly enable SSE2 when compiling for compat32 environment on amd64.

I think it would be also fine to enable SSE2 on i386 (in both cases I mean, for libomp, not for whole set of libs).
Comment 4 Jan Beich freebsd_committer freebsd_triage 2019-03-17 12:38:01 UTC
(In reply to Dimitry Andric from comment #2)
Probably another upstream bug. _mm_setcsr probably needs __DEFAULT_FN_ATTRS after https://reviews.llvm.org/rL239883.
Comment 5 Jan Beich freebsd_committer freebsd_triage 2019-03-17 12:54:42 UTC
Nevermind comment 4. GCC also complains.

In file included from /usr/src/contrib/openmp/runtime/src/kmp.h:103,
                 from /usr/src/contrib/openmp/runtime/src/kmp_csupport.cpp:16:
/usr/local/lib/gcc8/gcc/${CONFIGURE_TARGET}/8.2.0/include/xmmintrin.h: In function '__m64 _mm_cvtps_pi32(__m128)':
/usr/local/lib/gcc8/gcc/${CONFIGURE_TARGET}/8.2.0/include/xmmintrin.h:542:27: warning: MMX vector return without MMX enabled changes the ABI [-Wpsabi]
 _mm_cvtps_pi32 (__m128 __A)
                           ^
/usr/local/lib/gcc8/gcc/${CONFIGURE_TARGET}/8.2.0/include/xmmintrin.h: In function 'void __kmpc_end_serialized_parallel(ident_t*, kmp_int32)':
/usr/local/lib/gcc8/gcc/${CONFIGURE_TARGET}/8.2.0/include/xmmintrin.h:853:1: error: inlining failed in call to always_inline 'void _mm_setcsr(unsigned int)': target specific option mismatch
 _mm_setcsr (unsigned int __I)
 ^~~~~~~~~~
In file included from /usr/src/contrib/openmp/runtime/src/kmp_csupport.cpp:16:
/usr/src/contrib/openmp/runtime/src/kmp.h:3669:39: note: called from here
 #define __kmp_load_mxcsr(p) _mm_setcsr(*(p))
                             ~~~~~~~~~~^~~~~~
/usr/src/contrib/openmp/runtime/src/kmp_csupport.cpp:565:7: note: in expansion of macro '__kmp_load_mxcsr'
       __kmp_load_mxcsr(&serial_team->t.t_mxcsr);
       ^~~~~~~~~~~~~~~~
In file included from /usr/src/contrib/openmp/runtime/src/kmp.h:103,
                 from /usr/src/contrib/openmp/runtime/src/kmp_csupport.cpp:16:
/usr/local/lib/gcc8/gcc/${CONFIGURE_TARGET}/8.2.0/include/xmmintrin.h:853:1: error: inlining failed in call to always_inline 'void _mm_setcsr(unsigned int)': target specific option mismatch
 _mm_setcsr (unsigned int __I)
 ^~~~~~~~~~
In file included from /usr/src/contrib/openmp/runtime/src/kmp_csupport.cpp:16:
/usr/src/contrib/openmp/runtime/src/kmp.h:3669:39: note: called from here
 #define __kmp_load_mxcsr(p) _mm_setcsr(*(p))
                             ~~~~~~~~~~^~~~~~
/usr/src/contrib/openmp/runtime/src/kmp_csupport.cpp:565:7: note: in expansion of macro '__kmp_load_mxcsr'
       __kmp_load_mxcsr(&serial_team->t.t_mxcsr);
       ^~~~~~~~~~~~~~~~
Comment 6 commit-hook freebsd_committer freebsd_triage 2019-03-18 21:04:38 UTC
A commit references this bug:

Author: dim
Date: Mon Mar 18 21:04:29 UTC 2019
New revision: 345283
URL: https://svnweb.freebsd.org/changeset/base/345283

Log:
  Enable building libomp.so for 32-bit x86.  This is done by selectively
  enabling the functions that save and restore MXCSR, since access to this
  register requires SSE support.

  Note that you may run into other issues with OpenMP on i386, since this
  *not* yet supported upstream, and certainly not extensively tested.

  PR:		236062, 236582
  MFC after:	1 month
  X-MFC-With:	r344779

Changes:
  head/contrib/openmp/runtime/src/kmp.h
  head/contrib/openmp/runtime/src/kmp_runtime.cpp
  head/lib/Makefile
Comment 7 Jan Beich freebsd_committer freebsd_triage 2019-03-18 22:07:44 UTC
(In reply to commit-hook from comment #6)
Did you forget share/mk/src.opts.mk?
Comment 8 commit-hook freebsd_committer freebsd_triage 2019-03-19 06:59:10 UTC
A commit references this bug:

Author: dim
Date: Tue Mar 19 06:58:28 UTC 2019
New revision: 345291
URL: https://svnweb.freebsd.org/changeset/base/345291

Log:
  Turn on MK_OPENMP for i386 by default, now that it can build.

  Noticed by:	jbeich
  PR:		236062, 236582
  MFC after:	1 month
  X-MFC-With:	r344779

Changes:
  head/share/mk/src.opts.mk
Comment 9 Dimitry Andric freebsd_committer freebsd_triage 2019-03-23 14:21:06 UTC
We now have openmp on i386.
Comment 10 commit-hook freebsd_committer freebsd_triage 2019-04-17 20:09:17 UTC
A commit references this bug:

Author: dim
Date: Wed Apr 17 20:08:04 UTC 2019
New revision: 346331
URL: https://svnweb.freebsd.org/changeset/base/346331

Log:
  After r346168, also merge build infrastructure for LLVM libomp.

  MFC r345235:

  Add lib/libomp, with a Makefile, and generated configuration headers.
  Not connected to the main build yet, as there is still the issue of the
  GNU omp.h header conflicting with the LLVM one.  (That is, if MK_GCC is
  enabled.)

  PR:		236062

  MFC r345236:

  Connect lib/libomp to the build.

  * Set MK_OPENMP to yes by default only on amd64, for now.
  * Bump __FreeBSD_version to signal this addition.
  * Ensure gcc's conflicting omp.h is not installed if MK_OPENMP is yes.
  * Update OptionalObsoleteFiles.inc to cope with the conflicting omp.h.
  * Regenerate src.conf(5) with new WITH/WITHOUT fragments.

  Relnotes:	yes
  PR:		236062

  MFC r345242:

  Explicitly link libomp.so against -lpthread, as it depends on pthread
  functionality.  This should make example OpenMP programs work out of the
  box.

  Reported by:	jbeich
  PR:		236062, 236581

  MFC r345278:

  Also explicitly link libomp.so against -lm, as it transitively depends
  on scalbn and a few other math functions, via libcompiler-rt.  This
  should allow OpenMP programs to link with BFD linkers too.

  Reported by:	jbeich
  PR:		236062, 236581

  MFC r345282:

  Remove --as-needed from the linker flags for libomp.so, as these
  actually prevent the transitive dependency on libm.

  Reported by:	jbeich
  PR:		236062, 236581

  MFC r345291:

  Turn on MK_OPENMP for i386 by default, now that it can build.

  Noticed by:	jbeich
  PR:		236062, 236582

Changes:
_U  stable/12/
  stable/12/gnu/lib/Makefile
  stable/12/lib/libomp/
  stable/12/lib/libomp/Makefile
  stable/12/share/man/man5/src.conf.5
  stable/12/share/mk/src.opts.mk
  stable/12/tools/build/mk/OptionalObsoleteFiles.inc
  stable/12/tools/build/options/WITHOUT_OPENMP
  stable/12/tools/build/options/WITH_OPENMP
Comment 11 commit-hook freebsd_committer freebsd_triage 2019-04-17 20:17:51 UTC
A commit references this bug:

Author: dim
Date: Wed Apr 17 20:16:51 UTC 2019
New revision: 346333
URL: https://svnweb.freebsd.org/changeset/base/346333

Log:
  After r346168, also merge build infrastructure for LLVM libomp.

  MFC r345235:

  Add lib/libomp, with a Makefile, and generated configuration headers.
  Not connected to the main build yet, as there is still the issue of the
  GNU omp.h header conflicting with the LLVM one.  (That is, if MK_GCC is
  enabled.)

  PR:		236062

  MFC r345236:

  Connect lib/libomp to the build.

  * Set MK_OPENMP to yes by default only on amd64, for now.
  * Bump __FreeBSD_version to signal this addition.
  * Ensure gcc's conflicting omp.h is not installed if MK_OPENMP is yes.
  * Update OptionalObsoleteFiles.inc to cope with the conflicting omp.h.
  * Regenerate src.conf(5) with new WITH/WITHOUT fragments.

  Relnotes:	yes
  PR:		236062

  MFC r345242:

  Explicitly link libomp.so against -lpthread, as it depends on pthread
  functionality.  This should make example OpenMP programs work out of the
  box.

  Reported by:	jbeich
  PR:		236062, 236581

  MFC r345278:

  Also explicitly link libomp.so against -lm, as it transitively depends
  on scalbn and a few other math functions, via libcompiler-rt.  This
  should allow OpenMP programs to link with BFD linkers too.

  Reported by:	jbeich
  PR:		236062, 236581

  MFC r345282:

  Remove --as-needed from the linker flags for libomp.so, as these
  actually prevent the transitive dependency on libm.

  Reported by:	jbeich
  PR:		236062, 236581

  MFC r345291:

  Turn on MK_OPENMP for i386 by default, now that it can build.

  Noticed by:	jbeich
  PR:		236062, 236582

Changes:
_U  stable/11/
  stable/11/gnu/lib/Makefile
  stable/11/lib/libomp/
  stable/11/lib/libomp/Makefile
  stable/11/share/man/man5/src.conf.5
  stable/11/share/mk/src.opts.mk
  stable/11/tools/build/mk/OptionalObsoleteFiles.inc
  stable/11/tools/build/options/WITHOUT_OPENMP
  stable/11/tools/build/options/WITH_OPENMP