[Mlir-commits] [mlir] [mlir][vector] Fix round-trip parsing of operand-less vector.print (PR #231851)

[Federico Bruzzone]

FedericoBruzzone wrote:

@purnima-nlp yes, it would change existing IR. I still think it's the better fix.

I study a bit these days. Some history. `vector.print` has always printed a newline after the value (54759cef). `punctuation` came later (f36e909d), for `VectorToSCF`. `newline` was made the default just to keep the old output and not touch a lot of tests. That default is behind both problems here. The printer drops it, so an op with no operand comes out as bare `vector.print`, and the parser mistakes a following SSA operand for its source. It's also why `vector.print` and `vector.print punctuation <newline>` mean the same thing, as Matthias noticed.

So instead of a custom directive or parentheses, I'd drop the default. Every print would need a source, a string or an explicit `punctuation`, and bare `vector.print` would be **invalid**. The printer always has something to print and there's nothing left to confuse.

The catch is that `vector.print %x : T` would stop printing a newline, so tests and downstream users would need to add `punctuation <newline>`. Easy to do, but bigger than this fix. If we want to discuss the impact on downstream users, I can open an RFC on Discourse.

Let's wait for @banach-space and @matthias-springer before changing the PR.

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