Skip to content

compiler: Fix performance profiling of busy-waits - #3055

Merged
FabioLuporini merged 6 commits into
mainfrom
fix-wait-timing
Oct 22, 2026
Merged

FabioLuporini merged 6 commits into
mainfrom
fix-wait-timing

Conversation

@FabioLuporini

Copy link
Copy Markdown
Contributor

No description provided.

@FabioLuporini FabioLuporini self-assigned this Oct 14, 2026
@FabioLuporini FabioLuporini added the bug-C-minor bug in the generated code not affecting correctness label Oct 14, 2026
if size == 1:
continue
if v.is_double_buffering:
# E.g., `f[time] = b[t0]` must become `b1[t0] = b[t0]`

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

actually this is a bug fix (tested both here and in PRO)

@codecov

codecov Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.34711% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.25%. Comparing base (078bfe3) to head (b494165).

Files with missing lines Patch % Lines
devito/passes/clusters/buffering.py 55.55% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3055      +/-   ##
==========================================
+ Coverage   84.15%   84.25%   +0.10%     
==========================================
  Files         258      258              
  Lines       57267    57398     +131     
  Branches     4893     4911      +18     
==========================================
+ Hits        48191    48361     +170     
+ Misses       8249     8211      -38     
+ Partials      827      826       -1     
Flag Coverage Δ
pytest-gpu-aomp-amdgpuX 68.48% <70.41%> (+0.08%) ⬆️
pytest-gpu-gcc- 78.89% <22.72%> (-0.07%) ⬇️
pytest-gpu-icx- 78.81% <22.72%> (-0.09%) ⬇️
pytest-gpu-nvc-nvidiaX 69.19% <88.31%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@FabioLuporini
FabioLuporini force-pushed the fix-wait-timing branch 2 times, most recently from bd1fd36 to ea8fc74 Compare October 15, 2026 08:39
@FabioLuporini
FabioLuporini force-pushed the fix-wait-timing branch 2 times, most recently from 8d1a1fa to d07489c Compare October 21, 2026 08:59
FabioLuporini and others added 6 commits October 21, 2026 11:19
Select compute_100a/sm_100a in CudaCompiler for detected SM100 devices.
Use explicit Blackwell as the SM100 fallback when device detection is
unavailable, and preserve other detected targets.

Explicit gencode avoids the generic compute_100 PTX fallback generated by
-arch=sm_100a, which rejects architecture-specific instructions such as
tcgen05. Target selection no longer needs a downstream pass override.

Add hardware-independent coverage of native fallback, older GPUs, SM100,
and other Blackwell targets.
Reuse the first buffer's slot indices when inserting further buffering
levels, including initialization outside the time loop. Preserve physical
irregular-buffer slots in copy-in masks and flatten transfers using their
allocated stride independently of the transferred payload size.

Add a subsampled multilevel buffering regression for degrees 1, 2 and 4.
The lock and flag an asynchronous task synchronises on are plain volatile ints.
The fences added for them order nothing on their own: a release fence pairs with
an acquire fence only through atomic accesses, so a weakly ordered CPU may e.g.
see a lock released before the data it guards. Access every lock and flag with
__atomic acquire loads and release stores, which are valid in C and C++ and
need no declaration change, and drop the now redundant ThreadFence.
iet, _ = super()._make_waitlock(iet, sync_ops, *args)
waitloop, *body = iet.body

# NVHPC 25.7 miscompiles tiled loops following an inline atomic wait

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.

Is this documented anywhere? Perhaps a link to said documentation wouldn't go amiss?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

what documentation? it's an actual NVC compiler bug, like several we had to workaround in the past..

Comment on lines +75 to +76
flag = FieldFromComposite(sdata.symbolic_flag, sdata[d])
condition = CondNe(atomic_load(flag), 1)

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.

Hoist outside the if block?

Comment thread tests/test_arch.py
(BLACKWELL, 103, '-arch=sm_103'),
(BLACKWELL, 120, '-arch=sm_120')
])
def test_cuda_target(monkeypatch, platform, cc, target):

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.

Is there a better approach than using monkeypatching?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

in general probably yes, in this specific case I'm not so sure tbf

@FabioLuporini
FabioLuporini merged commit a9e3f31 into main Oct 22, 2026
62 of 63 checks passed
@FabioLuporini
FabioLuporini deleted the fix-wait-timing branch October 22, 2026 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-C-minor bug in the generated code not affecting correctness

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants