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

Fixes for NAG compiler #43

Merged
merged 10 commits into from
Jun 18, 2024
Merged

Fixes for NAG compiler #43

merged 10 commits into from
Jun 18, 2024

Conversation

DJDavies2
Copy link
Contributor

These fixes seem to enable fckit to build with NAG and run most of the ctests. The changes fall into 3 categories:

  1. Addition of target attribute to some variables

This affects fckit_mpi.fypp, fckit_tensor.F90 and test_tensor.F90. The issue here is that there are variables being passed to array_view1d, a function that returns a point to that same variable. However this only works if the variable is declared with the target in the calling routine. I think NAG is correct here; in Fortran pointers can only point to variables with the target attribute; without the target attribute these pointers are in an undefined state.

  1. Turn fckit_external and fckit_owned into subroutines

NAG doesn't compile these as functions and I think it is a compiler bug. However turning them into subroutines works and doesn't seem to cause any knock-on problems.

  1. Changes to cmake code to enable compiler options to be passed to downstream* ctests

These tests require building Fortran code but compile flags are needed for this to work. These changes do that.

What do you think? I don't think this has any downstream affects. The only possibility is if something outside of fckit is calling fckit_owned or fckit_external directly; I don't know of anything but maybe there is.

Copy link
Member

@wdeconinck wdeconinck left a comment

Choose a reason for hiding this comment

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

This looks like good changes / workarounds. I'll test it a bit further.

@@ -28,7 +28,7 @@ module fckit_shared_object_module
public :: fckit_c_deleter
public :: fckit_c_nodeleter
#if FCKIT_HAVE_ECKIT
public :: fckit_owned
public fckit_owned
Copy link
Member

Choose a reason for hiding this comment

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

This change is a debugging leftover ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, fixed.

@@ -25,8 +25,7 @@ module fckit_shared_ptr_module
#else
use fckit_refcount_module, only : &
& fckit_refcount, &
& fckit_refcount_interface, &
& fckit_external
Copy link
Member

Choose a reason for hiding this comment

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

The removal of fckit_external is strange. Is this required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I have added it back.

@wdeconinck wdeconinck added the approved-for-ci Approved for CI run label Jun 17, 2024
@github-actions github-actions bot removed the approved-for-ci Approved for CI run label Jun 17, 2024
@FussyDuck
Copy link

FussyDuck commented Jun 18, 2024

CLA assistant check
All committers have signed the CLA.

@wdeconinck wdeconinck added the approved-for-ci Approved for CI run label Jun 18, 2024
Copy link
Member

@wdeconinck wdeconinck left a comment

Choose a reason for hiding this comment

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

Thanks! I just added a filter for "--coverage" flag which seems to cause problems for the CI

@wdeconinck wdeconinck merged commit df7f157 into ecmwf:develop Jun 18, 2024
140 of 144 checks passed
@DJDavies2 DJDavies2 mentioned this pull request Jul 12, 2024
wdeconinck added a commit that referenced this pull request Sep 19, 2024
* hotfix/0.13.1:
  Version 0.13.1
  FCKIT_VENV: enable editable installation of python packages
  Prevent macros starting with '__' to be added to fypp
  Prevent string out of bounds. (#46)
  Fixes for NAG compiler (#43)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants