Nice! Actually last Friday I was looking into seeing whether doing the same would be beneficial for SPIRVOps.cpp. Using -ftime-trace shows that it takes ~19 seconds to cleanly compile it. Most of that time ~13 is spent in the backend. So I guess parallelized compilation can be beneficial here as well. If you aren't planning to do that already, I can split it.

Handle review comments.

Handle review comments:

Handle some formatting comments.

Add a new header for Op traits.

Rebase and bing! :).

Rebase.

Handle some minor clang-tidy comments.

Add missing header guard.

Remove empty line.

Fix file organization issue.

• Split GLSL-specific patterns in their own file.
• Add a test pass to test them.

2 things I still need to figure out/need to make sure I got right:

• How to enable these patterns only when the GLSL.std.450 extension is enabled?
• Whether you think it's suitable to consider these rewrites as canonicalization patterns or we should move them somewhere else?

Add more comments.

Rebase to see if CI bug is resolved.

Probably this hits a CI bug: https://github.com/google/llvm-premerge-checks/issues/268.

Rebase and erase extra space.

Revert unneeded changes.

In D92919#2452713, @antiagainst wrote:

This custom assembly format is okay but I think we can further simplify it given we have very rigid structure inside the region. What about something like

%result = spv.SpecConstantOperation wraps "spv.IAdd"(%lhs, %rhs) : (i32, i32) -> i32

?

The spv.mlir.yield can then be implicitly constructed during parsing and omitted during printing.

We have examples for this in TensorFlow: tf_executor.island. See its parsing and printing function here:

• https://github.com/tensorflow/tensorflow/blob/master/tensorflow/compiler/mlir/tensorflow/ir/tf_executor.cc#L357-L371
• https://github.com/tensorflow/tensorflow/blob/master/tensorflow/compiler/mlir/tensorflow/ir/tf_executor.cc#L321-L335

And examples here: https://github.com/tensorflow/tensorflow/blob/master/tensorflow/compiler/mlir/tensorflow/tests/executor_island_coarsening.mlir#L183-L188

Let me know how do you think. :)

• Add more traits to aid verification.
• Change custom syntax.

Friendly reminder to land this :). I would like to add (de-)serialization support for OpSpecConstantOp and conflicts will be easier to handle if this patch is merged before I start. If you need to wait on this one for a bit more, then no problem I will work on my new patch and handle conflicts later.

Rebase.

• Add IsolatedFromAbove trait to spv.SpecConstantOperation.

Rebase.

Handle review comments.

I don't have commit access. Please commit on my behalf whenever you like.

Rename to spv.mlir.yield.

This is an initial patch to make sure I didn't anything disastrously wrong :D.

Rename to endmodule.

In D91792#2405662, @antiagainst wrote:

What about naming it as spv.mlir.endmodule so it's more akin to spv.mlir.addressof and others? :)

• Rename to moduleend instead.

• Add missing check in deduplication code.

• Handle review comment.
• Rebase.

Fix header guards.

Re-organize files a bit differently.

Fix mistake due to batch editing using sed.

Fix mistake due to batch editing using sed.

Rename to addressof instead of address_of.

I think this should be actually renamed to spv.mlir.addressof to be truly consistent the style used in the llvm dialect. Are there any objections to this? If not, I will do as such.

Thanks for taking the time to review.

Handle further comments.

Handle review comments.

Rebase.

Some minor re-formatting.

Rebase

Abandoned in favor of: https://reviews.llvm.org/D90951

Abandoned in favor of: https://reviews.llvm.org/D90951

General comment regarding this patch as well as this: https://reviews.llvm.org/D90932.

I am not familiar with the implicit conversion rules within the spv dialect (or MLIR in general). But, I was expecting that replacing all uses of bar with foo in the following to snippet to fail:

• Remove unneeded comment.