Restore upstream safety conditions on loop-parallel stores#262
Draft
AntonOresten wants to merge 1 commit into
Draft
Restore upstream safety conditions on loop-parallel stores#262AntonOresten wants to merge 1 commit into
AntonOresten wants to merge 1 commit into
Conversation
The token-ordering pass's loop-parallel store optimization accepted any transitive dependence on the induction variable as an 'injective' store index — including non-injective derivations like (i + 1) ÷ 2 — and dropped upstream's may_alias_internally guard entirely. Both allowed stores from different iterations to hit the same memory with no happens-before edge between them (a WAW race). - Replace is_iv_derived with an injective-affine matcher: the index tuple must contain the IV composed only of addi/subi with a loop-invariant operand, muli by a nonzero constant, or exti. This still sees through Julia's 1-based '.- One()' index lowering (the reason the port used transitive dependence), while rejecting iv ÷ 2, iv % k, min(iv, c), and friends. - Reinstate the internal-overlap guard: ArraySpec gains a MayAliasInternally parameter (whether two distinct in-bounds indices can map to the same memory, e.g. a zero stride). compute_array_spec derives it exactly from the runtime sizes/strides via the standard non-overlapping strided layout criterion; hand-written specs default to false, matching upstream cuTile's signature default. The pass keeps iteration-ordering tokens unless the stored array provably cannot alias itself internally. Matches Python's _get_parallel_stores / _filter_by_store_index (token_order.py:462-538). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Problem
The token-ordering pass's loop-parallel store optimization (which lets a store in a
forbody consume the pre-loop token instead of a loop-carried one) dropped two of upstream Python's safety conditions:is_iv_derivedaccepted any transitive IV dependence — including non-injective derivations like(i + 1) ÷ 2,i % k,min(i, c). Upstream's_filter_by_store_indexrequires the index to literally be the induction variable. Two iterations mapping to the same tile then store with no happens-before edge between them: a WAW race with a nondeterministic winner.may_alias_internallyguard was dropped — an array whose layout maps two distinct in-bounds indices to the same memory (e.g. a zero stride, easily produced by adapting a broadcast-style wrapper) races even with an identity-IV index. The Julia port had no equivalent concept.Fix
is_iv_derived→ injective-affine matcher: the index tuple must contain the IV composed only ofaddi/subiwith a loop-invariant operand,muliby a nonzero constant, orexti. Julia's 1-basedstorealways lowers the index throughiv - 1, so upstream's bare-identity check would never fire here — the affine whitelist is what upstream's ownTODO: allow more complex injective checkanticipated. Anything else keeps the token carry (conservative = correct).ArraySpecgains aMayAliasInternallyparameter.compute_array_specderives it exactly from runtime sizes/strides (standard non-overlapping strided layout criterion — stricter than Python, which trusts a user-declared flag); hand-written specs default tofalse, matching upstream's signature default.get_parallel_storesresolves the store's alias root to itsTileArrayargument spec and bails unless the layout provably cannot self-overlap.sci.argtypesis threaded through the transform chain so the pass can consult argument specs.Tests
New
test/codegen/token_order.jl:2i(affine) stores stay loop-parallel (store consumes the entrymake_token, noiter_values)(i + 1) ÷ 2keeps the loop-carried token +join_tokensmay_alias_internally = truespec keeps the carry even with an identity IVlayout_may_alias_internally(zero/repeated/negative/padded strides) and thecompute_array_specderivationRan locally:
codegen/*(all),types,analysis/dataflow,host/cache,device/core,examples/softmax— all pass on 2× RTX 6000 Ada.🤖 Generated with Claude Code