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

Workaround for Cray compiler bug involving NULL() intrinsic #1560

Merged
merged 4 commits into from
Oct 31, 2024

Conversation

J-Lentz
Copy link
Contributor

@J-Lentz J-Lentz commented Jul 26, 2024

Description
The Cray compiler contains a bug where the NULL() intrinsic fails to produce a pointer of the correct type when it is passed directly to a subroutine expecting a pointer to a derived type.

This PR implements a workaround where NULL() is assigned to a variable which is passed to the subroutine, rather than calling NULL() directly as a subroutine argument.

How Has This Been Tested?
Builds with Cray compiler 15.0.1 on C5.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New check tests, if applicable, are included
  • make distcheck passes

A workaround has been introduced for a bug in the Cray compiler, where the
NULL() intrinsic fails to return a pointer of the correct type. Rather than
using NULL() directly as a subroutine argument, it is assigned to a variable
which is passed to the subroutines.
@rem1776
Copy link
Contributor

rem1776 commented Aug 28, 2024

@J-Lentz Just a heads up, I'm going to push an empty commit to this PR to re-trigger the CI. It looks like it only failed due to #1480.

@J-Lentz
Copy link
Contributor Author

J-Lentz commented Sep 6, 2024

This bug should be fixed in the next CCE release in a few months, so perhaps this PR doesn't need to be merged.

@J-Lentz J-Lentz closed this Sep 12, 2024
@laurenchilutti
Copy link
Contributor

We should reopen this PR

@J-Lentz J-Lentz reopened this Oct 24, 2024
@J-Lentz J-Lentz requested a review from uramirez8707 as a code owner October 24, 2024 16:13
rem1776
rem1776 previously approved these changes Oct 24, 2024
@@ -224,6 +224,7 @@ integer function fms_register_diag_field_obj &
integer, allocatable :: file_ids(:) !< The file IDs for this variable
integer :: i !< For do loops
integer, allocatable :: diag_field_indices(:) !< indices where the field was found in the yaml
class(diagDomain_t), pointer :: null_ptr => NULL() !< Workaround for a Cray compiler bug
Copy link
Contributor

Choose a reason for hiding this comment

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

can you rename this variable to like null_diag_domain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in eb10e18.

Copy link
Contributor

@bensonr bensonr left a comment

Choose a reason for hiding this comment

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

Will approve once the change requested by @uramirez8707 is addressed.

@rem1776 rem1776 merged commit 180f866 into NOAA-GFDL:main Oct 31, 2024
28 checks passed
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.

5 participants