[Mlir-commits] [mlir] Check linalg.generic arguments to prevent crashing when they are deleted (PR #119110)

[Andrzej Warzyński]

banach-space wrote:

> it is a bit unclear if I need an attention of code owners and how to get it.

Sadly, there are no code-owners in MLIR. Things are bound to improve, see:
*  https://discourse.llvm.org/t/rfc-mlir-project-charter-and-restructuring/

However, it will take same time and I suspect you'd like your fix to land sooner than that ;-) I'd normally just ping whoever touched relevant code most recently, but I haven't noticed any familiar (i.e. "active") names. I can try my best to help instead. Mehdi is one of our core maintainers, so we should definitely make sure we act on his feedback.

Now, I see that things go wrong in this hook: https://github.com/llvm/llvm-project/blob/f85579fb510faa0a57500b8fd3642f0269c4a4a1/mlir/lib/Transforms/RemoveDeadValues.cpp#L616-L637

Clearly, what's currently happening is incorrect:
1. Compute liveness analysis.
2. Walk overall all Ops in a module and remove them if they are not alive and have no memory effects.

Your test case demonstrates that this logic brakes if an operand of an Op is removed before the corresponding Op is processed. The logic needs to be updated.

> Two walks within the pass, one to compute and one to delete? Does not look natural, maybe there is an analysis like Liveness which could be relied on.

It won't be as compact, but it will be correct. Correctness is much higher priority. I'd do it in two steps (this is probably what you had in mind):
1. Walk over all Ops within a module and "collect" the ones that are not live and have no memory effects. Lets call the corresponding container (probably a vector): `opsToErase`. 
2. Go over all Ops in `opsToErase` and remove them (without checking for memory effects).

Perhaps you will have to process "simple" Ops and "everything else" (e.g. functions) separately. I am almost definitely missing some fine details or nuance here ;-)

You may need to experiment a bit. Ultimately, you want to make sure to address Mehdi's feedback and to preserve current functionality.

Once you have an updated version, I am happy to review and we can try pinging somebody more active in this area (though no guarantee they will reply).

Thanks for working on this!

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