Skip to content

Add amdflang to CI #248

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 19 commits into
base: develop
Choose a base branch
from
Open

Add amdflang to CI #248

wants to merge 19 commits into from

Conversation

@samhatfield
Copy link
Collaborator

No description provided.

@samhatfield samhatfield force-pushed the add_amd-flang branch 3 times, most recently from f9d311f to ef22e1a Compare April 2, 2025 08:52
@samhatfield samhatfield added enhancement New feature or request actions labels Apr 2, 2025
@samhatfield samhatfield force-pushed the add_amd-flang branch 10 times, most recently from bde3d37 to 9494959 Compare April 2, 2025 17:27
@samhatfield
Copy link
Collaborator Author

samhatfield commented Apr 3, 2025 •
edited
Loading

Status:

  • MPI has been disabled for amdflang. Somehow the OpenMPI built by amdflang can't be found by CMake. We will work on this in a future PR.
  • Otherwise all tests are passing with amdflang except for ectrans_test_install which segfaults when running transi_sptogp. This is very similar to what we saw with IntelLLVM actually, which is very suspicious. However I cannot yet debug this crash interactively as I can't build a working amdflang on ECMWF's systems (I could probably build amdflang on my Mac but I have a feeling it's OS-specific). Will liaise with @PaulMullowney about how to debug this further. - Edit: now working by setting stack size to unlimited.

@samhatfield samhatfield force-pushed the add_amd-flang branch 4 times, most recently from 39317d6 to bf23883 Compare April 4, 2025 12:38
@samhatfield samhatfield requested a review from wdeconinck April 4, 2025 12:39
@samhatfield
Copy link
Collaborator Author

Note #249 must be merged into this in order for the amdflang tests to pass.

wdeconinck
wdeconinck approved these changes Apr 10, 2025
View reviewed changes
Copy link
Collaborator

@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 is great! Thanks also for the install-amd-flang script. Very useful.

@marsdeno
Copy link
Collaborator

Excellent!

@PaulMullowney
Copy link
Contributor

We should hook the GPU build in, next. It will runtime fail, but at least we'll be covering compilation.

@samhatfield
Copy link
Collaborator Author

Note #249 must be merged into this in order for the amdflang tests to pass.

Correction: #249 must be merged into this in order for the amdflang tests to run. In this branch the TESTS feature is disabled for amdflang because we are building without MPI. After #249 is merged to develop (and I rebase this branch) we will be able to enable TESTS without MPI and we can then check whether the test suite passes for amdflang.

@wdeconinck
Copy link
Collaborator

Let's merge #249 first regardless

samhatfield and others added 17 commits September 9, 2025 13:08
@samhatfield @PaulMullowney
Add amdflang to CI
882fc7f 
Co-authored-by: Paul Mullowney <[email protected]>
@samhatfield
Allow matrix CMake options to take precedence for overrides
98b5ab6
@samhatfield
Disable MPI for amdclang
8ee0eb3
@samhatfield
Add -fPIC option for amdflang
2953b01
@samhatfield
Add -fopenmp for amdclang in CI
ee5fd8f
@samhatfield
Increase stack size to unlimited for ectrans_test_install
45ab57f
@samhatfield
Revert "Add -fopenmp for amdclang in CI"
8d63917 
This reverts commit 63839fc.
@samhatfield
Use OpenMP_{lang}_FLAGS
ce0ef58
@samhatfield
Update amdflang to 7.0.5
43a26ca
@samhatfield
Update reference to amdflang
2c08902 
amdflang-new has now become amdflang.
@samhatfield
Add llvm/lib to library load path for amdflang
1ed8b90
@samhatfield
Remove references to "NEW"
54a2257
@samhatfield
Hide progress bar
985985f
@samhatfield
Sort out version selection
ae3345f
@samhatfield
Ensure LD_LIBRARY_PATH is on GITHUB_ENV
3bb07bb
@samhatfield
Simplify DIR_TRANS adjoint test
b740f95 
This should reduce the pressure on the stack.
@samhatfield
Simplify INV_TRANS adjoint test
9fa4f15
@wdeconinck
Copy link
Collaborator

Could this be of inspiration to try putting automatic arrays on the heap?
https://fortran-lang.discourse.group/t/flang-new-how-to-force-arrays-to-be-allocated-on-the-heap-a-possible-bug/7824

@samhatfield
Copy link
Collaborator Author

Could this be of inspiration to try putting automatic arrays on the heap? https://fortran-lang.discourse.group/t/flang-new-how-to-force-arrays-to-be-allocated-on-the-heap-a-possible-bug/7824

I can certainly try adding that flag.

@samhatfield samhatfield force-pushed the add_amd-flang branch 2 times, most recently from 03917a7 to 04a4082 Compare September 17, 2025 12:50
@samhatfield samhatfield force-pushed the add_amd-flang branch 3 times, most recently from d639036 to 16af3f6 Compare September 17, 2025 12:54
@samhatfield
Copy link
Collaborator Author

Hey @PaulMullowney - is there an online manpage somewhere for AFAR? amdflang doesn't recognise

flang-22: error: unknown argument: '-fdynamic-heap-array'

and so I'm wondering if there's an equivalent option for amdflang.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wdeconinck wdeconinck wdeconinck approved these changes

Assignees

No one assigned

Labels

actions enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@samhatfield @marsdeno @PaulMullowney @wdeconinck