[Mlir-commits] [mlir] [mlir][mem2reg] Promote whole-buffer memref to a vector SSA value (PR #211880)

[Andrzej Warzyński]

banach-space wrote:

Hi @Jianhui-Li!

> When a vectorized `scf.for` carries a reduction or accumulation tensor — the C accumulator of a contraction, or a reduction buffer — bufferization materializes that loop-carried tensor into a scratch memref, and `promote-buffers-to-stack` turns it into a small `memref.alloca`.

This statement in the summary makes me wonder whether this change may be solving a problem for which MLIR already has relevant machinery 😅 Let me explain.

As I understand it, the motivation for this change can be summarised as:

* What happens to a contraction accumulator _after_ bufferization?

Bufferization introduces aliasing, which is notoriously tricky to analyse. Rather than recovering the SSA representation after bufferization, could this problem be addressed at the tensor level, before aliasing is introduced?

MLIR already provides several potentially relevant transformations:

* https://mlir.llvm.org/docs/Dialects/Transform/#transformloophoist_loop_invariant_subsets-transformhoistloopinvariantsubsetsop
* https://mlir.llvm.org/docs/Dialects/Transform/#transformbufferizationbuffer_loop_hoisting-transformbufferloophoistingop
* https://mlir.llvm.org/docs/Dialects/Transform/#transformstructuredhoist_redundant_vector_transfers-transformhoistredundantvectortransfersop

Funnily enough, @steplong, @egebeysel, and I have recently gone through the exercise of identifying the right set and ordering of transformations to ensure that this kind of hoisting happens:

* https://github.com/llvm/llvm-project/issues/201562

In other words, if the existing hoisting machinery does not work for the motivating example, I think it would be useful to first open an issue with the relevant input IR and pipeline and discuss why it fails. That would help establish whether this PR addresses a genuinely distinct problem or works around a missing canonicalization or an incomplete transformation elsewhere.

Could you provide a minimal end-to-end example showing:

1. the tensor-level IR before bufferization;
2. the relevant transformation pipeline;
3. the resulting allocation and transfers; and
4. why the existing hoisting transformations cannot eliminate them?

More generally, this transformation appears fairly specialised, and some of its functionality may overlap with existing tensor-level or vector-transfer hoisting. Perhaps I am missing an important distinction, but based on the current motivation I am not yet convinced that the upstream community should maintain this additional mechanism.

As a side note, I find the summary quite long and difficult to follow. It contains implementation details that distract from the central design question. For example:

> The machinery is already type-agnostic — `MemorySlot` carries an arbitrary `elemType`, and `scf.for` already threads a slot of any type as an `iter_arg`/result — so no changes to the pass or to SCF are needed.

A generic transformation such as this would normally be expected to be element-type-agnostic, so I do not think this detail adds much to the motivation. I would suggest shortening the summary and focusing it on:

* the precise problem;
* why existing transformations do not solve it;
* the proposed abstraction; and
* the intended scope and limitations.

Given the disclosed substantial AI assistance, I would also ask that the final summary be carefully edited by the author so that it communicates the design and motivation directly, rather than documenting every implementation detail.


https://github.com/llvm/llvm-project/pull/211880