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

Install and use libatomic on windows. #22189

Merged
merged 3 commits into from
Jul 19, 2017
Merged

Install and use libatomic on windows. #22189

merged 3 commits into from
Jul 19, 2017

Conversation

vchuravy
Copy link
Member

@vchuravy vchuravy commented Jun 2, 2017

As mentioned in #20152 (comment) we should use libatomic on windows as well.
So far we have gotten around it but in #21959 the windows CI fails due to the lack of libatomic.

@vchuravy
Copy link
Member Author

vchuravy commented Jun 2, 2017

I am worried that I might be misusing the win-extra build step since libatomic needs to go into usr/bin and usr/lib along the other normally from the system provided libraries.

@tkelman
Copy link
Contributor

tkelman commented Jun 2, 2017

This has had a mean time between breaking of a few months, for several years now. I think what we may need to do is go back to the situation from before #10244 where we use the build system's gcc runtime libraries, rather than replacing them with opensuse's. The windows buildbots aren't going to be on gcc 7 for a while, so openblas, arpack, etc won't be linked against libgfortran-4, they'll still be linked against libgfortran-3. What will require testing is seeing if we have any chance of maintaining compatibility with winrpm packages. Maybe if we upgrade them from gcc 4.9 to 5.4 it'll have a better chance - there was a bug on 32 bit with 5.4 where the cygwin cross-compiler maintainer applied a patch, though it wasn't really the right patch that eventually fixed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77333. There hasn't been a gcc 5.5 release including the proper patch yet. Or we could do something to the effect of #15521 again where we use something saved from juliacache.

This also probably won't help you on appveyor, since it doesn't run make win-extras. That only happens when building an installer, not in a usual source build.

@tkelman
Copy link
Contributor

tkelman commented Jun 2, 2017

Just tested 3119778 using the vagrantfile at https://github.com/JuliaLang/julia/blob/master/contrib/windows/Vagrantfile (which installs the latest gcc 5.4 cross-compiler package from cygwin by default), and at least on win64 both Ipopt.jl and ZMQ.jl passed tests, which is encouraging.

@vchuravy
Copy link
Member Author

vchuravy commented Jun 3, 2017

Ok I can confirm that this works on i686-w64-mingw32 with gcc 5.4.0:

Julia Version 0.7.0-DEV.460
Commit 299178ca59 (2017-06-03 11:14 UTC)
Platform Info:
  OS: Windows (i686-w64-mingw32)
  CPU: Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz
  WORD_SIZE: 32
  BLAS: libopenblas (DYNAMIC_ARCH NO_AFFINITY Nehalem)
  LAPACK: libopenblas
  LIBM: libopenlibm
  LLVM: libLLVM-3.9.1 (ORCJIT, haswell)

I used the test case from #19647 and I am testing right now that all tests pass for #21959

@vchuravy vchuravy force-pushed the vc/win_libatomic branch from 299178c to eca1f1e Compare June 6, 2017 04:08
@vchuravy vchuravy changed the title [WIP/RFC] Install and use libatomic on windows. Install and use libatomic on windows. Jun 6, 2017
@vchuravy
Copy link
Member Author

vchuravy commented Jun 6, 2017

Rebased on the current master which includes #22197.

@tkelman this should be backport friendly, but it is not necessary since I don't plan to backport #21959

@yuyichao
Copy link
Contributor

yuyichao commented Jun 6, 2017

FWIW #21959 shouldn't require this since you can still specify 16bytes alignment for Atomic{Int128}.

@ararslan ararslan added building Build system, or building Julia or its dependencies system:windows Affects only Windows labels Jun 13, 2017
@tkelman
Copy link
Contributor

tkelman commented Jun 13, 2017

I'm a little surprised this worked on appveyor, which is the last windows-related place that we're still using gcc 4.

@vchuravy
Copy link
Member Author

That is indeed odd, but it is installing libatomic correctly

cp /c/projects/julia/mingw32/bin/libatomic-1.dll /c/projects/julia/usr/bin
cp /c/projects/julia/mingw32/bin/libatomic-1.dll /c/projects/julia/usr/tools

and checking the bintray archive: https://bintray.com/artifact/download/tkelman/generic/i686-4.9.2-release-win32-sjlj-rt_v4-rev3.7z there is indeed a mingw32/i686-w64-mingw32/lib/libatomic-1.dll.

@tkelman
Copy link
Contributor

tkelman commented Jun 16, 2017

Oh strange, so that mingw build of 4.9 that we've been using on appveyor (but nowhere else) has it, and the cygwin-cross build of 4.9 (which I'm still using locally, but not on the buildbots any more) doesn't.

@vtjnash vtjnash requested a review from tkelman July 15, 2017 02:01
@tkelman
Copy link
Contributor

tkelman commented Jul 15, 2017

So this will break my personal setup as I've been keeping back to gcc 4.9 from cygwin locally. I'm marginally worried that I may have a harder time building llvm binary updates for appveyor that work correctly if I upgrade locally, and it's hard to undo.

@tkelman
Copy link
Contributor

tkelman commented Jul 15, 2017

I'll see if I can get appveyor switched over to using cygwin instead of the current odd msys hack, which would unblock that concern.

Copy link
Contributor

@tkelman tkelman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep we can merge this now, should be safe - CI failures are unrelated timeouts

@tkelman tkelman merged commit 44f3d7c into master Jul 19, 2017
@tkelman tkelman deleted the vc/win_libatomic branch July 19, 2017 18:32
@vchuravy
Copy link
Member Author

Awesome to see this merged! Thanks Tony for helping me figure this out and pushing it along.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
building Build system, or building Julia or its dependencies system:windows Affects only Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants