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

Add gfortran -frecursive option from upstream and #1847 #1857

Merged
merged 4 commits into from
Nov 7, 2018
Merged

Add gfortran -frecursive option from upstream and #1847 #1857

merged 4 commits into from
Nov 7, 2018

Conversation

brada4
Copy link
Contributor

@brada4 brada4 commented Nov 6, 2018

Most people assume thread-safety from single-threaded calls. This is not the case by default with gfortran. It is hinted already by LAPACK makefile.inc

@brada4
Copy link
Contributor Author

brada4 commented Nov 6, 2018

Should fix #1847 thread safety issue

@martin-frbg
Copy link
Collaborator

I have been meaning to look up the reasoning behind the "only on 64bit Linux" note, for what looks more like a gfortran topic. (And I seem to remember some attempt to eliminate the need for it in Reference-LAPACK a while back).
There probably needs to be a similar change in cmake (fc.cmake ?) as well.

@brada4
Copy link
Contributor Author

brada4 commented Nov 6, 2018

It is still there in reference lapack. Probably needs to be looked at while importing...

@brada4
Copy link
Contributor Author

brada4 commented Nov 6, 2018

@hjbreg OpenBLAS does not have alternative implementation of LAPACK, it just includes a copy of reference implementation. They use -frecursive with gfortran, and now so will we.

@martin-frbg
Copy link
Collaborator

Also discussed in http://icl.cs.utk.edu/lapack-forum/viewtopic.php?t=1930 - the one attempt to get rid of -frecursive in netlib was apparently based on a misunderstanding of the scope of the problem.

@brada4
Copy link
Contributor Author

brada4 commented Nov 6, 2018

I think discussion there goes about gaming compiler allocating (bigger fixed size private) temporary arrays as global structures. Since there is no deep recursion in LAPACK they get allocated on the stack and we are fine.

@martin-frbg
Copy link
Collaborator

Yes, but the entire point of -frecursive is to keep gfortran from creating static storage for temporary arrays that it "thinks" are too big to put on the stack. (Which is probably why this is a gfortran-specific issue - just not limited to multithreaded builds and/or 64bit linux as the comment in Makefile.rule claims)

@brada4
Copy link
Contributor Author

brada4 commented Nov 6, 2018

Exactly.... And in this issue we hit a problem that single-threaded build is presumed thread-safe, when it is not courtesy GCC.

@hjbreg
Copy link

hjbreg commented Nov 7, 2018

Do not know why -frecursive was disabled by the commit 9d6f2b5 from #390

@martin-frbg
Copy link
Collaborator

Either he was using a different fortran compiler at that time (and got a warning about an unknown option) or he misjudged the consequences. Putting it in the middle of an unrelated changeset certainly did not help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants