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

Set correct no bounds check for Intel and gfortran only for Debug build #219

Merged
merged 3 commits into from
Jan 25, 2024

Conversation

TimothyCera-NOAA
Copy link
Contributor

Older versions of gfortran don't accept "-fcheck=no-bounds" but do accept "-fno-bounds-check" even though the later form is deprecated.

… build.

Older versions of gfortran don't accept "-fcheck=no-bounds" but do accept
"-fno-bounds-check" even though the later form is deprecated.
@AlexanderRichert-NOAA
Copy link
Contributor

Good catch, thanks. This looks good, my only question is if you could do a loop over the three files to reduce the number of lines? Something like foreach(filename fftpack.F sptranf.f sptranfv.f) so we don't need three of each 'set_source_files_properties'.

@AlexanderRichert-NOAA
Copy link
Contributor

I tested on my personal machine, and it doesn't work with gcc 6-9; it appears that it wants the "new" version of the argument, i.e., "-fcheck=no-bounds". With gcc 5 the build fails with an unrelated error, so if it sounds reasonable, can we take out the else block for GNU and just use the newer flag? Obviously let me know if you've tested it and got different results.

@TimothyCera-NOAA
Copy link
Contributor Author

The newer flag didn't work with gfortran 5.3 on Windows when packaging with conda. Conda only has that version available for Windows. Maybe set the if statement to decide which format to use to 6?

Flang is also available with conda on Windows, but I thought introducing another compiler just for Windows would be more difficult and confusing than just dealing with an ancient version of gfortran.

@AlexanderRichert-NOAA
Copy link
Contributor

Setting the cutoff to 6 is fine with me. I don't think any of our Linux platforms/users are going to use gcc 5 or older.

@AlexanderRichert-NOAA
Copy link
Contributor

This looks good, thanks again. I'll do one or two more tests and assuming it all works I'll merge in the next little while.

Do you urgently need these changes? At some point I'm going to do a 5.1 release but I don't have a timeline, so if you needed this very soon we could do a 5.0.1 release.

@TimothyCera-NOAA
Copy link
Contributor Author

No need for a new release. Thanks for asking.

@AlexanderRichert-NOAA AlexanderRichert-NOAA merged commit 5aa4807 into NOAA-EMC:develop Jan 25, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants