Move Frontier to CCE 21.0.2 / ROCm 7.2.0 - #1695
Conversation
CCE 21 dereferences the null dope vector an absent optional array presents to the OpenACC data environment, segfaulting s_ibm_correct_state. pb_ts/mv_ts are always allocated (zero-sized when unused), so pass them unconditionally and drop the optional attribute. No present() guards existed in m_ibm, so this is logic-neutral.
AMDGPUPromoteAllocaToVector silently discards a store to arr(v) when arr is 1-based and v is a runtime value. CCE 21 canonicalizes the element index as v + 0x3FFFFFFF, harmless in wrapping address arithmetic but not once the array is registerized and the index becomes an equality comparison, so every test misses and the store vanishes. Here idx(norm_dir) = idx(norm_dir) + 1 never executed and the viscous flux averaged the gradient at cell j instead of j+1. Write the constant index explicitly instead.
ftn builds a flat pointer to a private object as zext(offset) with no aperture. The flat->private cast back is select(p == null, ~0, trunc p), so an object at frame offset 0 yields 0xFFFFFFFF and the scratch store lands 4 GiB out of bounds, faulting every chemistry case on CCE 21. A lower bound of 0 as an unused pad keeps Y_rs(1) off offset 0.
…che.txt is_configured() only checked for CMakeCache.txt, which CMake writes before it generates the build system. A configure interrupted in between made MFC skip configure and fail with 'No rule to make target Makefile'; a dependency build that crashed made MFC skip that dependency forever, leaving CMAKE_PREFIX_PATH pointing at an empty install tree and surfacing as a confusing find_package error elsewhere. Require the generator's build file, and gate the dependency skip on install_manifest.txt, which CMake writes only after a successful install.
Covers the dynamically-indexed private-array stores that the source fix does not reach. The value must stay quoted: modules.sh runs eval export $line, so an unquoted multi-word value exports only the first -plugin-opt and reports the rest to stderr, which is routinely discarded.
On CCE 21.0.2 the presence of any defaultmap clause makes a device-resident array read as all zeros inside the target region. A 60-line reproducer with a derived-type pointer component, a derived-type allocatable component and a bare module array reads all three correctly under a bare directive and gets zero from all three once a defaultmap clause is added; each of tofrom:aggregate, present:allocatable and present:pointer reproduces it alone. Negative controls rule out negative lower bounds and named multi-level exits. This is what broke immersed boundaries under -homp: ib_markers read as empty, so the ghost-point count came back 0, the allocation was zero-length, the explicit-shape dummy mapped as 0 bytes, and the runtime handed the kernel a host pointer to store through. Dropping the clauses is also semantically free, since OpenMP 5.0 already defaults scalars to firstprivate and the AMD branch has never emitted them. Full suite on mp goes 566/627 to 622/627, all 60 aborts eliminated, with both offload paths then failing the identical five tests.
The comment claimed the find_in_present_table failure at m_ib_patches.fpp:145 was a latent MFC residency bug that the tofrom default had been masking. It is not. 'length' is real(wp) :: length(3) and is explicitly listed in that loop's private() clause, so nothing should ever look for it in the present table. Measured on a 20-line standalone: an explicitly-private local array passes with a bare directive and aborts with the identical find_in_present_table error once defaultmap(present:aggregate) is added. That makes it the same compiler defect already described above, overriding an explicit private clause rather than an explicit map or present-table residency.
|
Closing as a duplicate of #1694 — same port, opened in parallel from a second worktree. #1694 is the more complete of the two: it carries the QBMM hoist (so it does not depend on #1688) and a hipfort-tag fix that derives the tag from Withdrawing the performance claim in this description. It stated CCE 21 is 5.8% faster than CCE 19. That was measured with but this branch ships So the honest position is #1694's: the correctness case stands on its own; the performance of the shipped configuration is unmeasured. A clean CCE 19 vs shipped-CCE-21 comparison, one variable at a time and with repeats, has not been run. That error is worth recording rather than just deleting. I compared the fastest CCE 21 configuration I had against CCE 19 and reported it as the port's speedup, without checking that it matched what the branch actually ships. The two flags went into One item here is not in #1694 and should not be lost: Branch |
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Updates the Frontier toolchain and codebase to run correctly (and faster) on CCE 21.0.2 / ROCm 7.2.0, including workarounds for CCE 21 GPU codegen defects and more robust dependency/build skipping logic.
Changes:
- Pin Frontier environment to
cpe/26.03 cce/21.0.2 rocm/7.2.0and add quoted CCE device-link workarounds viaCRAY_CCE_LLD_ARGS. - Fix CCE 21 GPU correctness/crash issues by adjusting OpenACC/OpenMP macro emission and rewriting dynamic local-array indexing patterns.
- Make superbuild dependency skipping depend on successful installation (and improve “configured” detection).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| toolchain/modules | Updates Frontier module set and adds quoted CCE 21 device link/workaround flags. |
| toolchain/mfc/build.py | Improves configured/installed detection to avoid skipping partially configured/failed dependency builds. |
| src/simulation/m_time_steppers.fpp | Always passes IBM arrays to avoid optional-array/null-dope-vector issues with CCE 21 OpenACC. |
| src/simulation/m_riemann_state.fpp | Rewrites dynamic-index updates into constant-index branches to avoid CCE 21 store dropping. |
| src/simulation/m_riemann_solver_lf.fpp | Same constant-index rewrite for LF solver’s “right state” indexing. |
| src/simulation/m_ibm.fpp | Makes pb_in/mv_in non-optional to match unconditional passing and avoid OpenACC null-dope-vector deref. |
| src/common/m_variables_conversion.fpp | Adds padding to keep private array off frame offset 0 and updates slices passed to chemistry routines. |
| src/common/include/parallel_macros.fpp | Fixes Cray inline/noinline directive generation and ensures it coexists with offload directives. |
| src/common/include/omp_macros.fpp | Removes defaultmap usage on CCE due to observed incorrect zero reads / mapping overrides. |
| #:if cray_inline or cray_noinline | ||
| #:if not isinstance(function_name, str) | ||
| #:stop "When using cray_noinline, function name must be given and given as a string" | ||
| #:stop "When using cray_inline/cray_noinline, function name must be given and given as a string" | ||
| #:endif | ||
| #:set cray_noinline_directive = ('!DIR$ NOINLINE ' + function_name).strip('\n') | ||
| #:set cray_keyword = 'INLINEALWAYS' if cray_inline else 'INLINENEVER' | ||
| #:set cray_directive = ('!DIR$ ' + cray_keyword + ' ' + function_name).strip('\n') |
| if (norm_dir == 1) then | ||
| idx_rp(1) = j + 1 | ||
| else if (norm_dir == 2) then | ||
| idx_rp(2) = k + 1 | ||
| else | ||
| idx_rp(3) = l + 1 | ||
| end if |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1695 +/- ##
=======================================
Coverage 61.04% 61.05%
=======================================
Files 83 83
Lines 20978 20984 +6
Branches 3099 3102 +3
=======================================
+ Hits 12807 12812 +5
+ Misses 6126 6125 -1
- Partials 2045 2047 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Moves Frontier off
cce/19.0.0. Context: #1684.Stacked on #1688 (QBMM hoist), which is the tenth fix and is deliberately not duplicated here. QBMM tests abort on CCE 21 until that merges; everything else in this PR is independent of it.
Result
Frontier, CCE 21.0.2 / ROCm 7.2.0, MPI, full suite:
--gpu acc--gpu mpAnd it is 5.8% faster than CCE 19.0.0 — same source, full fix set on both trees, six benchmark cases, faster in every one:
5eq_rk3_weno3_hll5eq_rk3_weno3_hllc5eq_rk3_weno3_lfhypo_hlligrviscous_weno5_sgb_acousticSingle runs, no repeats; the sign is uniform.
-mai-instsis in effect on the CCE 21 side, so the AGPR workaround's cost is already priced in.Worth noting chemistry on GPU does not work at all under CCE 19 — every chemistry case memory-faults there — so this is the first configuration in which those cases run.
The application-side fixes
fix: honor cray_inline/cray_noinline on Cray GPU builds and use INLINENEVER—!DIR$ NOINLINE <name>is not a valid named directive; ftn rejects it with ftn-790. The named form isINLINENEVER. Separately, the directive was being emitted instead of the offload directive rather than alongside it, so on Cray GPU buildscray_noinlinewas silently a no-op. Collapses three near-identical branches into one, −30 LOC.fix: pass IBM pb/mv unconditionally instead of as optional args— an absentoptionalarray reaches the OpenACC data environment as a null dope vector, which CCE 21 dereferences.pb_ts/mv_tsare always allocated (zero-sized when unused), so pass them unconditionally. Nopresent()guards existed inm_ibm, so this is logic-neutral.build: base dependency and configure skips on real state, not CMakeCache.txt— #1689.is_configured()checked only forCMakeCache.txt, which CMake writes before generating the build system and long before installing. An interrupted configure then makes MFC skip configure and fail withNo rule to make target 'Makefile'; a crashed dependency build makes MFC skip that dependency permanently, pointingCMAKE_PREFIX_PATHat an empty install tree. Now requires the generator's build file for "configured" andinstall_manifest.txtfor "installed".This one is not CCE-specific and is worth taking on its own. Over this port it destroyed six batch jobs, produced a false negative in a compiler-flag experiment, broke a control run, and silently voided two benchmark campaigns — every time presenting as a compiler or missing-package problem.
The compiler workarounds
Each of these works around a CCE 21 defect. Reproducers, with verified reproduce steps, are at https://github.com/sbryngelson/compiler-bugs/tree/main/cce.
fix: avoid CCE 21 dropping dynamically-indexed private-array stores—AMDGPUPromoteAllocaToVectorsilently discards a store toarr(v)whenarris 1-based andvis a runtime value. CCE canonicalizes the element index asv + 0x3FFFFFFF, harmless in wrapping address arithmetic but not once the array is registerized and the index becomes an equality comparison.idx(norm_dir) = idx(norm_dir) + 1never executed, so the viscous flux averaged the gradient at celljinstead ofj+1. Three sites rewritten to use constant indices.fix: keep Y_rs off private frame offset 0 in s_compute_pressure— ftn builds a flat pointer to a private object aszext(offset)with no aperture. The flat→private cast back isselect(p == null, ~0, trunc p), so an object at frame offset 0 yields0xFFFFFFFFand the store lands 4 GiB out of bounds. A lower bound of 0 as an unused pad keepsY_rs(1)off offset 0.fix: emit no defaultmap clauses for CCE OpenMP offload— the largest single effect. Anydefaultmapclause makes a device-resident array read as all zeros inside the target region. MFC emitted them on every OpenMPtargetloop as its translation ofdefault(present), soib_markersread as empty, the ghost-point count came back 0, the allocation was zero-length, and the first store faulted through a host pointer. Removing them took the OpenMP suite from 566/627 to 622/627 in one change, with nothing that previously passed regressed. Dropping them is also semantically free — OpenMP 5.0 already defaults scalars to firstprivate, and the AMD branch never emitted them.build: move Frontier to CCE 21.0.2 and ROCm 7.2.0andbuild: add -disable-promote-alloca-to-vector— two-plugin-optvalues inCRAY_CCE_LLD_ARGS. The first works around a back-end assertion inAMDGPU Rewrite AGPR-Copy-MFMAthat blocks the device link outright; the second covers the promote-alloca sites the source fix above does not reach (m_ibm,m_riemann_solver_hll,m_qbmm,m_collisionsall still carry markers). Both are needed: with only the source fix, five tests still fail, one of them by 1.9e-3.The quoting is load-bearing.
modules.sh:126doeseval "export $line", so an unquoted multi-word value exports only the first-plugin-optand reports the rest to stderr, whichmfc.sh loadoutput is routinely discarded. Unquoted, this ships the AGPR workaround and silently loses the correctness fix.Reviewer notes
toolchain/modulespins compiler-internal flags, which will rot. The alternative is patching the remaining promote-alloca sites in source so the flag can be dropped — more work, but not dependent on a flag surviving future CCE releases. Happy to do that instead if preferred../mfc.sh precheckpasses 7/7.m_qbmm.fppis untouched here by design.