[Mlir-commits] [mlir] 7219b1e - [mlir][spirv] Enable validation of selection and phi tests (#166794)

[llvmlistbot at llvm.org]

Author: Igor Wodiany
Date: 2025-11-07T09:29:44Z
New Revision: 7219b1e8a9f741e00f3a1952b66a4ed28d743ffe

URL: https://github.com/llvm/llvm-project/commit/7219b1e8a9f741e00f3a1952b66a4ed28d743ffe
DIFF: https://github.com/llvm/llvm-project/commit/7219b1e8a9f741e00f3a1952b66a4ed28d743ffe.diff

LOG: [mlir][spirv] Enable validation of selection and phi tests (#166794)

Some of the test cases are failing as they introduce unstructured
control flow. It would be good to catch this in MLIR, but I am not sure
it is currently feasible. We could enforce the conditional branch to
always be in `spirv.mlir.selection` however from my perspective it will
break our downstream project as we rely on unstructured control flow,
i.e., the control flow graph is structured but we do not use
`spirv.mlir.selection` and `spirv.mlir.loop` as they do not currently
support early exists. It seems that the support for early exists is
slowly coming into MLIR so we probably can revise what is being enforced
regarding control flow ops in the future.

Added: 
    

Modified: 
    mlir/test/Target/SPIRV/phi.mlir
    mlir/test/Target/SPIRV/selection.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/test/Target/SPIRV/phi.mlir b/mlir/test/Target/SPIRV/phi.mlir
index ca635a469eeaa..92a3387c2f8d4 100644
--- a/mlir/test/Target/SPIRV/phi.mlir
+++ b/mlir/test/Target/SPIRV/phi.mlir
@@ -1,5 +1,10 @@
 // RUN: mlir-translate -no-implicit-module -split-input-file -test-spirv-roundtrip %s | FileCheck %s
 
+// RUN: %if spirv-tools %{ rm -rf %t %}
+// RUN: %if spirv-tools %{ mkdir %t %}
+// RUN: %if spirv-tools %{ mlir-translate --no-implicit-module --serialize-spirv --split-input-file --spirv-save-validation-files-with-prefix=%t/module %s %}
+// RUN: %if spirv-tools %{ spirv-val %t %}
+
 // Test branch with one block argument
 
 spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
@@ -295,15 +300,26 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
     %true = spirv.Constant true
     %zero = spirv.Constant 0 : i32
     %one = spirv.Constant 1 : i32
+    spirv.mlir.selection {
 // CHECK:   spirv.BranchConditional %{{.*}}, ^[[true1:.*]](%{{.*}}, %{{.*}} : i32, i32), ^[[false1:.*]]
-    spirv.BranchConditional %true, ^true1(%zero, %zero: i32, i32), ^false1
+      spirv.BranchConditional %true, ^true1(%zero, %zero: i32, i32), ^false1
 // CHECK: [[true1]](%{{.*}}: i32, %{{.*}}: i32)
-  ^true1(%arg0: i32, %arg1: i32):
-    spirv.Return
+    ^true1(%arg0: i32, %arg1: i32):
+      spirv.Return
 // CHECK: [[false1]]:
-  ^false1:
+    ^false1:
+      spirv.Return
+    ^merge:
+      spirv.mlir.merge
+    }
+
+    spirv.Return
+  }
+
+  spirv.func @main() -> () "None" {
     spirv.Return
   }
+  spirv.EntryPoint "GLCompute" @main
 }
 
 // -----
@@ -314,15 +330,26 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
     %true = spirv.Constant true
     %zero = spirv.Constant 0 : i32
     %one = spirv.Constant 1 : i32
+    spirv.mlir.selection {
 // CHECK:   spirv.BranchConditional %{{.*}}, ^[[true1:.*]], ^[[false1:.*]](%{{.*}}, %{{.*}} : i32, i32)
-    spirv.BranchConditional %true, ^true1, ^false1(%zero, %zero: i32, i32)
+      spirv.BranchConditional %true, ^true1, ^false1(%zero, %zero: i32, i32)
 // CHECK: [[true1]]:
-  ^true1:
-    spirv.Return
+    ^true1:
+      spirv.Return
 // CHECK: [[false1]](%{{.*}}: i32, %{{.*}}: i32):
-  ^false1(%arg0: i32, %arg1: i32):
+    ^false1(%arg0: i32, %arg1: i32):
+      spirv.Return
+    ^merge:
+      spirv.mlir.merge
+    }
+
+    spirv.Return
+  }
+
+  spirv.func @main() -> () "None" {
     spirv.Return
   }
