Repository navigation
Conversation
Motivation
Naming an NVHPC NVTX wrapper explicitly breaks GPU linking when its library name changes.
Expectation
Let the compiler driver select its NVTX wrapper and include cmake changes in the CI build filter.
Change
For CUDA >= 12.9 the targets linked nvhpcwrapnvtx by name in addition
to -cudalib=nvtx3. NVHPC 26.9 renamed that library to libnvhpcnvtx, so
a GPU build stops at its first link (syscheck):
/usr/bin/ld: cannot find -lnvhpcwrapnvtx
-cudalib=nvtx3 already links the wrapper under each release's name
(-lnvhpcwrapnvtx on 25.9 and 26.3, -lnvhpcnvtx on 26.9, per
nvfortran -dryrun), so the explicit name is dropped and a comment
records why.
Also add cmake/** to the cmakelist filter in file-filter.yml. It lists
CMakeLists.txt and toolchain/cmake/** but not the cmake/ directory
added in MFlowCode#1552, so a PR that only changes cmake/*.cmake skipped every
build. Same line as in MFlowCode#2019.
Result
The release-dependent hardcoded wrapper name is removed. The precheck passes. The older CUDA::nvToolsExt path is unchanged.
Co-authored-by: ruru25254649 <ruru.2525.4649@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WPhkY8p1EHhGY7jScizJmC
platelett
requested review from
danieljvickers
and removed request for
a team
October 25, 2026 09:56
|
Claude Code Review Head SHA: 4870e05 Files changed:
Findings: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2111 +/- ##
=======================================
Coverage 61.95% 61.95%
=======================================
Files 86 86
Lines 22906 22906
Branches 3378 3378
=======================================
Hits 14192 14192
Misses 6206 6206
Partials 2508 2508 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GPU builds (
--gpu accor--gpu mp) with NVHPC 26.9 fail at their first link:libnvhpcnvtx, andMFCTargets.cmakenames the old one.-cudalib=nvtx3flag already on that line links the wrapper under each release's own name.cmake/changes; it is the same line as Fix compiler configuration, build-cache identity, and CMake CI coverage #2019.Change
cmake/MFCTargets.cmake. For CUDA ≥ 12.9 (noCUDA::nvToolsExt), the targets linked the wrapper twice:The first line is removed. The driver adds the wrapper itself:
-cudalib=nvtx3 -dryrunlink line... -lnvhpcwrapnvtx ...... -lnvhpcnvtx ...A comment records this next to the flag. The CUDA ≤ 12.8 branch (
CUDA::nvToolsExt) is unchanged..github/file-filter.yml. It adds'cmake/**'to thecmakelistfilter. The filter listsCMakeLists.txtandtoolchain/cmake/**, but not thecmake/directory added in #1552. As a result, a PR that changes onlycmake/*.cmake, like this one, skips the build and test matrix. This is the same line as #2019, so whichever lands second merges cleanly. I can drop this line if you prefer.Side effects
-dryrunon 25.9 and 26.3, and a clean 26.3--gpu accbuild showslibnvhpcwrapnvtx.soinldd, as before.cmake/changes, which costs more CI time for such PRs.Verification
On 2× H100 NVL:
--gpu acc--gpu accnsys(./mfc.sh run ... --nsys,2D_riemann_test, 50 steps)TIMESTEP×50,COMPUTE-RHS×150,RHS-RIEMANN-SOLVER×300,RHS-RECONSTRUCTION×300--gpu accsuite, NVHPC 26.9On an RTX 4060 Ti (sm_89):
--gpu accld: cannot find -lnvhpcwrapnvtx--gpu acc, clean build directorysyscheckandpre_processbuild, andlddshowslibnvhpcwrapnvtx.sonsysNVTX summary, 64² 2D case, 20 steps, NVHPC 26.9TIMESTEP×20,COMPUTE-RHS×60,RHS-RIEMANN-SOLVER×120,RHS-RECONSTRUCTION×120The 47 hypoelasticity failures on 26.9 are unrelated to this PR:
34F3999Bgivesabs: 1.56E-02, rel: 1.56E-05in both.--reldebugand--debugbuilds pass, with no bounds errors../mfc.sh precheckpasses.Contribution Policy
All contributions are expected to demonstrate:
If these expectations are not met, we would prefer to implement the changes ourselves rather than spend time reviewing low-effort submissions.
Acknowledgement
This PR is from Diablo (Tsinghua University) participating in the SC26 Student Cluster Competition.