compiler: order the async task handshake - #3009
Open
mloubout wants to merge 1 commit into
Open
Conversation
mloubout
force-pushed
the
fix-async-memory-ordering
branch
from
August 20, 2026 16:15
b54188d to
9bf31a1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3009 +/- ##
==========================================
+ Coverage 83.67% 83.68% +0.01%
==========================================
Files 257 257
Lines 54663 54687 +24
Branches 4683 4683
==========================================
+ Hits 45739 45765 +26
+ Misses 8114 8113 -1
+ Partials 810 809 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mloubout
force-pushed
the
fix-async-memory-ordering
branch
from
August 20, 2026 16:35
9bf31a1 to
991305b
Compare
mloubout
force-pushed
the
fix-async-memory-ordering
branch
2 times, most recently
from
August 20, 2026 18:23
efc7bb8 to
7e6ac61
Compare
The lock and flag an asynchronous task synchronises on are plain volatile ints,
written by both threads with no ordering imposed. `volatile` guarantees the
loads are re-issued; it does not order stores, so on a weakly ordered target
they can be observed out of order and the handshake loses an update:
compute: lock0[0] = 0; (release_lock0) -- observed late
sdata0->flag = 2; (activate0) -- observed first
task: sees the request, delivers lock0[0] = 2, sets flag = 1
compute: the late lock0[0] = 0 lands, wiping the delivery
the next release_lock0 waits for a 2 nobody will write again
Both threads then spin forever: the compute waiting for data whose request it
has already spent, the task waiting to be asked. `lock == 0 && flag == 1` is
reachable only this way -- the task sets the lock before the flag, so a
completed cycle must leave the lock at 2 -- and that is the state a stalled run
sits in.
Fenced on both sides: a release before the flag that publishes a request or a
completion, an acquire before reading what either stands for. `__atomic_thread_fence`
is a builtin, valid in C and C++ alike, so the device targets that render
through CXXPrinter are unaffected; declaring the two objects `_Atomic` would
have been the standard-clean alternative but that is not valid C++ under g++.
Found through a streaming-checkpoint FWI gradient on arm64, where it stalled one
worker in six within minutes and, in the same runs, had CvxCompress trip its own
assertion on a buffer the task never filled. x86's store ordering hides it.
The tests index the two callables positionally, so they move with the fences.
mloubout
force-pushed
the
fix-async-memory-ordering
branch
from
August 20, 2026 18:34
7e6ac61 to
12166a8
Compare
| arguments.append(i) | ||
| activation.extend([DummyExpr(FieldFromComposite(i.base, sdata[d]), i) | ||
| for i in arguments]) | ||
| # The flag is what publishes the request, so everything the thread will |
Contributor
There was a problem hiding this comment.
probably blank lines
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.
The lock and flag an asynchronous task synchronises on are written by both
threads and declared
volatile. That re-issues the loads and orders nothing, sothe handshake can lose an update:
Both threads then spin forever — the compute waiting for data whose request it
has already spent, the task waiting to be asked.
lock == 0 && flag == 1isreachable only this way: the task sets the lock before the flag, so a completed
cycle must leave the lock at 2. That is the state a stalled run sits in.
What's here
ir/iet/nodes.py—ThreadFence, aCallsubclass in the same vein asProdder, so it renders itself and the passes emit one agreed thing ratherthan each hand-rolling a call.
passes/iet/asynchrony.py— a release before the flag that publishes arequest, an acquire before the task reads what it stands for, a release
before the flag that reports completion.
passes/iet/orchestration.py— inrelease_lock, an acquire after the waitand a release after the lock is handed back, before the request that refills
it.
_make_waitlockwants an acquire too, so the body's reads cannot behoisted above the wait, but adding a second node to that
Listchanges theIET's shape — a one-node List is denested, a two-node one is not — and
test_streaming_fusedmatches on that shape. Left out with a note; theordering the deadlock turns on is the store side, which the other five carry.
tests/test_iet.py—test_thread_fence_cgen, that the node renders forboth orders and rejects any other. Runs anywhere.
tests/test_gpu_common.py— the existing lock tests index those twocallables positionally, so they move with the fences; they also now assert the
fences are where they should be.
__atomic_thread_fenceis a compiler builtin valid in C and C++ alike, so thedevice targets that render through
CXXPrinterare unaffected.Two alternatives considered and rejected:
_Atomicis the standard-clean fix and satisfiesThreadSanitizer, where a fence does not — but
_Atomicis not valid C++ underg++ (
'_Atomic' does not name a type), and the qualifier mapper is shared withCXXPrinter, so it would break the CUDA/HIP/SYCL paths.std::atomic<int>isa type rather than a qualifier and cannot come through that mapper.
Fencehierarchy intypes/parallel.py(ThreadCommit,ThreadArrive,ThreadWait) reads like the right vocabulary, but those classify Clusters sopasses cannot reorder across them, and emit nothing.
Reproducer
Drop this in
debug-scripts/and runpython async_handshake_deadlock.py 6 900:a checkpointed wavefield streamed to disk, written by a forward operator and
read back by an adjoint one, six workers for contention. It needs several
workers — one on its own rarely hits the window.
async_handshake_deadlock.py
Same script, same budget, only the compiler differing:
volatile int flag; volatile int lock0[1], 0 fencesDEADLOCK: worker(s) [2] stopped making progress while still runningno deadlock in 900sThe failing run also trips CvxCompress's own assertion —
Decompress: nx=216, ny=256, nz=1, nx_check=0— the compute decompressing abuffer the task never filled. Same lost update wearing its other face: this does
not only hang, it can hand garbage to the decompressor.
Notes
Found through a streaming-checkpoint FWI gradient on arm64, where it stalled one
worker in six within minutes. x86's store ordering hides it.
The
test_gpu_common.pyassertions need a device; devitopro carries anequivalent one for the streaming path, which runs anywhere
(devitocodespro/devitopro#954).