[refactor] improved integration of block-gather fast path

[lasch]

#147 introduced a new fast path for block-gather of indirect mem access. At the time it was intentionally kept separate from the existing code paths at the expense of:

• extended internal APIs
• duplication of functionality/code

This PR proceeds with the forked block-gather implementation as-is.
The concerns raised in the discussion of #147 should be subsequently addressed.

• Negative-index guard bug fixed; LX-aliasing fix deferred to a separate PR (conflicted with fix: materialize constant outs in inplace_outs ops (straggling #140 case) #149).
• Reuse-vs-fork, shared validation, and simulator API surface deferred to a new follow-up issue per offline agreement.

[yuhaohaoyu]

Thanks @lasch

On LX-aliasing comments and the postponed addressing (data readonly by default). This is what My agent told me, found it clear and make sense. Thus putting here.

Backgrounds:

1. potential aliasing (contending on write, read, etc.) in _place_in_lx
2. one id(var) treatment appeared, conflicts with my guarding data as 'read only' and only 'gain-writeable, update, release-writeable'. a link lock for 'serial computing'. moved it out due to the ordering of review comments.
3. PR dragging too long, better to merge the meat and trim the edge later.

Quite clear note to move forward for the item 2 of this issue from AI:

---

PR #147 → Issue #171: LX-aliasing & freeze-lx-invariant notes

Lars' original comment (PR #147, Major #1, non-blocking)

_place_in_lx aliasing contract is violated but does not affect existing paths

All three _place_in_lx call sites produce a C-contiguous alias: data, LX.memory[lx_ptr], and Tile.data all point to the same backing buffer. linalg.matmul and linalg.batch_matmul both mutate acc.data in-place with +=, writing through the alias. However, the mutation is intentional (inplace_outs=True) and is a last write before HBM storage. No existing path observes the pre-accumulation LX state. Latent risk remains if a future kernel reuses %C after the matmul as a distinct value, or if a transitive alias via arith.bitcast / tensor.reshape reaches an in-place mutation.

Recommended follow-up: change _place_in_lx to store data.ravel().copy(). This closes all three call sites and transitive paths atomically at the cost of one extra allocation per load.

The three approaches compared

Approach	What it does	Lars' concern addressed?
Lars' suggestion	.copy() at placement — break alias permanently	Yes — no alias exists
PR #149 id(acc)	Detect one specific shared object (uncharged constant), copy on write	Partially — only constants
Our writeable=False	Keep alias, guard with permission flag	Yes — alias exists but writes are gated

• Lars says: "eliminate the alias" (defensive, costs one alloc per load)
• PR fix: materialize constant outs in inplace_outs ops (straggling #140 case) #149 says: "detect the dangerous case by identity" (narrow, misses transitive aliases)
• We say: "keep the alias, control access" (hardware-accurate, zero extra alloc, fails loudly on violations)

PR #149 context

PR #149 (fix/load, merged Jul 16) introduced _accumulate_inplace:

• Fixes: arith.constant used directly as outs of matmul (without scf.for iter_args)
• The shared constant gets mutated across loop iterations → silent data corruption
• Fix: if id(acc) in context._uncharged_tiles: acc = acc.copy()
• This is NOT de-aliasing in Lars' sense — it detects one specific alias (shared constant) by Python object identity

The id(var) approach only handles the constant-as-outs case. It does NOT address:

• The 3-way alias (data / LX.memory[ptr] / Tile.data) from _place_in_lx
• Transitive aliases via bitcast/reshape
• Any future kernel reading %C after matmul mutates it through a non-constant alias

Commits saved for future PR

On branch freeze-lx-invariant (saved before rebase):

• 3076238 — Freeze LX tile data after placement to enforce read-only invariant
• 49893ae — Commenting to disambiguate the permission for in-place acc op: HW allows, SW use unfreeze, freeze to guard

To create standalone PR after #147 merges:

git checkout -B freeze-lx-invariant upstream/main
git cherry-pick 3076238 49893ae
# Resolve conflict with PR #149's _accumulate_inplace (compose both)

Design rationale for writeable over .copy()

The writeable=False flag models physical storage protocols:

• Real SRAM/register files have write-enable pins — you either have permission or you don't
• No copy-on-write exists in scratchpad hardware
• The alias IS the hardware reality (Tile.data IS the LX buffer)
• The flag makes contention observable (numpy raises ValueError on unauthorized write)
• The MAC unfreeze/re-freeze is explicit permission, not a silent workaround

[yuhaohaoyu]

Let's keep it here as one story. @lasch

User Story

From developer's perspective, it's essential to maximize code and name reuse, so minimize the dups in the execution paths for support element-level and block-level indirection access is important.

A branch: fabianlim:ktir-cpu:block-gather-reuse-validation shows that it's likely doable. And it's the time after the 2-versioning approach had established the usefulness of the block-indirection treatment to refactor code for general software engineering practice.

Context / Background

When lifting up a performance optimization feature, the block-indirection emulation, it's useful to keep the necessary redundant code for reviewers to have an integrated view of the change set. Now the feature is merged, it's important to take care of the duplicated code, names, steps, concepts.

Acceptance Criteria

• Pass all indirect-access func tests
• Compare performance of 3 code flows: element-wise indirect treatment, block-indirect treatment from PR 147, and block-indirect treatment after refactoring.

[yuhaohaoyu]

Thanks to @lasch comments in PR #188 It'd be a natural way to move along when both speed and style are in discussion in coding.

1. create a benchmark that can compare speed from different code path.
2. present the speed differences than we will have a converging conversation on moving-forward.

I am adding a small PR, complying w/ Lars' suggestion: simple benchmark that can measure the general path and ongoing code branches for blocked-indirection. (plan to remove the benchmark code associated with pr-188.)

The benchmark PR should defines a simple ruler that measures the speed of indirect-load for realistic access patterns: for a workload, it'd give T_total_indirect_load, matching to MemoryOps::indirect_load() func and T_load, matching to MemoryOps::load(). Use T_offsets = T_total_indirect_load - T_load for the steps computing a flat offsets array.

Acceptance Criteria

• Useful to communicate the benefits of the 2 optimizations on the indirect_load code path
  1. blocked indirection fast path
  2. use gather/scatter in the memaccessor or not to bother (for PR 188).

PR #193 is added to address and only address the specifically purposed need for benchmarking the indirect_load code path.