1. Berkeley Lab Avatar Berkeley Lab
  2. CRD-CLaSS
  3. upcxx
  4. Issues

make exe support for injecting pthread and OMP flags

Issue #319 new
john bachan created an issue

I can't build test/view.cpp due to linkage death.

mkdir mybld; cd mybld
../configure
make SRC=../test/view.cpp exe

Here's the relevant stderr:

Configuring debug-mode build of GASNet
/home/jdbachan/upcxx/mybld/bld/GASNet-stable/configure  --disable-parsync --enable-seq --enable-par --enable-pthreads --disable-segment-everything --enable-smp --enable-debug

[...text elided...]

/usr/bin/g++ -std=c++11 -g3   -Wno-unused -Wunused-result -Wno-unused-parameter -Wno-address    -DUPCXX_ASSERT_ENABLED=1 -DUPCXX_BACKEND=1 -DUPCXX_MPSC_QUEUE_ATOMIC=1 -DUPCXX_BACKEND_GASNET_SEQ=1 -I/home/jdbachan/upcxx/mybld/bld/upcxx.assert1.optlev0.dbgsym1.gasnet_seq.smp/include -I/home/jdbachan/upcxx/mybld/bld/upcxx.assert1.optlev0.dbgsym1.gasnet_seq.smp/gen_include   -D_GNU_SOURCE=1 -DGASNET_SEQ     -I/home/jdbachan/upcxx/mybld/bld/GASNet-stable -I/home/jdbachan/upcxx/mybld/bld/GASNet-stable/smp-conduit -I/home/jdbachan/upcxx/mybld/bld/GASNet-stable/other   -I/home/jdbachan/upcxx/mybld/bld/GASNet-stable/extended-ref/vis -I/home/jdbachan/upcxx/mybld/bld/GASNet-stable/extended-ref/coll -I/home/jdbachan/upcxx/mybld/bld/GASNet-stable/extended-ref/ratomic -I/home/jdbachan/upcxx/mybld/bld/GASNet-stable/extended-ref -I/home/jdbachan/upcxx/mybld/bld/gasnet.debug   -g3  -Wno-unused -Wunused-result -Wno-unused-parameter -Wno-address      ../test/view.cpp -L/home/jdbachan/upcxx/mybld/bld/upcxx.assert1.optlev0.dbgsym1.gasnet_seq.smp/lib -lupcxx -L/home/jdbachan/upcxx/mybld/bld/gasnet.debug/smp-conduit  -lgasnet-smp-seq    -lrt -L/usr/lib/gcc/x86_64-linux-gnu/9 -lgcc -lm  -o /home/jdbachan/upcxx/mybld/bld/upcxx.assert1.optlev0.dbgsym1.gasnet_seq.smp/exe/6ee4b22a69c53f1361c0c3b760109d5d6b35469d 
/usr/bin/ld: /tmp/cch7iZt7.o: undefined reference to symbol 'pthread_create@@GLIBC_2.2.5'
/usr/bin/ld: /lib/x86_64-linux-gnu/libpthread.so.0: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
make[4]: *** [/home/jdbachan/upcxx/bld/Makefile-exe.mak:36: /home/jdbachan/upcxx/mybld/bld/upcxx.assert1.optlev0.dbgsym1.gasnet_seq.smp/exe/6ee4b22a69c53f1361c0c3b760109d5d6b35469d] Error 1
make[3]: *** [/home/jdbachan/upcxx/bld/Makefile-exe.mak:16: default] Error 2
make[2]: *** [/home/jdbachan/upcxx/bld/Makefile:543: do-exe] Error 2
make[1]: *** [/home/jdbachan/upcxx/bld/Makefile:549: exe] Error 2
make: *** [/home/jdbachan/upcxx/bld/Makefile.rules:62: exe] Error 2

And here's the contents of bld/gasnet.debug/smp-conduit/gasnet-smp-seq.pc. It strikes me odd that --enable-pthreads was passed to gasnet configure yet no pthread flags are reported here.

# WARNING: This file is automatically generated - do NOT edit directly
# Copyright 2017, The Regents of the University of California
# Terms of use are as specified in license.txt
# See the GASNet README for instructions on using these variables
GASNET_CC=/usr/bin/gcc
GASNET_OPT_CFLAGS=-g3
GASNET_MISC_CFLAGS=-Wno-unused -Wunused-result -Wno-unused-parameter -Wno-address
GASNET_MISC_CPPFLAGS=
GASNET_DEFINES=-D_GNU_SOURCE=1 -DGASNET_SEQ
GASNET_INCLUDES=-I/home/jdbachan/upcxx/mybld/bld/GASNet-stable -I/home/jdbachan/upcxx/mybld/bld/GASNet-stable/smp-conduit -I/home/jdbachan/upcxx/mybld/bld/GASNet-stable/other -I/home/jdbachan/upcxx/mybld/bld/GASNet-stable/extended-ref/vis -I/home/jdbachan/upcxx/mybld/bld/GASNet-stable/extended-ref/coll -I/home/jdbachan/upcxx/mybld/bld/GASNet-stable/extended-ref/ratomic -I/home/jdbachan/upcxx/mybld/bld/GASNet-stable/extended-ref -I/home/jdbachan/upcxx/mybld/bld/gasnet.debug
GASNET_CXX=/usr/bin/g++
GASNET_OPT_CXXFLAGS=-g3
GASNET_MISC_CXXFLAGS=-Wno-unused -Wunused-result -Wno-unused-parameter -Wno-address
GASNET_MISC_CXXCPPFLAGS=
GASNET_LD=/usr/bin/gcc
GASNET_LDFLAGS=-g3 -Wno-unused -Wunused-result -Wno-unused-parameter -Wno-address
GASNET_LIBS=-L/home/jdbachan/upcxx/mybld/bld/gasnet.debug/smp-conduit -lgasnet-smp-seq -lrt -L/usr/lib/gcc/x86_64-linux-gnu/9 -lgcc -lm
GASNET_CFLAGS = ${GASNET_OPT_CFLAGS} ${GASNET_MISC_CFLAGS}
GASNET_CPPFLAGS = ${GASNET_MISC_CPPFLAGS} ${GASNET_DEFINES} ${GASNET_INCLUDES}
GASNET_CXXFLAGS = ${GASNET_OPT_CXXFLAGS} ${GASNET_MISC_CXXFLAGS}
GASNET_CXXCPPFLAGS = ${GASNET_MISC_CXXCPPFLAGS} ${GASNET_DEFINES} ${GASNET_INCLUDES}

Name: gasnet-smp-seq
Description: GASNet smp-conduit in seq threading mode
URL: https://gasnet.lbl.gov
Version: 2019.9.1
Cflags: ${GASNET_CPPFLAGS} ${GASNET_CFLAGS}
Libs: ${GASNET_LDFLAGS} ${GASNET_LIBS}

Comments (16)

  1. Dan Bonachea

    This test requires the par backend, but you are getting the seq default one.

    Try adding UPCXX_BACKEND=gasnet_par on the make command line

  2. john bachan reporter

    That requirement wasn't there before. This test respects the (admittedly not actually defined) requirements for using the seq backend despite its use of threads. The intention is that this test should continue to operate against seq, and that is especially so once the seq requirements are fully defined (which I promise will be inclusive of this test). In the future, the build infra will need a way to build tests with threads against a seq gasnet, so this issue stands.

  3. Dan Bonachea

    I cannot speak to "intentions", but this test has only ever been built using the PAR backend in both the old run-tests script and our automated CI.

    This test creates C++11 threads that explicitly call upcxx::progress() (line 138), while the master persona on the primordial thread is concurrently active making UPC++ communication calls - that doesn't fit into any definition of "seq" as I understand the concept. Certainly if those progress calls expand to any call into GASNet (including AMPoll) then it violates GEX seq mode.

  4. Paul Hargrove

    In the future, the build infra will need a way to build tests with threads against a seq gasnet, so this issue stands.

    To ignore the violation Dan noted of the "seq" definition and build view.cpp with seq backend and pthreads: 

    $ make exe SRC=view.cpp EXTRAFLAGS=-pthread

    Admittedly that spelling is compiler-dependent (w/ PGI being the only outlier), which was not the case when nobs was maintaining a manually-curated list of the pthread-using tests and the pthread flags.

    This is certainly NOT a "major bug" for this release.

    I'd downgraded to an Enhancement, but also acknowledge in the Title that the same issue is present for tests or examples using OpenMP (with similar compiler-dependent solution via EXTRAFLAGS, FWIW)

  5. john bachan reporter

    @Dan Bonachea the implementation of seq does support calls to progress() from non-master owning threads. Such calls ignore gasnet work and simply sniff the lpc queues. @Paul Hargrove thanks for EXTRAFLAGS, adding that works, and confirms that this test meets the intended notion of seq-compatibilitiy.

  6. Paul Hargrove

    Note to future self:

    Thanks to the order in which things are evaluated, something like the following would "just work" if configure and/or compler.mak can provide a definition for $(USE_PTHREADS). The only key is ensuring the $ passes safely through your shell. 

    $ make run SRC=view.cpp EXTRAFLAGS='$(USE_PTHREADS)'

    The same principle could be applied to OpenMP, of course.

  9. Paul Hargrove

    CI testing: add probe for OpenMP support

    This commit partially addresses issue 319 by adding an automated probe for OpenMP support and the corresponding compiler flags. Support for OpenMP is an optional feature which only impacts the dev-tests and dev-check family of make targets.

    The most significant change made in the PR is to automatically set UPCXX_HAVE_OPENMP if the new probe succeeds, thus enabling OpenMP tests in dev-check and related targets. This change means manually setting of this variable is no longer necessary for well-behaved systems. However, setting it explicitly to 1 on a system without support is sufficient to generate failures on the OpenMP tests since the required flags will be missing. Therefore, such settings have been intentionally preserved in .gitlab-ci.yml to prevent any silent loss of coverage.

    Due to issues with both cross-compilation and library search paths at distributed-application launch, is does not seem worth adding a run check to this probe. So, in the case of a platform which can link OpenMP but cannot run, one may need to begin passing a new (but undocumented) configure argument --without-openmp to prevent new test failures. Similarly, one can pass --with-openmp to ensure a build failure if support is not found. Setting UPCXX_HAVE_OPENMP to an empty string at testing time remains available as a means to disable the OpenMP tests, and again such usage has been retained in the various CI scripts.

    → <<cset e49c257298ad>>

  10. Paul Hargrove

    As noted in the commit message reproduced in the previous comment, we now have automatic discovery of OpenMP flags. The expectation when referencing this issue in that commit message was that use of the probed flags could be requested via EXTRAFLAGS='$(UPCXX_OPENMP_FLAGS)'. However, it seems that does not work due to the order in which things are evaluated.

    The following works, but is hard to endorse as "best practice".

    $ make run SRC=rput_omp.cpp UPCXX_BACKEND=gasnet_par EXTRAFLAGS=$(eval $(make echovar VARNAME=UPCXX_OPENMP_FLAGS); echo $UPCXX_OPENMP_FLAGS)
make[5]: `gasnet-smp-par.pc' is up to date.
make[5]: `libgasnet-smp-par.a' is up to date.
make[6]: `gen_include/upcxx/upcxx_config.hpp' is up to date.
make[6]: `libupcxx.a' is up to date.
make[6]: `upcxx_headers.d' is up to date.
Test: rput_omp.cpp
Ranks: 4
Threads: 10
Test result: SUCCESS

    However, is may be sufficient to run just make echovar VARNAME=UPCXX_OPENMP_FLAGS and manually apply the resulting flag to ones builds.

    I suspect more work could be done to improve on this state, but I don't perceive the demand.

  17. Log in to comment
Assignee
Type
enhancement
Priority
minor
Status
new
Component
Build/install scripts
Milestone
Deferred indefinitely
Version
Development Branch
Votes
0
Watchers
2
Jira: the preferred issue tracker for Bitbucket. Join the team!