[mlir][SPIRV] Add SPIRVToLLVM conversion for selection with yielding values by secona · Pull Request #210600 · llvm/llvm-project

secona · 2026-07-19T14:51:44Z

The current SPIRVToLLVM conversion for SelectionOp does not handle merge blocks with yielding values. This change implements that by adding arguments to the continue block in the SelectionPattern.

Closes #204714

…values

llvmorg-github-actions · 2026-07-19T14:52:19Z

@llvm/pr-subscribers-mlir-spirv

@llvm/pr-subscribers-mlir

Author: Vito Secona (secona)

Changes

The current SPIRVToLLVM conversion for SelectionOp does not handle merge blocks with yielding values. This change implements that by adding arguments to the continue block in the SelectionPattern.

Closes #204714

Full diff: https://github.com/llvm/llvm-project/pull/210600.diff

2 Files Affected:

(modified) mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp (+8)
(modified) mlir/test/Conversion/SPIRVToLLVM/control-flow-ops-to-llvm.mlir (+56)

diff --git a/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp b/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
index c5f9305a5c226..2982061c957e0 100644
--- a/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
+++ b/mlir/lib/Conversion/SPIRVToLLVM/SPIRVToLLVM.cpp
@@ -1470,6 +1470,14 @@ class SelectionPattern : public SPIRVToLLVMConversion<spirv::SelectionOp> {
     auto position = rewriter.getInsertionPoint();
     auto *continueBlock = rewriter.splitBlock(currentBlock, position);
 
+    // Add arguments to the continue block for selections that yield values.
+    for (auto ty : op.getResultTypes()) {
+      Type dstTy = getTypeConverter()->convertType(ty);
+      if (!dstTy)
+        return rewriter.notifyMatchFailure(op, "failed to convert type");
+      continueBlock->addArgument(dstTy, loc);
+    }
+
     // Extract conditional branch information from the header block. By SPIR-V
     // dialect spec, it should contain `spirv.BranchConditional` or
     // `spirv.Switch` op. Note that `spirv.Switch op` is not supported at the
diff --git a/mlir/test/Conversion/SPIRVToLLVM/control-flow-ops-to-llvm.mlir b/mlir/test/Conversion/SPIRVToLLVM/control-flow-ops-to-llvm.mlir
index ec47dd31cc637..389a21681bafb 100644
--- a/mlir/test/Conversion/SPIRVToLLVM/control-flow-ops-to-llvm.mlir
+++ b/mlir/test/Conversion/SPIRVToLLVM/control-flow-ops-to-llvm.mlir
@@ -214,6 +214,62 @@ spirv.module Logical GLSL450 {
     %one = spirv.Constant 1 : i32
     spirv.ReturnValue %one : i32
   }
+
+  spirv.func @selection_with_yielding_value(%cond: i1) -> i32 "None" {
+    // CHECK: llvm.cond_br %{{.*}}, ^bb1, ^bb2
+    %0 = spirv.mlir.selection -> i32 {
+      spirv.BranchConditional %cond, ^true, ^false
+    // CHECK: ^bb1:
+    ^true:
+      // CHECK: %[[C1:.*]] = llvm.mlir.constant(1 : i32) : i32
+      %cst1 = spirv.Constant 1 : i32
+      // CHECK: llvm.br ^bb3(%[[C1]] : i32)
+      spirv.Branch ^merge(%cst1 : i32)
+    // CHECK: ^bb2:
+    ^false:
+      // CHECK: %[[C2:.*]] = llvm.mlir.constant(2 : i32) : i32
+      %cst2 = spirv.Constant 2 : i32
+      // CHECK: llvm.br ^bb3(%[[C2]] : i32)
+      spirv.Branch ^merge(%cst2 : i32)
+    // CHECK: ^bb3(%[[ARG:.*]]: i32):
+    ^merge(%1: i32):
+      // CHECK: llvm.br ^bb4(%[[ARG]] : i32)
+      spirv.mlir.merge %1 : i32
+    }
+    // CHECK: ^bb4({{.*}}):
+    %one = spirv.Constant 1 : i32
+    spirv.ReturnValue %one : i32
+  }
+
+  spirv.func @selection_with_multiple_yielding_values(%cond: i1) -> i32 "None" {
+    // CHECK: llvm.cond_br %{{.*}}, ^bb1, ^bb2
+    %0:2 = spirv.mlir.selection -> i32, i32 {
+      spirv.BranchConditional %cond, ^true, ^false
+    // CHECK: ^bb1:
+    ^true:
+      // CHECK: %[[C1:.*]] = llvm.mlir.constant(1 : i32) : i32
+      %cst1 = spirv.Constant 1 : i32
+      // CHECK: %[[C3:.*]] = llvm.mlir.constant(3 : i32) : i32
+      %cst3 = spirv.Constant 3 : i32
+      // CHECK: llvm.br ^bb3(%[[C1]], %[[C3]] : i32, i32)
+      spirv.Branch ^merge(%cst1, %cst3 : i32, i32)
+    // CHECK: ^bb2:
+    ^false:
+      // CHECK: %[[C2:.*]] = llvm.mlir.constant(2 : i32) : i32
+      %cst2 = spirv.Constant 2 : i32
+      // CHECK: %[[C4:.*]] = llvm.mlir.constant(4 : i32) : i32
+      %cst4 = spirv.Constant 4 : i32
+      // CHECK: llvm.br ^bb3(%[[C2]], %[[C4]] : i32, i32)
+      spirv.Branch ^merge(%cst2, %cst4 : i32, i32)
+    // CHECK: ^bb3(%[[ARG:.*]]: i32, %[[ARG1:.*]]: i32):
+    ^merge(%1: i32, %2: i32):
+      // CHECK: llvm.br ^bb4(%[[ARG]], %[[ARG1]] : i32, i32)
+      spirv.mlir.merge %1, %2 : i32, i32
+    }
+    // CHECK: ^bb4({{.*}}):
+    %one = spirv.Constant 1 : i32
+    spirv.ReturnValue %one : i32
+  }
 }
 
 // -----

