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

NAG CMake support is incomplete/broken #5002

Closed
mathomp4 opened this issue Dec 4, 2024 · 2 comments · Fixed by #5003
Closed

NAG CMake support is incomplete/broken #5002

mathomp4 opened this issue Dec 4, 2024 · 2 comments · Fixed by #5003

Comments

@mathomp4
Copy link
Contributor

mathomp4 commented Dec 4, 2024

All,

I recently tried to build OpenBLAS with NAG Fortran using CMake and it was...not happy.

In my testing I've found incomplete NAG support in CMake itself. This was fixable but in doing so, I found there were Fortran Standards issues in:

  • lapack-netlib
  • local ctest code

So now here's the crux. I have a set of fixes for the CMake and the Fortran Standards issues and can issue a PR at anytime, but I'm not sure how to proceed. I'll detail my reasoning below.


CMake

These should be safe to take since, well, without them, NAG just has issues. They are partly bringing stuff from Makefile to CMake systems and partly from changes I found from @ACSimon33 from Reference-LAPACK/lapack#686 (also seen below)

lapack-netlib

The lapack-netlib fixes are all thanks to @ACSimon33 who fixed the same issues in a couple Reference-LAPACK PRs:

But, my question is: How often are Reference-LAPACK changes brought into OpenBLAS? If a sync with the Reference is coming, well, those changes in my PR might be moot as they are fixed "upstream". But if the sync might not happen for a bit, I can put them in my PR.

local ctest code

These changes are caused by #4918 which changed all the STOP calls to CALL ABORT. ABORT is not part of the Fortran Standard though many compilers support it via extensions. But, NAG is very strict and will not support extensions that have equivalents in the Fortran Standard. That equivalent is ERROR STOP.

The main reason I'm hesitant here is I don't know how old of compilers you need to support. ERROR STOP is from Fortran 2008 and while every compiler I use is happy with it, I don't have to build things with GCC 4 or whatever. (The oldest compiler I can find is GCC 7 and it is happy with ERROR STOP.)

So, if you'd prefer to keep CALL ABORT (and thus not touch "working" code), I believe it should be fairly simple to make a routine that is only compiled when NAG Fortran is used that can provide an ABORT routine (that underneath calls ERROR STOP).

@martin-frbg
Copy link
Collaborator

Yeah, seems like I missed both LAPACK PRs back then, mostly due to OpenBLAS not using the original CMakeFiles of Reference-LAPACK. :(
So a PR for these would be appreciated, if it is not too much extra work for you. I usually copy LAPACK PRs either directly when they come out or shortly before building a release, but a full sync may happen only after the next release (already massively delayed due to me catching covid) due to some major changes.

Regarding the CALL ABORT, I had accepted that PR based on the claim that it would be supported everywhere, and the general idea at the time was to merge the Meson build system support (being worked on by the Scipy team) into OpenBLAS. Unfortunately I do not have access to the NAG compiler on any of the systems available to me AFAIK. If the GCC changelogs are to be trusted, ERROR STOP appears to have made its first appearance in gfortran 4.6, which I think should be ancient enough for anybody still keeping outdated systems alive - no need for a special subroutine then.

@mathomp4
Copy link
Contributor Author

mathomp4 commented Dec 4, 2024

@martin-frbg Sounds good. I'll submit my PR so you can review. Heck for all I know the CI will go nuts and more changes would be needed.

As for ERROR STOP if something is needed in the future (due to an older compiler), it should be simple enough for me to "undo" what I did and create an ABORT routine for NAG. (Or, I guess, we can use #ifdef but...ehhhh.

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 a pull request may close this issue.

2 participants