Skip to content

lint: refuse an array element passed to a device routine that runs a seq loop - #1816

Merged
sbryngelson merged 4 commits into
MFlowCode:masterfrom
sbryngelson:lint/device-routine-element-args
Sep 4, 2026
Merged

lint: refuse an array element passed to a device routine that runs a seq loop#1816
sbryngelson merged 4 commits into
MFlowCode:masterfrom
sbryngelson:lint/device-routine-element-args

Conversation

@sbryngelson

@sbryngelson sbryngelson commented Sep 4, 2026

Copy link
Copy Markdown
Member

Closes #1815

Enforces the workaround for #1815 so it cannot be reintroduced.

CCE OpenACC (19.0.0 through 21.0.2, -O2) miscompiles one pairing: a device routine that contains an acc loop at any level (seq included; the conforming loop vector in routine vector fails the same way), called with an array element as an actual argument. The element is read as garbage and never written back; either ingredient alone is fine, and CCE OpenMP offload of the same shape is correct. The pairing is what #1811 hit when it added Newton and RK4 loops to helpers that kernels called with q_prim_vf%vf(i)%sf(j,k,l) and blkmod1(k,l,q).

check_device_routine_element_args in lint_source.py: collect the GPU_ROUTINEs that contain a GPU_LOOP, close that set over device routines that call them (CCE inlines the chain, so the loop counts wherever it ends up), then flag any call to one of them, inside a GPU_PARALLEL_LOOP body or another device routine, whose argument is exactly an element reference such as a(i) or q(i)%sf(j,k,l). Expressions (a(i)/max(b(i), eps)), intrinsic and f_* calls, array sections (w(:,q)), whole arrays and host call sites pass. Contained procedures scope correctly.

Validation: master is clean; #1811 before its fix (c167a0aa) gets 38 findings in exactly the seven files that were changed by hand (m_rhs, m_hypoelastic, m_riemann_solver_hypo_hlld, m_acoustic_src, m_ibm, m_pressure_relaxation, m_variables_conversion); #1811 after its fix (1ee119b9) is clean. Four unit tests: flagged element in and out, flagged through a caller, host call site not flagged, scalars/expressions/loop-free routines not flagged.

One paragraph in contributing.md; the mechanism itself is documented in .claude/rules/common-pitfalls.md on #1811 and reproduced standalone in sbryngelson/compiler-bugs cce/acc-routine-element-by-reference.

…seq loop (Cray OpenACC miscompiles the pair)
Copilot AI lite review requested due to automatic review settings September 4, 2026 03:22

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.

Warning

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.

Pull request overview

Adds a new source-lint rule (with tests + contributor guidance) to prevent reintroducing a known Cray OpenACC miscompile pattern: passing array elements into device routines that contain (or inline) acc loop seq-equivalent loops.

Changes:

  • Implement check_device_routine_element_args in lint_source.py and wire it into the lint runner.
  • Add unit tests covering flagged vs allowed call sites and propagation through device-routine call chains.
  • Document the rule/workaround in docs/documentation/contributing.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
toolchain/mfc/lint_source.py Adds new linter logic to detect/flag element actual-args into “tainted” device routines and integrates it into main().
toolchain/mfc/test_lint_source.py Adds unit tests exercising the new lint rule across host/device/kernels and call-chain propagation.
docs/documentation/contributing.md Documents the new constraint and links it to the relevant issue + linter enforcement.

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

Comment thread toolchain/mfc/lint_source.py Outdated
Comment thread toolchain/mfc/lint_source.py Outdated
Comment thread toolchain/mfc/lint_source.py Outdated
Comment thread docs/documentation/contributing.md Outdated
@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.26%. Comparing base (8010057) to head (3262cbb).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1816   +/-   ##
=======================================
  Coverage   62.26%   62.26%           
=======================================
  Files          84       84           
  Lines       21558    21558           
  Branches     3188     3188           
=======================================
  Hits        13423    13423           
  Misses       5937     5937           
  Partials     2198     2198           

☔ 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.

@sbryngelson
sbryngelson merged commit 7f78b45 into MFlowCode:master Sep 4, 2026
78 of 86 checks passed
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.

CCE OpenACC 19-21: an acc loop inside a routine seq plus an array-element actual argument reads garbage and drops the store

2 participants