Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Topic/enable atomics #288

Closed
wants to merge 362 commits into from
Closed

Topic/enable atomics #288

wants to merge 362 commits into from

Conversation

bosilca
Copy link
Member

@bosilca bosilca commented Nov 26, 2014

As most compilers gain support for builtin atomics I propose to switch the default Open MPI behavior with regard to atomics. Instead of relying on our own atomics, we should leverage what the compiler, or the OS, is supporting. This patch change the default behavior for builds on OSX or with a compiler supporting gcc-type atomic builtins.

@goodell and @hjelmn please review.

@bosilca bosilca self-assigned this Nov 26, 2014
@mellanox-github
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/72/
Test PASSed.

@mellanox-github
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/76/
Test PASSed.

@mellanox-github
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/78/
Test PASSed.

@mellanox-github
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/81/
Test PASSed.

@jsquyres
Copy link
Member

jsquyres commented Dec 1, 2014

@bosilca George -- can you rebase this PR branch such that your unique commits are on the top of the current master? Right now, I can't read this patch to know what is new / part of this PR vs. what is already on the master.

(ping me if you want some help doing this rebase)

Thanks!

@mellanox-github
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/82/
Test PASSed.

@bosilca
Copy link
Member Author

bosilca commented Dec 1, 2014

What a mess. I finally manage to rebase this is a decent way (I had to use --force-with-lease to do this). Hopefully this was the correct way.

@mellanox-github
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/83/
Test PASSed.

@mellanox-github
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/84/
Test PASSed.

@goodell
Copy link
Member

goodell commented Dec 2, 2014

George, there are still 21 commits added by this PR just to add what looks like a single small change, and some of them look badly broken. Multiple merge commits appear to still be present on this branch. The right thing to do is to avoid ever merging in the first place, which will make your rebases work properly. Since you already have all these merges on this branch, you'll now need to do something a bit more complicated. Ping me on IM if you want help with this.

The --force-with-lease (or even a plain --force) option to git push is fine for for PRs like this when they come from your personal repository. But it should only be used once you've actually correctly fixed your local history, which hasn't happened yet.

@mellanox-github
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/119/
Test PASSed.

ggouaillardet and others added 16 commits April 14, 2015 15:06
as reported by Coverity with CID 1196722
as reported by Coverity with CIDs 1269848, 1269852 and 1269862
as reported by Coverity with CIDs 710628, 1196713 and 1269855
as reported by Coverity with CIDs 1269901 ans 1269902
as reported by Coverity with CID 710627
as reported by Coverity with CIDs 72125, 72126, 1269899 and 1269900
as reported by Coverity with CID 72202
as reported by Coverity with CIDs 1196737 and 1269850
as reported by Coverity with CIDs 72127, 72145, 72146, 72177, 72179,
72186, 731276, 731278, 1269888, 1269890
as reported by Coverity with CIDs 1196785, 1196787 and 1269896
as reported by Coverity with CIDs 71091, 71230, 71231, 72274, 72389,
1196718 and 1196719
This way, if the call to ibv_fork_init() fails, the job will still
continue.
of datatypes. Thanks Bogdan Sataric for the bug report.
jsquyres and others added 19 commits April 14, 2015 15:06
OMPI *requires* a C99 compiler, so we don't need the AC_C_INLINE or
AC_C_RESTRICT tests anymore (because the C99 compiler guarantees to
support them).  Indeed, in some cases (see #491),
AC_C_INLINE gets the wrong answer and defines `inline` to be empty,
which screws up some systems (e.g., OS X with clang).

As an added bonus, we get to get rid of a bunch of gcc 2.96-specific
code (!) in configure.ac.
The oob/ud configure was not honoring the case
if the ompi is configured with --with-verbs=no.
This fixes that problems.

Fixes #522

Signed-off-by: Howard Pritchard <howardp@lanl.gov>
Instead, use a safe environment variable name (that is SCOPE_PUSHed and
SCOPE_POPed).
…igure

== Short version

Do not export special variables into the environment (e.g., LIBS,
LDFLAGS, etc.) when invoking subdir configure scripts.  This prevents
problems described in #471.

== More detail

Exporing special env variables before invoking a subdir configure
script causes problems in some cases.  E.g., in #471,
when the user configures with `--with-hwloc=/path/to/hwloc`, and that
directory is *not* in a default linker search location will cause the
libevent subdir configuration to fail.

This happens because:

1. We'll pass LIBS="-L/path/to/hwloc/lib -lhwloc" to the libevent
   configure script
1. Meaning: configure-generated executables will link successfully
1. But unless LD_LIBRARY_PATH (or some other
   tell-the-linker-where-to-find-things mechanism) includes
   /path/to/hwloc/lib, the executable can't run.

Specifically, the libevent "hey, does the compiler generate proper
executables?" check will fail, and configure will abort (because OMPI
needs libevent).

I checked the history: exporting these vars dates all the way back to
LAM/MPI.  I can't think of a reason why we need to export these
variables -- AC_CONFIG_SUBDIRs doesn't do it; subdir configure scripts
should be orthogonal from the upper-layer configure script (and its
variables).  So let's remove these export statements and see if
anything breaks.
…mponent can be used.

This doesn't matter on the master, but it does matter on the 1.8 branch as the MTL select logic is different over there.
This commit fixes a typo in mca_btl_vader_progress_endpoints where
OPAL_THREAD_LOCK was used when OPAL_THREAD_UNLOCK was intended.

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
… the user has asked LSF to assign bindings. Fix a couple of typos in lex parser definitions. Tell hostfile parser to ignore binding designations in hostfiles. Add an attribute to indicate that cpusets were provided as physical cpu ids.

Once validated, a version of this will be backported to the v1.8.4 release.
… the semi-colon vs comma situation in hwloc slot_lists
Also alphebetize the ignored files in test/class.
… the user has asked LSF to assign bindings. Fix a couple of typos in lex parser definitions. Tell hostfile parser to ignore binding designations in hostfiles. Add an attribute to indicate that cpusets were provided as physical cpu ids.

Once validated, a version of this will be backported to the v1.8.4 release.
@mellanox-github
Copy link

Refer to this link for build results (access rights to CI server needed):
http://bgate.mellanox.com/jenkins/job/gh-ompi-master-pr/433/

Build Log
last 50 lines

GitHub pull request #288 of commit e9ab8e0d4394676073a9d7a64e49fa5878c0fe72.
Setting status of e9ab8e0d4394676073a9d7a64e49fa5878c0fe72 to PENDING with url http://bgate.mellanox.com:8888/jenkins/job/gh-ompi-master-pr/433/ and message: Build started.
[EnvInject] - Loading node environment variables.
Building on master in workspace /var/lib/jenkins/jobs/gh-ompi-master-pr/workspace
Wiping out workspace first.
Cloning the remote Git repository
Cloning repository https://github.com/open-mpi/ompi
 > /usr/bin/git init /var/lib/jenkins/jobs/gh-ompi-master-pr/workspace # timeout=60
Fetching upstream changes from https://github.com/open-mpi/ompi
 > /usr/bin/git --version # timeout=60
using .gitcredentials to set credentials
 > /usr/bin/git config --local credential.helper store --file=/tmp/git145290382710597451.credentials # timeout=60
 > /usr/bin/git -c core.askpass=true fetch --tags --progress https://github.com/open-mpi/ompi +refs/heads/*:refs/remotes/origin/* # timeout=60
 > /usr/bin/git config --local --remove-section credential # timeout=60
 > /usr/bin/git config remote.origin.url https://github.com/open-mpi/ompi # timeout=60
 > /usr/bin/git config --add remote.origin.fetch +refs/heads/*:refs/remotes/origin/* # timeout=60
 > /usr/bin/git config remote.origin.url https://github.com/open-mpi/ompi # timeout=60
Pruning obsolete local branches
Fetching upstream changes from https://github.com/open-mpi/ompi
using .gitcredentials to set credentials
 > /usr/bin/git config --local credential.helper store --file=/tmp/git2673383877297659309.credentials # timeout=60
 > /usr/bin/git -c core.askpass=true fetch --tags --progress https://github.com/open-mpi/ompi +refs/pull/*:refs/remotes/origin/pr/* --prune # timeout=60
 > /usr/bin/git config --local --remove-section credential # timeout=60
 > /usr/bin/git rev-parse e9ab8e0d4394676073a9d7a64e49fa5878c0fe72^{commit} # timeout=60
Merging Revision e9ab8e0d4394676073a9d7a64e49fa5878c0fe72 (detached) to origin/master, UserMergeOptions{mergeRemote='origin', mergeTarget='master', mergeStrategy='default', fastForwardMode='--ff'}
 > /usr/bin/git rev-parse origin/master^{commit} # timeout=60
 > /usr/bin/git config core.sparsecheckout # timeout=60
 > /usr/bin/git checkout -f origin/master # timeout=20
 > /usr/bin/git merge --ff e9ab8e0d4394676073a9d7a64e49fa5878c0fe72 # timeout=60
 > /usr/bin/git config core.sparsecheckout # timeout=60
 > /usr/bin/git checkout -f e9ab8e0d4394676073a9d7a64e49fa5878c0fe72 # timeout=20
ERROR: Branch not suitable for integration as it does not merge cleanly: Could not merge AnyObjectId[e9ab8e0d4394676073a9d7a64e49fa5878c0fe72]
TAP Reports Processing: START
Looking for TAP results report in workspace using pattern: **/*.tap
Did not find any matching files.
Anchor chain: could not read file with links: /var/lib/jenkins/jobs/gh-ompi-master-pr/workspace/jenkins_sidelinks.txt (No such file or directory)
[copy-to-slave] The build is taking place on the master node, no copy back to the master will take place.
Setting commit status on GitHub for https://api.github.com/repos/open-mpi/ompi/commit/e9ab8e0d4394676073a9d7a64e49fa5878c0fe72
[BFA] Scanning build for known causes...

[BFA] Done. 0s
Setting status of e9ab8e0d4394676073a9d7a64e49fa5878c0fe72 to FAILURE with url http://bgate.mellanox.com:8888/jenkins/job/gh-ompi-master-pr/433/ and message: Build finished.

Test FAILed.

@hjelmn
Copy link
Member

hjelmn commented Apr 14, 2015

@bosilca Can you rebase this branch instead of merging?

@hppritcha
Copy link
Member

This is a complete mess. I want to see this PR either rebased to make for a clean commit or else closed.

@jsquyres
Copy link
Member

@bosilca Want to catch me on IM to figure out how to rebase it properly?

@hppritcha
Copy link
Member

Can one of the admins verify this patch?

@hjelmn
Copy link
Member

hjelmn commented May 11, 2015

I think we still want this but its hard to tell which commits are the PR. I see ffff47f which is simple enough. Are there others?

@bosilca
Copy link
Member Author

bosilca commented Jun 10, 2015

This has been replaced by #633.

@bosilca bosilca closed this Jun 10, 2015
jsquyres pushed a commit to jsquyres/ompi that referenced this pull request Sep 19, 2016
dong0321 pushed a commit to dong0321/ompi that referenced this pull request Feb 17, 2020
Fix some typos and naming errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.