[Mlir-commits] [mlir] [mlir][spirv] Split conditional basic blocks during deserialization (PR #127639)

[Igor Wodiany]

================
@@ -105,6 +105,8 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
     %var = spirv.Variable : !spirv.ptr<i1, Function>
 // CHECK-NEXT:    spirv.Branch ^[[BB:.+]]
 // CHECK-NEXT:  ^[[BB]]:
+// CHECK:    spirv.Branch ^[[BB:.+]]
+// CHECK-NEXT:  ^[[BB]]:
----------------
IgWod-IMG wrote:

This is a good point, but I don't think it'll be much of an issue outside running `--test-spirv-roundtrip` that is only used for testing. The splitting only happens in the deserializer, so it won't affect any lowering that use SPIR-V, etc. Then when it comes to deserialization, it shouldn't be an issue when going directly from SPIR-V generated from outside MLIR (GLSL, etc.). I would expect the code to always have something meaningful to split without introducing superfluous blocks.

Now why do we get extra blocks with the roundtrip? I serialized the MLIR code from the test you commented and got (trimmed):

```
          %4 = OpLabel
         %12 = OpVariable %_ptr_Function_bool Function
               OpBranch %13
         %13 = OpLabel
               OpSelectionMerge %16 None
               OpBranchConditional %true %14 %15
         %14 = OpLabel
               OpStore %12 %true
               OpBranch %16
         %15 = OpLabel
               OpStore %12 %false
               OpBranch %16
         %16 = OpLabel
```

After serializing we are getting an extra block (%13) that would be unlikely to be present in non-MLIR generated SPIR-V, as OpSeletionMerge would be part of the predeceasing block. Actually, if you think about this is something my patch do, it isolates OpSeletionMerge and OpBranchConditional, so it shows my approach in deserialization matches how serializer works. Now because the block is already split, deserializng it again does further splitting creating superfluous blocks.

So, yes extra blocks are possible, but I think that would only happen if the input SPIR-V is already split and I don't think that would happen often with the upstream code. But even if it does happen, this only introduces some direct branches, which I believe are easy to optimise somewhere down the line - just collapse blocks together.

Hope that makes sense!

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