Paths

Table of Contentst

Differential D11507

Add LLVM's OpenMP 4.0.0 library to the base system
AbandonedPublic
Actions

Authored by dim on Jul 6 2017, 6:29 PM.

Project Tags

None

Referenced Files

None

Subscribers

eborisch_gmail.com

jbeich

Details

Reviewers

bdrewery
emaste
jmd

Summary

Add stripped version of OpenMP 4.0.0 to contrib
Add fix for PAGE_SIZE redefinition
Add lib/libomp Makefile plus generated headers
Install omp.h into /usr/include
Hook up libomp to bulidworld, only for amd64
Add src.conf(5) descriptions, and regenerate it.

There are probably some details left to finish, this is work in
progress.

Test Plan

Builds OK, would need some actual OpenMP software (from e.g. ports)
tested and validated against it.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 10392
Build 10803: arc lint + arc unit

Event Timeline

dim created this revision.Jul 6 2017, 6:29 PM

Harbormaster completed remote builds in B10353: Diff 30501.Jul 6 2017, 6:29 PM

emaste added inline comments.Jul 6 2017, 6:45 PM

tools/build/options/WITH_OPENMP
3	should we say something like "LLVM's OpenMP runtime, libomp"?

emaste added inline comments.Jul 6 2017, 6:47 PM

lib/libomp/Makefile
44–48	I wonder if upstream would mind renaming it?
52	ick :)

dim added inline comments.Jul 7 2017, 11:46 AM

lib/libomp/Makefile
44–48	Maybe they won't mind indeed. For Windows users the case doesn't matter anyway.
52	Yeah, this is a gross hack, but the `register` keyword is used in a lot of places, and -Werror complains a lot about it being deprecated. It should really be removed upstream, as this keyword has made no sense since the last few decades.
tools/build/options/WITH_OPENMP
3	My initial text was "Set to build LLVM's OpenMP shared library". But it also installs a header, so it isn't exactly correct, nor is just mentioning libomp. Maybe: "Set to build LLVM's OpenMP runtime library (libomp) and header (omp.h)." ?

emaste added inline comments.Jul 7 2017, 2:34 PM

lib/libomp/Makefile
44–48	compiler-rt builds many asm files and they are all `.S` a quick search of some other llvm projects found asm files used only for tests and they seem to be mostly `.s` - e.g. LLVM has 2024 `.s` and 1 `.S`. I think we could make the case to use `.S` for consistency with compiler-rt though.
52	https://reviews.llvm.org/D35124
tools/build/options/WITH_OPENMP
3	I think that's fine although perhaps slightly verbose? Maybe "Set to build LLVM's OpenMP runtime"? Even if not 100% pedantically correct (is the header part of the runtime?) I think it's probably suitable for readers of src.conf.

Upstream r307399: remove duplicate symbol version script entries
Upstream r307441: remove deprecated register storage class specifier
Rename z_Linux_asm.s to z_Linux_asm.S, and get rid of symlink hack.

Update WITH/WITHOUT_OPENMP descriptions.

Since we are compiling with -ffunction-sections -fdata-sections, link with --gc-sections.

Harbormaster completed remote builds in B10394: Diff 30582.Jul 8 2017, 5:20 PM

Thanks for this work! I am trying to test it on my workloads. However, a recent HEAD with the patch applied does not compile:

/usr/src_svn/contrib/openmp/runtime/src/kmp_runtime.cpp:7621:18: error: "Unknown or unsupported OS"
                #error "Unknown or unsupported OS"
                 ^
1 error generated.

Am I missing something obvious?

In D11507#239703, @jmd wrote:
Thanks for this work! I am trying to test it on my workloads. However, a recent HEAD with the patch applied does not compile:
/usr/src_svn/contrib/openmp/runtime/src/kmp_runtime.cpp:7621:18: error: "Unknown or unsupported OS"
                #error "Unknown or unsupported OS"
                 ^
1 error generated.
Am I missing something obvious?

Are you building this on amd64? Because that is currently the only supported architecture.

@dim Yes, HEAD/amd64. So it is indeed weird how my compilation ends up in the "other" branch. May need to try a fresh checkout and fresh patch. I'll report back.

Different machine, clean checkout, same issue. Could this have to do with setting CPUTYPE?= in make.conf? I have this on both machines (bdver4 and broadwell).

In D11507#240121, @jmd wrote:

Different machine, clean checkout, same issue. Could this have to do with setting CPUTYPE?= in make.conf? I have this on both machines (bdver4 and broadwell).

It's hard to say without reproduction details. How are you running buildworld, and what are your make.conf and src.conf settings?

Ok, same issue after commenting CPUTYPE in make.conf.

make.conf:

MALLOC_PRODUCTION="YES"
#CPUTYPE?=broadwell
OPTIONS_UNSET=DOCS

No src.conf. This is a drm next machine: FreeBSD 12.0-CURRENT #11 3de4bc9eac3(drm-next): Sat Jun 3 00:29:48 EDT 2017 /usr/obj/usr/src/sys/GENERIC amd64 , so is the other machine I tried this on.

Installed clang:

# clang -v
FreeBSD clang version 4.0.0 (tags/RELEASE_400/final 297347) (based on LLVM 4.0.0)
Target: x86_64-unknown-freebsd12.0
Thread model: posix
InstalledDir: /usr/bin

Anything else you want to know?

In D11507#240181, @jmd wrote:

Anything else you want to know?

What is the exact command you are running, and at which stage of buildworld do you see the error? Can you post a full build log somewhere (not on this review please)?

@dim please see mail on Friday that indicates the library gets built as 32bit.

This might fix the lib32 issue: http://people.freebsd.org/~bdrewery/patches/libcompat-target-arch-src-opts.diff

@bdrewery unfortunately no change with your patch for me

jbeich subscribed.Jul 28 2017, 5:35 PM

jbeich added inline comments.

lib/libomp/Makefile

Are you trying to resurrect bug 214258? Maybe replace with -Wl,--no-undefined.

$ fetch https://computing.llnl.gov/tutorials/openMP/samples/C/omp_hello.c
$ cc omp_hello.c -fopenmp
/usr/lib/libomp.so: undefined reference to `scalbnl'
/usr/lib/libomp.so: undefined reference to `fmaxl'
/usr/lib/libomp.so: undefined reference to `pthread_attr_getstack'
/usr/lib/libomp.so: undefined reference to `logbl'
/usr/lib/libomp.so: undefined reference to `pthread_create'
/usr/lib/libomp.so: undefined reference to `pthread_attr_get_np'
/usr/lib/libomp.so: undefined reference to `scalbnf'
/usr/lib/libomp.so: undefined reference to `logb'
/usr/lib/libomp.so: undefined reference to `pthread_condattr_init'
/usr/lib/libomp.so: undefined reference to `logbf'
/usr/lib/libomp.so: undefined reference to `scalbn'
cc: error: linker command failed with exit code 1 (use -v to see invocation)