IgWod

Looks good in general, just few comments on the test.

IgWod · 2026-07-20T08:35:47Z

+    }
+    // CHECK: ^bb4({{.*}}):
+    %one = spirv.Constant 1 : i32
+    spirv.ReturnValue %one : i32


Would it make more sense to return %0, so that the yielded value is being used?

I've made it return %0 in 82837f0

IgWod · 2026-07-20T08:42:59Z

+    }
+    // CHECK: ^bb4({{.*}}):
+    %one = spirv.Constant 1 : i32
+    spirv.ReturnValue %one : i32


Same comment as in the other test.

spirv.func does not return multiple value. What do you think about adding the two yielded selection value then returning? Its implemented in 2669281

IgWod · 2026-07-20T08:46:21Z

+      // CHECK: llvm.br ^bb4(%[[ARG]], %[[ARG1]] : i32, i32)
+      spirv.mlir.merge %1, %2 : i32, i32
+    }
+    // CHECK: ^bb4({{.*}}):


Should it explicitly check for 2 values so it matches // CHECK: llvm.br ^bb4(%[[ARG]], %[[ARG1]] : i32, i32)?

I think it should. I've also edited the yield 1 value test, so its clearer that its 1 value. edb1949

IgWod

LGTM, just two final comments.

IgWod · 2026-07-22T12:24:53Z

+      spirv.mlir.merge %1 : i32
+    }
+    // CHECK: ^bb4({{.*}}: i32):
+    spirv.ReturnValue %0 : i32


Probably worth adding check for return here as well.

IgWod · 2026-07-22T12:26:18Z

+    %sum = spirv.IAdd %0#0, %0#1 : i32
+    spirv.ReturnValue %sum : i32


Please add check for those two lines.

secona · 2026-07-22T13:46:43Z

This should be good now. I'll need help with the merge when CI passes; I don't have merge access yet.

IgWod · 2026-07-22T14:03:58Z

Could you please make your email address public before I merge it: https://llvm.org/docs/DeveloperPolicy.html#email-addresses?

secona · 2026-07-22T14:12:57Z

I believe its now public

…values (llvm#210600) The current SPIRVToLLVM conversion for SelectionOp does not handle merge blocks with yielding values. This change implements that by adding arguments to the continue block in the SelectionPattern. Closes llvm#204714

[mlir][SPIRV] Add SPIRVToLLVM conversion for selection with yielding …

8aa7d41

…values

secona requested review from IgWod, antiagainst and kuhar as code owners July 19, 2026 14:51

llvmorg-github-actions Bot added mlir:spirv mlir labels Jul 19, 2026

IgWod reviewed Jul 20, 2026

View reviewed changes

secona added 3 commits July 20, 2026 19:52

test: clearer 1 value and 2 values

edb1949

test: return %0 in selection_with_yielding_value

82837f0

test: use both values in selection_with_multiple_yielding_values

2669281

IgWod approved these changes Jul 22, 2026

View reviewed changes

test: add CHECK for remaining lines

92b379c

IgWod merged commit 155689a into llvm:main Jul 22, 2026
12 checks passed

secona deleted the selection-yield-values branch July 22, 2026 14:22

		%sum = spirv.IAdd %0#0, %0#1 : i32
		spirv.ReturnValue %sum : i32

Uh oh!

Conversation

secona commented Jul 19, 2026

Uh oh!

llvmorg-github-actions Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IgWod left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IgWod left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

secona commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IgWod commented Jul 22, 2026

Uh oh!

secona commented Jul 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

llvmorg-github-actions Bot commented Jul 19, 2026 •

edited

Loading

secona commented Jul 22, 2026 •

edited

Loading