Skip to content

Avoid NVHPC 25.5 fort2 ICE by expanding f_compute_multidim_cfl_terms - #1718

Open
sbryngelson wants to merge 2 commits into
masterfrom
fix-nvhpc-255-inline-ice
Open

Avoid NVHPC 25.5 fort2 ICE by expanding f_compute_multidim_cfl_terms#1718
sbryngelson wants to merge 2 commits into
masterfrom
fix-nvhpc-255-inline-ice

Conversation

@sbryngelson

Copy link
Copy Markdown
Member

Problem

Building simulation with NVHPC 25.5 and our two-pass IPO crashes the compiler:

nvfortran-Fatal-.../25.5/compilers/bin/tools/fort2 TERMINATED by signal 11
gmake[3]: *** [.../simulation.dir/fypp/simulation/m_data_output.fpp.f90.o] Error 2

Reported by OLCF on a GH200 (Grace + H100, NVHPC 25.5, CUDA 12.9) while running the
scaling benchmark. It is not architecture specific — it reproduces on x86_64 with
the stock NVHPC 25.5 tarball, same signal and same file.

Root cause

fort2 segfaults during the -Minline pass when it cross-file inlines a routine whose
body contains a call to f_compute_multidim_cfl_terms.

The crash belongs to the call site, not the callee. Bisected against a live reproducer:

Body of s_compute_stability_from_dt Result
gutted builds
calls f_compute_filtered_dtheta (scalar args) builds
calls f_compute_multidim_cfl_terms SIGSEGV
...with that helper's body emptied SIGSEGV
...with its array dummy replaced by a scalar SIGSEGV
call removed (helper expanded in place) builds

This also explains why except:f_compute_multidim_cfl_terms does not help: excluding the
callee still leaves the offending call inside the inlined caller. Only excluding the
caller avoids it.

It also explains the second crash site. m_time_steppers calls s_compute_dt_from_cfl,
the sibling routine in the same module with the same call pattern — which is exactly where
the ICE relocates if m_data_output is added to the -Mnoinline list.

Fix

f_compute_multidim_cfl_terms was private with exactly two call sites, so it is expanded
in place and removed. No -Mnoinline, no macro, net −1 line. The resulting
if (p > 0) / else if (n > 0) / else structure matches the viscous and capillary blocks
already present in both routines.

Semantics are unchanged: the original nested logic selected 3D for p > 0 and 2D
otherwise, guarded by if (p > 0 .or. n > 0). Every read of fltr_dtheta in both routines
is still immediately preceded by its own assignment.

Verification

Config Before After
NVHPC 25.5, GPU+MPI, case-optimized, IPO on ❌ SIGSEGV ✅ builds
NVHPC 25.5, same, all -Mnoinline exclusions removed ✅ builds
NVHPC 25.11, same config ✅ (no regression)
cfl_adap_dt golden-file tests (CPU) ✅ 7/7

Full CPU test suite is running locally and was clean through the first ~315/628 cases at
the time of opening; CI covers it here regardless.

Follow-up (not in this PR)

With this fix, NVHPC 25.5 builds cleanly with the existing m_start_up / m_cbc
-Mnoinline exclusions in cmake/MFCTargets.cmake removed entirely. Those were added
for an NVHPC 25.x ICE and look like the same bug. I left them in place here to keep this
change minimal — worth removing separately once CI confirms across the matrix, since it
would restore full IPO to those files.

NVHPC 25.11 compiles the unmodified source fine, so the underlying defect is a 25.5 codegen
bug; this change avoids the construct that triggers it. Worth filing upstream with NVIDIA.

NVHPC 25.5's fort2 segfaults (SIGSEGV) during the -Minline pass of our
two-pass IPO when it cross-file inlines a routine whose body contains a
call to f_compute_multidim_cfl_terms. It manifests as:

  nvfortran-Fatal-.../25.5/compilers/bin/tools/fort2 TERMINATED by signal 11
  gmake[3]: *** [.../simulation.dir/fypp/simulation/m_data_output.fpp.f90.o]

The crash is at the call site, not in the callee: it reproduces with the
helper's body emptied and with its array dummy replaced by a scalar, and
it survives except:f_compute_multidim_cfl_terms (excluding the callee
leaves the offending call inside the inlined caller). Excluding the
caller -- s_compute_stability_from_dt -- is what avoids it.

Because the helper was private with exactly two call sites, expanding it
in place removes the pattern entirely. The resulting
if (p > 0) / else if (n > 0) / else structure matches the viscous and
capillary blocks already present in both routines.

Not architecture specific: originally reported on GH200/aarch64, and
reproduced here on x86_64 with the stock NVHPC 25.5 tarball. NVHPC 25.11
compiles the unmodified source cleanly, so this is a 25.5 codegen bug;
this change simply avoids the construct that triggers it.

Verified:
  - NVHPC 25.5, GPU+MPI, case-optimized: fails before, builds after
  - NVHPC 25.11, same config: builds before and after (no regression)
  - cfl_adap_dt golden-file tests: 7/7 pass
Copilot AI lite review requested due to automatic review settings August 9, 2026 21:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request adjusts the simulation CFL/stability computations to avoid an NVHPC 25.5 fort2 internal compiler error triggered by cross-file inlining of a routine containing a call to f_compute_multidim_cfl_terms. The fix removes the helper function and expands its logic directly at the two call sites in m_sim_helpers, preserving the existing CFL selection behavior while eliminating the problematic call pattern.

Changes:

  • Removed the private helper f_compute_multidim_cfl_terms from src/simulation/m_sim_helpers.fpp.
  • Inlined the 2D/3D inviscid CFL term selection logic directly into s_compute_stability_from_dt and s_compute_dt_from_cfl, including the grid_geometry == 3 (filtered dtheta) path.
  • Added an in-code comment documenting the NVHPC 25.5 -Minline/IPO inlining-triggered crash and why the logic is duplicated.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@sbryngelson

Copy link
Copy Markdown
Member Author

Local verification complete

Ran both full suites against the golden files, plus a GPU run on the compiler that actually has the bug.

Full test suite

Suite Compiler Hardware Result
GPU (--gpu, sm_80) NVHPC 25.5 4× A100 80GB 628 passed / 0 failed (73 min)
CPU (--no-gpu) NVHPC 25.11 Icelake, 128c 626 passed / 2 timeouts → 628/628

The two CPU "failures" were per-test timeouts, not golden-file mismatches, on
2D -> Phase Change model 6 -> 3 Fluid(s) (02BD9B5E, AAE5C27B). They were running while
the GPU suite loaded the same box; re-run unloaded they both pass (~7 min each). No numerical
diffs anywhere in either suite.

The GPU run is the important one: it executes the modified s_compute_stability_from_dt /
s_compute_dt_from_cfl as OpenACC device code under NVHPC 25.5 and compares against the
committed golden files.

Builds

Config Before After
25.5, GPU+MPI, case-optimized, IPO on fort2 SIGSEGV
25.11, same ✅ (no regression)

On removing the inline exclusions

With this fix, NVHPC 25.5 builds cleanly with both the -Mnoinline file list and the
entire except: list removed from cmake/MFCTargets.cmake.

Still not proposing that here, for two different reasons:

  • The m_start_up / m_cbc -Mnoinline entries are ICE workarounds and do look removable,
    but they came from CI (add nvhpc to test suite github runners #1317) which covers a matrix I can't reproduce locally. Worth a
    separate PR so CI decides.
  • The except: entries are a different question. s_mpi_abort, s_prohibit_abort,
    my_inquire, s_int_to_str are error/IO paths you likely don't want inlined everywhere
    regardless of any ICE. "It builds without them" isn't a reason to drop them; that needs a
    code-size/perf argument.

The NVHPC 25.5 fort2 ICE fixed in the previous commit was invisible to
CI, and would have stayed invisible. Two axes never intersect:

  - The NVHPC version matrix (23.11 -> 26.3) builds gpu targets without
    --case-optimization. An unmodified master builds cleanly on 25.5 in
    that configuration, which is why those jobs are green today.
  - The case-optimization jobs run only on self-hosted clusters:
    Phoenix, pinned to nvhpc/24.5, and Frontier, which is CCE/AMD.

So "case-optimized x NVHPC 25.5" was never exercised. Case optimization
hard-codes case parameters into the generated sources, which sharply
increases cross-file inlining pressure in the -Mextract/-Minline pass --
precisely where NVHPC fort2 has repeatedly hit ICEs (see the -Mnoinline
list in cmake/MFCTargets.cmake).

This adds one extra build to the existing gpu matrix jobs. It is
build-only and a single case, because the failure is at compile time and
nothing needs to run.

  - 3D specifically: m_sim_helpers.fpp guards its 3D block with
    "#:if not MFC_CASE_OPTIMIZATION or num_dims > 2", so a case-optimized
    2D build elides the code that crashed.
  - OpenACC only: the two-pass IPO is disabled for OpenMP offload
    (MFCTargets.cmake), so gpu-omp cannot hit this class of bug.

Verified with a local NVHPC 25.5 install, running this exact command:

  - unmodified master: fort2 TERMINATED by signal 11 on m_data_output
  - with the previous commit: builds cleanly
@sbryngelson

Copy link
Copy Markdown
Member Author

Added: CI guard (0410833)

This bug was invisible to CI and would have stayed that way. I confirmed the gap rather than assuming it:

Control experiment — unmodified master, NVHPC 25.5, GPU+MPI, without --case-optimization: builds cleanly. So --case-optimization is required to trigger the ICE, and that is why the existing NVHPC 25.5 (gpu) job is green on master today.

Two CI axes that never intersect:

Axis Coverage Sees this bug?
NVHPC version matrix, 23.11 → 26.3 test.yml:96 — gpu is build-only, no --case-optimization ❌ builds the config proven clean above
Case-opt jobs Phoenix pinned to nvhpc/24.5; Frontier is CCE/AMD ❌ wrong compiler

So case-optimized × NVHPC 25.5 was never exercised.

The guard

One extra build in the existing gpu matrix jobs — build-only, one case, since the failure is at compile time and nothing needs to run:

mfc.sh build -v -j 2 --gpu acc -t simulation \
  -i examples/3D_sphbubcollapse/case.py --case-optimization

Two deliberate constraints:

  • 3D. m_sim_helpers.fpp guards its 3D block with #:if not MFC_CASE_OPTIMIZATION or num_dims > 2, so a case-optimized 2D build elides the code that crashed. A 2D guard would be theater.
  • OpenACC only. MFCTargets.cmake:26 disables the two-pass IPO for OpenMP offload, so gpu-omp cannot hit this class of bug. No point paying for it.

Guard validated in both directions

Using a local NVHPC 25.5 install and this exact command:

Tree Result
unmodified master (7f69cb53) fort2 TERMINATED by signal 11 on m_data_output.fpp.f90
with the fix (1bcc45a0) ✅ builds cleanly

Red before, green after — it actually catches the regression it claims to.

I went with the matrix approach over bumping Phoenix off nvhpc/24.5: it covers every NVHPC release rather than one, and adds a single build to jobs that already exist.

@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.24%. Comparing base (7f69cb5) to head (0410833).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/simulation/m_sim_helpers.fpp 78.57% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1718      +/-   ##
==========================================
- Coverage   61.24%   61.24%   -0.01%     
==========================================
  Files          83       83              
  Lines       20700    20702       +2     
  Branches     3072     3072              
==========================================
+ Hits        12677    12678       +1     
  Misses       5969     5969              
- Partials     2054     2055       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants