[Bugfix] Resolve reg offset on dynamic add_offset chains

[Phil-amd]

Summary

resolveRegOffset accumulated add_offset displacements without checking whether each link was static, so a register access through a dynamic offset was promoted to a hardcoded read of slot 0.

Motivation

A dynamic IntAttr carries value == 0 (FlyAttrDefs.cpp), so IntAttr::getValue() on a dynamic offset silently returns 0 instead of failing. Before this change, add_offset(reg_ptr, dynamic) lowered to:

%0 = ub.poison : vector<4xf32>
%1 = vector.extract %0[0] : f32 from vector<4xf32>

The dynamic index is dropped entirely and the access is hardcoded to slot 0 — a deterministic read of the wrong element, not merely an undefined value.

Changes

• resolveRegOffset now returns the root alloca plus an optional offset. The root is always reported, so the recast-exclusion walk and collectTouchedRegAllocaInRegion keep their conservative behaviour; only getRegAccessInfo, which needs a concrete slot index, bails out.
• Allocas reached through a dynamic chain are excluded from promotion up front, the same way the pass already excludes recast-reachable ones. Guarding the offset alone was not sufficient: the alloca would still be promoted while the unresolvable access stayed in memory form, tripping the pass's own post-condition check (register operand/result remain after rmem SSA promotion).

No kernel in the tree currently hits this path — a scan of 15 compiled kernels found no register-space add_offset with a dynamic offset — so this is a latent-correctness fix rather than a behaviour change for existing code.

Performance (if applicable)

Not applicable. Per-kernel ISA resource comparison against the base commit shows no change: gfx942 33 kernels and gfx950 31 kernels, all identical in VGPR/SGPR/spill/scratch/ds_read.

Testing

• Unit tests added/updated — tests/mlir/Transforms/promote_regmem_to_ssa_invalid.mlir gains @skip_register_dynamic_offset, verified to fail without the source change and pass with it
• Performance benchmarks run — not applicable, see above
• Tested on MI300X — full MLIR FileCheck suite (36/36) on gfx942; clang-format clean

Dependencies

• No new third-party dependencies added

Breaking Changes

None.

[sjfeng1999]

From a performance perspective, I do not think dynamic offsets on register pointers/tensors should be supported. A runtime offset often prevents promotion to vector-SSA/register and may force a private-memory or spill fallback.

Since this storage is intended to remain register-backed, it would be better to reject the case with a clear diagnostic instead of silently compile it.

[Phil-amd]

From a performance perspective, I do not think dynamic offsets on register pointers/tensors should be supported. A runtime offset often prevents promotion to vector-SSA/register and may force a private-memory or spill fallback.

Since this storage is intended to remain register-backed, it would be better to reject the case with a clear diagnostic instead of silently compile it.

From a performance perspective, I do not think dynamic offsets on register pointers/tensors should be supported. A runtime offset often prevents promotion to vector-SSA/register and may force a private-memory or spill fallback.

Since this storage is intended to remain register-backed, it would be better to reject the case with a clear diagnostic instead of silently compile it.

Thanks comment — I agree with the underlying concern: silently spilling register-backed storage to scratch would be a bad outcome, and an invisible performance cliff is worse than a compile-time error. I checked the generated code before responding, however, and the situation is narrower than “a runtime offset forces a private-memory fallback.”

The fallback is real, but only beyond LLVM’s promotion threshold. An unpromoted alloca does lower to addrspace(5), but AMDGPUPromoteAlloca promotes smaller allocations to vectors accessed with extractelement. I measured the following on gfx950 using arrays populated at runtime to prevent constant folding (opt -O2, then llc):

array size	scratch_store	scratch_load	result
4	0	0	registers (extractelement)
8	0	0	registers
16	0	0	registers
32	0	0	registers
64	16	1	scratch

For up to 32 elements, the alloca is eliminated after -O2, and the final ISA contains no scratch traffic. Rejecting every dynamic offset on a register pointer would therefore also reject code that currently compiles to register-only ISA. If we want a diagnostic, it should be based on whether promotion can succeed — for example, on the allocation’s size or shape — rather than simply on the presence of a dynamic offset.

On scope: this PR fixes a silent miscompile. Previously, add_offset(reg_ptr, dynamic) was promoted to vector.extract %v[0]: because a dynamic IntAttr has value == 0, the index was discarded, and the access silently and deterministically read the wrong element. Making this case a hard error would be a reasonable policy, but it is separate from fixing the incorrect read, and I would prefer not to couple the two decisions. The miscompile exists today, whereas the diagnostic would have no immediate in-tree effect: across 15 compiled kernels, I found no register-space add_offset with a dynamic offset.

My proposal is to land this PR as the correctness fix, then follow up with the dynamic vector.extract path and a size-based diagnostic.

[sjfeng1999]

The rest looks good.