+  spirv.EntryPoint "GLCompute" @main
 }
 
 // -----
@@ -333,13 +360,24 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
     %true = spirv.Constant true
     %zero = spirv.Constant 0 : i32
     %one = spirv.Constant 1 : i32
+    spirv.mlir.selection {
 // CHECK:   spirv.BranchConditional %{{.*}}, ^[[true1:.*]](%{{.*}} : i32), ^[[false1:.*]](%{{.*}}, %{{.*}} : i32, i32)
-    spirv.BranchConditional %true, ^true1(%one: i32), ^false1(%zero, %zero: i32, i32)
+      spirv.BranchConditional %true, ^true1(%one: i32), ^false1(%zero, %zero: i32, i32)
 // CHECK: [[true1]](%{{.*}}: i32):
-  ^true1(%arg0: i32):
-    spirv.Return
+    ^true1(%arg0: i32):
+      spirv.Return
 // CHECK: [[false1]](%{{.*}}: i32, %{{.*}}: i32):
-  ^false1(%arg1: i32, %arg2: i32):
+    ^false1(%arg1: i32, %arg2: i32):
+      spirv.Return
+    ^merge:
+      spirv.mlir.merge
+    }
+
     spirv.Return
   }
+
+  spirv.func @main() -> () "None" {
+    spirv.Return
+  }
+  spirv.EntryPoint "GLCompute" @main
 }

diff  --git a/mlir/test/Target/SPIRV/selection.mlir b/mlir/test/Target/SPIRV/selection.mlir
index 44625cc299230..12daf68538d0a 100644
--- a/mlir/test/Target/SPIRV/selection.mlir
+++ b/mlir/test/Target/SPIRV/selection.mlir
@@ -1,5 +1,10 @@
 // RUN: mlir-translate -no-implicit-module -test-spirv-roundtrip -split-input-file %s | FileCheck %s
 
+// RUN: %if spirv-tools %{ rm -rf %t %}
+// RUN: %if spirv-tools %{ mkdir %t %}
+// RUN: %if spirv-tools %{ mlir-translate --no-implicit-module --serialize-spirv --split-input-file --spirv-save-validation-files-with-prefix=%t/module %s %}
+// RUN: %if spirv-tools %{ spirv-val %t %}
+
 // Selection with both then and else branches
 
 spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
@@ -136,19 +141,31 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
 
 // CHECK-NEXT:    spirv.Load "Function" %[[VAR]]
     %cond = spirv.Load "Function" %var : i1
+    spirv.mlir.selection {
 //      CHECK:    spirv.BranchConditional %1, ^[[THEN1:.+]](%{{.+}} : i32), ^[[ELSE1:.+]](%{{.+}}, %{{.+}} : i32, i32)
-    spirv.BranchConditional %cond, ^then1(%one: i32), ^else1(%zero, %zero: i32, i32)
+      spirv.BranchConditional %cond, ^then1(%one: i32), ^else1(%zero, %zero: i32, i32)
 
 // CHECK-NEXT:  ^[[THEN1]](%{{.+}}: i32):
 // CHECK-NEXT:    spirv.Return
-  ^then1(%arg0: i32):
-    spirv.Return
+    ^then1(%arg0: i32):
+      spirv.Return
 
 // CHECK-NEXT:  ^[[ELSE1]](%{{.+}}: i32, %{{.+}}: i32):
 // CHECK-NEXT:    spirv.Return
-  ^else1(%arg1: i32, %arg2: i32):
+    ^else1(%arg1: i32, %arg2: i32):
+      spirv.Return
+    ^merge:
+      spirv.mlir.merge
+    }
+
     spirv.Return
   }
+
+  spirv.func @main() -> () "None" {
+    spirv.Return
+  }
+  spirv.EntryPoint "GLCompute" @main
+  spirv.ExecutionMode @main "LocalSize", 1, 1, 1
 }
 
 // -----