[Mlir-commits] [mlir] [mlir][emitc] Lower SCF using memrefs (PR #93371)

[Gil Rapaport]

aniragil wrote:

> I'm not so clear about the motivation of this change. From what I understand, SCF lowered correctly to emitc before.

Correct, this change is not functional. It's a refactoring trying to separate the concerns of (1) lowering value-carrying control-flow ops to emitc through memory and (2) lowering memory ops to emitc. In fact, we can do (1) within the scf dialect and have `scf-to-emitc` simply reject `scf.{if, for}` with return values.
The `scf-to-emitc` pass was written to generate `emitc.variable` ops directly since we didn't have memref-to-emitc lowering. Now that we do, I think it's better to have all memory modeling in emitc handled in a single pass and have other passes generate memref ops as needed.
Concrete motivation for this PR is to simplify @simon-camp's [lvalue PR](https://github.com/llvm/llvm-project/pull/91475), which currently modifies `scf-to-emitc` even though its reg2mem logic remains the same. This PR replaces that code dependency with a pass order dependency, which seems cleaner.

> The generated C++ code looks different in that we now use arrays with one element instead of scalars to model loop carried values. Why is that better?

This PR promotes scalars to 1-element arrays, which indeed leads to [less natural C/C++ code](https://github.com/llvm/llvm-project/pull/92684#issuecomment-2128949830). Getting back to natural C/C++ code can be achieved by either:
(a) Having `scf-to-emitc` generate rank-0 memrefs and `memref-to-emitc` lower them to `emitc.variable` ops, as suggested [here](https://github.com/llvm/llvm-project/pull/92684).
(b) Having `memref-to-emitc` lower rank-1 memrefs to `emitc.variable` ops where possible. This seems less desired as such generic memory transformations (SROA) belong in the memref dialect (but that leads back to option (a)).

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