Nice! Thanks!

Rebase

Thank you, Adrian!

https://github.com/llvm/llvm-project/commit/a28fe17d7315f72b802b4ac4b4bc1603ffe7a23b

Rebase

Remove empty line

Address the comments.

Thank you, Adrian!

Thanks, Ben!

Awesome

In D101861#2738284, @bkramer wrote:

Looks good (if Nicolas signed off on this before he went on vacation)

Update.

Thank you, Tobias!

Ok.

In D101749#2733182, @ftynse wrote:

In D101749#2733171, @herhut wrote:

In D101749#2733073, @ftynse wrote:

I am missing the rationale for this one. Maybe "roundtrip" is a bad name, but we have these tests for all dialects and the point has always been to check that MLIR prints what you expect. Without the checks, you only test that MLIR can parse back what it printed, but you lose the consistency check. Back when we had to write parsers manually, I remember having caught several cases where an ill-written parser would accept A and B keywords, but construct an op always with A; so it would be printed with A instead of B and parsed back successfully (thus passing the trimmed-down test) but would contain an error.

Not all dialects have this and with the now auto-generated parser/printer pairs using assemblyFormat I am not convinced of the value of these tests. They do create work when evolving syntax. On the other hand, as we have them, unless there is a change to syntax, we can also keep them.

I checked, and all upstream dialects do have those. Some just call the file ops.mlir, which I personally consider a cargo cult of the prehistorical MLIR version that had core-ops.mlir for testing the syntax of currently Affine+Standard+Math+Memref ops. Many dialects also check attribute and type syntax there.

While I do agree that it creates churn to have these tests if the printers/parsers are generated, Linalg printers and parsers are not!

We might need a separate pass that lowers only TiledLoopOp and not the linalg ops inside its region. But for now it should be fine. Thanks!

Why do we create single-operand bcasts at all?

Fix lowering to Affine loops.

Nice, thanks!

@ThomasRaoux Yes, this is dangerous, if the "inner" builder was created without rewriter.getListener() passed to it.

More comments.

Address the comments.

Thank you for cleaning this up, Thomas!