improve: sharper Verrou dd_line attribution (source filter, function scope, -fno-inline)#1437
Merged
sbryngelson merged 3 commits intoMay 14, 2026
Conversation
…function scoping Three changes to reduce spurious dd_line reports pointing at do-loop boundaries, #:endfor lines, or inlined-function call sites: 1. _CONTROL_FLOW_RE + _is_arithmetic_loc: post-filter dd_line results by reading the actual source line; skip pure control-flow delimiters (end do, end if, #:endfor, $:END_GPU_PARALLEL_LOOP, etc.) that share DWARF info with the preceding arithmetic due to template expansion or inlining. 2. restrict_syms in _write_dd_run_sh / _run_dd_line: when dd_sym has already identified responsible functions, dd_line_run.sh embeds --source-function=<sym> flags so Verrou only perturbs FP ops inside those functions, narrowing the bisection space. 3. FFLAGS=-fno-inline in fp-stability.yml: prevents gfortran from inlining small helpers into their callers, so DWARF correctly attributes arithmetic to the callee source line rather than the caller loop header.
sbryngelson
force-pushed
the
improve/verrou-attribution
branch
from
3671682 to
7bbeab7
Compare
dd_run.sh never forwarded the VERROU_EXCLUDE and VERROU_SOURCE env vars injected by verrou_dd_sym and verrou_dd_line respectively. Without these, dd_sym perturbs all functions uniformly (over-inclusive results) and dd_line cannot narrow to individual source lines. Also fix _parse_rddmin_syms: it was returning every non-empty line including the 'ddmin0: Fail Ratio...' metadata lines. These got embedded literally as --source-function flags (not a valid Verrou flag), crashing the dd_line reference run and producing an empty rddmin_summary. Remove the broken restrict_syms / --source-function approach entirely. Verrou's own bisection protocol handles scope restriction via env vars. Also exclude fp-stability-logs/ from typos and gitignore since Verrou generates files containing 'unamed_filename_verrou' (its internal token).
…e patterns
The _CONTROL_FLOW_RE filter was only applied to dd_line results; the
cancellation check was passing its locs through unfiltered, producing
subroutine declarations, else-if branches, and loop headers in the step
summary alongside real arithmetic sites.
Changes:
- Apply _is_arithmetic_loc filter to cancellation_locs in
_run_cancellation_check; log how many control-flow boundaries skipped.
- Extend _CONTROL_FLOW_RE to also catch:
- else / else if (...) then branches
- subroutine declarations (recursive/pure/elemental variants)
Contributor
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1437 +/- ##
=======================================
Coverage 65.00% 65.00%
=======================================
Files 72 72
Lines 18810 18810
Branches 1553 1553
=======================================
Hits 12227 12227
Misses 5615 5615
Partials 968 968 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
1 participant
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.
Summary
Verrou
dd_linesometimes reports loop boundaries (end do,#:endfor,$:END_GPU_PARALLEL_LOOP) or inlined-function call sites instead of the actual arithmetic responsible for FP instability. Three targeted fixes:Source-line content filter (
_CONTROL_FLOW_RE/_is_arithmetic_loc): afterdd_lineproduces itsrddmin_summary, each reported location is checked against the actual.fppsource line. Pure control-flow delimiters that share DWARF debug info with the preceding arithmetic (loop boundaries, fypp directive closers, blank/comment lines) are dropped with a diagnostic note.Restrict
dd_linescope todd_symfunctions (restrict_symsin_write_dd_run_sh/_run_dd_line): whendd_symhas already identified the responsible functions, the generateddd_line_run.shembeds--source-function=<sym>flags so Verrou only perturbs FP operations inside those functions. This prevents the bisection from landing on#:for-template lines shared across loop iterations or code inlined from unrelated callees.FFLAGS=-fno-inlineinfp-stability.yml: prevents gfortran from inlining small helpers (e.g.s_compute_speed_of_sound) into their callers. Without this, DWARF attributes the inlined ops to the caller's loop header line rather than the callee's actual arithmetic.Test plan
./mfc.sh precheckpasses (enforced by pre-commit hook — already verified locally)fp-stability.ymlCI run passes with the new build flagdd_lineresults point to arithmetic lines (assignments, expressions) rather thanend door#:endforboundaries