We observed that there seems to be some inconsistency in the assumptions of what operation folders are allowed to do and when a “fold” is considered to be successful.
Operation::fold returns success, even for in-place folding, while OpBuilder::tryFold does only return success when the operation can be erased after this.
Apart from being confusing, this seems to break some internals of the dialect conversion framework. The dialect conversion uses tryFold in an attempt to legalize an operation. In our downstream case, a in-place fold is triggered, which in the process legalized the operation. Sadly, the dialect conversion does not realize that the operation changed and then attempts to convert the already legal op.
This issue of course requires addressing in the dialect conversion, but it might also be sensible to change the OpBuilder::tryFold API to be more consistent with the Operation::fold function. Any opinions on this?