conda-forge / openblas-feedstock

A conda-smithy repository for openblas.
BSD 3-Clause "New" or "Revised" License
9 stars 38 forks source link

Build LAPACK with `-frecursive` #12

Closed jakirkham closed 8 years ago

jakirkham commented 8 years ago

Was seeing this error from OpenBLAS in some BLAS intensive programs. This was often followed by a segfault.

BLAS : Program is Terminated. Because you tried to allocate too many memory regions.

After a little digging, I found this suggestion. As we are using pthreads, this is the right way to go. Rebuilt using this locally and the warning and segfaults went away.

conda-forge-linter commented 8 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

jakirkham commented 8 years ago

Would be curious to hear your thoughts on this, @zym1010.

jakirkham commented 8 years ago

While this works on OS X, it appears we may have some issues to sort out on Linux.

zym1010 commented 8 years ago

@jakirkham well I have no objection, as I never encountered this problem. But seems that this option is a default setting, from https://github.com/xianyi/OpenBLAS/blob/ec4390a967cb57eddff39f1b0bf6f3e45a5387a0/lapack-netlib/INSTALL/make.inc.gfortran#L20. So maybe it's good to turn it on?

Update: -frecursive is not in the default setting. See my comment below.

jakirkham commented 8 years ago

That's interesting. I wonder why it helped then. 😕

Is that Makefile actually used or is it Linux only?

zym1010 commented 8 years ago

@jakirkham well sorry for my mistake. I think that option is not used by OpenBLAS's make by default, as I just compiled the master branch of OpenBLAS and searched through the output, finding no frecursive.

jakirkham commented 8 years ago

No worries. Thanks for looking into it. If OpenBLAS was setting -frecursive by default, I was a bit worried that we might be overriding their default options and thus the real problem remained hidden. Good to know that is not the case.

Did you see if -O2 was being set too? If not, maybe we should be setting that as well.

zym1010 commented 8 years ago

@jakirkham -O2 is all over the place. See https://www.dropbox.com/s/7wr0b96nkp6iuvi/openblas_output_842d842751c98c92d1c32870b10d81bcec9bb232.txt?dl=0 for raw output.

jakirkham commented 8 years ago

Sure. Just wanted to make sure we weren't suppressing it by setting the other Fortran flags.

jakirkham commented 8 years ago

Saw some gfortran options were dropped/overridden by setting FCOMMON_OPT. Have switched to FFLAGS, which seems to alleviate this problem and still ensures -frecursive is passed through.

jakirkham commented 8 years ago

Any thoughts on this, @jschueller?

jschueller commented 8 years ago

No idea, but I found some frecursive flags: https://github.com/xianyi/OpenBLAS/search?utf8=%E2%9C%93&q=frecursive

jakirkham commented 8 years ago

Yeah, though that flag isn't actually getting added to the build normally FWICT. Take a look at @zym1010's comment for an example. The build log is completely devoid of this flag.

jakirkham commented 8 years ago

Thanks everyone for your feedback. As there doesn't seem to be any strong opposition and this is a recommended flag by upstream when building OpenBLAS's multithreaded LAPACK, going to go ahead and merge.