[Mlir-commits] [mlir] [mlir][sparse] fix stack overflow due to alloca instruction inside loops (PR #69765)

[Peiming Liu]

PeimingLiu wrote:

> do stack save/restore doesn't mean that the semantics of the op is different. Semantics are not defined by lowerings, and the lowering may be incorrect. I remember we had very long and inconclusive discussions on whether this lowering should insert stack save/restore. Even if we did remove the trait, it absolutely should not happen in a change tagged "mlir, sparse, fix <...>".

Thanks for the insights, the reason why I need to remove the trait here is due to `AllocaScopeHoister`, which hoist `memref::alloca` from the `alloca_scope` into `scf.for`, leading to incorrect code.

E.g.,
```
scf.for {
  memref.alloca_scope {
     memref.alloca
     xxx
  }
}
```
is canonicalized into 
```
scf.for {
  memref.alloca
  memref.alloca_scope {
    xxx
  }
}
```

Note that this is a correct transformation **provided that `scf.for` indeed adjusts stack pointer**. Since it now does not, what is your suggestion? I think insert `llvm.stack.save` and `llvm.stack.restore` should also works.

I can of course submit the scf.for related change in a separate PR to address your second concern;-)

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