[mlir][spirv] Add definition for OpKill #126554

IgWod-IMG · 2025-02-10T17:40:34Z

Although the operation is deprecated in the most recent version of the SPIR-V spec, it is still used by older shaders, so having it defined is valuable and incurs negligible maintenance overhead, due to op simplicity.

llvmbot · 2025-02-10T17:41:16Z

@llvm/pr-subscribers-mlir-spirv

Author: Igor Wodiany (IgWod-IMG)

Changes

Although the operation is deprecated in the most recent version of the SPIR-V spec, it is still used by older shaders, so having it defined is valuable and incurs negligible maintenance overhead, due to op simplicity.

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

4 Files Affected:

(modified) mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td (+2-1)
(modified) mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td (+44)
(modified) mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir (+12)
(modified) mlir/test/Target/SPIRV/terminator.mlir (+6)

diff --git a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td
index 6b2e4189aea028e..16bccd2b72d18f5 100644
--- a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td
+++ b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td
@@ -4472,6 +4472,7 @@ def SPIRV_OC_OpSelectionMerge                : I32EnumAttrCase<"OpSelectionMerge
 def SPIRV_OC_OpLabel                         : I32EnumAttrCase<"OpLabel", 248>;
 def SPIRV_OC_OpBranch                        : I32EnumAttrCase<"OpBranch", 249>;
 def SPIRV_OC_OpBranchConditional             : I32EnumAttrCase<"OpBranchConditional", 250>;
+def SPIRV_OC_OpKill                          : I32EnumAttrCase<"OpKill", 252>;
 def SPIRV_OC_OpReturn                        : I32EnumAttrCase<"OpReturn", 253>;
 def SPIRV_OC_OpReturnValue                   : I32EnumAttrCase<"OpReturnValue", 254>;
 def SPIRV_OC_OpUnreachable                   : I32EnumAttrCase<"OpUnreachable", 255>;
@@ -4599,7 +4600,7 @@ def SPIRV_OpcodeAttr :
       SPIRV_OC_OpAtomicAnd, SPIRV_OC_OpAtomicOr, SPIRV_OC_OpAtomicXor,
       SPIRV_OC_OpPhi, SPIRV_OC_OpLoopMerge, SPIRV_OC_OpSelectionMerge,
       SPIRV_OC_OpLabel, SPIRV_OC_OpBranch, SPIRV_OC_OpBranchConditional,
-      SPIRV_OC_OpReturn, SPIRV_OC_OpReturnValue, SPIRV_OC_OpUnreachable,
+      SPIRV_OC_OpKill, SPIRV_OC_OpReturn, SPIRV_OC_OpReturnValue, SPIRV_OC_OpUnreachable,
       SPIRV_OC_OpGroupBroadcast, SPIRV_OC_OpGroupIAdd, SPIRV_OC_OpGroupFAdd,
       SPIRV_OC_OpGroupFMin, SPIRV_OC_OpGroupUMin, SPIRV_OC_OpGroupSMin,
       SPIRV_OC_OpGroupFMax, SPIRV_OC_OpGroupUMax, SPIRV_OC_OpGroupSMax,
diff --git a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
index cc2f0e4962d8a8a..dad693f2d28cbfd 100644
--- a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
+++ b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
@@ -242,6 +242,50 @@ def SPIRV_FunctionCallOp : SPIRV_Op<"FunctionCall", [
 
 // -----
 
+def SPIRV_KillOp : SPIRV_Op<"Kill", [Terminator]>,
+                   CppDeprecated<"use 'spirv.TerminateInvocation' or 'spirv.DemoteToHelperInvocation' instead">
+{
+  let summary = [{
+    Deprecated (use OpTerminateInvocation or OpDemoteToHelperInvocation).
+  }];
+
+  let description = [{
+    Fragment-shader discard.
+
+    Ceases all further processing in any invocation that executes it: Only
+    instructions these invocations executed before OpKill have observable
+    side effects. If this instruction is executed in non-uniform control
+    flow, all subsequent control flow is non-uniform (for invocations that
+    continue to execute).
+
+    This instruction must be the last instruction in a block.
+
+    This instruction is only valid in the Fragment Execution Model.
+
+    <!-- End of AutoGen section -->
+
+    #### Example:
+
+    ```mlir
+    spirv.Kill
+    ```
+  }];
+
+  let availability = [
+    MinVersion<SPIRV_V_1_0>,
+    MaxVersion<SPIRV_V_1_6>,
+    Extension<[]>,
+    Capability<[SPIRV_C_Shader]>
+  ];
+
+  let arguments = (ins);
+  let results = (outs);
+  let assemblyFormat = "attr-dict";
+  let hasVerifier = 0;
+}
+
+// -----
+
 def SPIRV_LoopOp : SPIRV_Op<"mlir.loop", [InFunctionScope]> {
   let summary = "Define a structured loop.";
 
diff --git a/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir b/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir
index 8496448759f0c99..1d1e2840a579a62 100644
--- a/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir
@@ -789,3 +789,15 @@ func.func @unreachable() {
   // expected-error @+1 {{cannot be used in reachable block}}
   spirv.Unreachable
 }
+
+// -----
+
+//===----------------------------------------------------------------------===//
+// spirv.Kill
+//===----------------------------------------------------------------------===//
+
+// CHECK-LABEL: func @kill
+func.func @kill() {
+  // CHECK: spirv.Kill
+  spirv.Kill
+}
diff --git a/mlir/test/Target/SPIRV/terminator.mlir b/mlir/test/Target/SPIRV/terminator.mlir
index 065b68b9bdfbb80..8338a575681f129 100644
--- a/mlir/test/Target/SPIRV/terminator.mlir
+++ b/mlir/test/Target/SPIRV/terminator.mlir
@@ -24,4 +24,10 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
     // CHECK-NOT: spirv.Unreachable
     spirv.Unreachable
   }
+
+  // CHECK-LABEL: @kill
+  spirv.func @kill() -> () "None" {
+    // CHECK: spirv.Kill
+    spirv.Kill
+  }
 }

llvmbot · 2025-02-10T17:41:17Z

@llvm/pr-subscribers-mlir

Author: Igor Wodiany (IgWod-IMG)

Changes

Although the operation is deprecated in the most recent version of the SPIR-V spec, it is still used by older shaders, so having it defined is valuable and incurs negligible maintenance overhead, due to op simplicity.

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

4 Files Affected:

(modified) mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td (+2-1)
(modified) mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td (+44)
(modified) mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir (+12)
(modified) mlir/test/Target/SPIRV/terminator.mlir (+6)

diff --git a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td
index 6b2e4189aea028e..16bccd2b72d18f5 100644
--- a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td
+++ b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td
@@ -4472,6 +4472,7 @@ def SPIRV_OC_OpSelectionMerge                : I32EnumAttrCase<"OpSelectionMerge
 def SPIRV_OC_OpLabel                         : I32EnumAttrCase<"OpLabel", 248>;
 def SPIRV_OC_OpBranch                        : I32EnumAttrCase<"OpBranch", 249>;
 def SPIRV_OC_OpBranchConditional             : I32EnumAttrCase<"OpBranchConditional", 250>;
+def SPIRV_OC_OpKill                          : I32EnumAttrCase<"OpKill", 252>;
 def SPIRV_OC_OpReturn                        : I32EnumAttrCase<"OpReturn", 253>;
 def SPIRV_OC_OpReturnValue                   : I32EnumAttrCase<"OpReturnValue", 254>;
 def SPIRV_OC_OpUnreachable                   : I32EnumAttrCase<"OpUnreachable", 255>;
@@ -4599,7 +4600,7 @@ def SPIRV_OpcodeAttr :
       SPIRV_OC_OpAtomicAnd, SPIRV_OC_OpAtomicOr, SPIRV_OC_OpAtomicXor,
       SPIRV_OC_OpPhi, SPIRV_OC_OpLoopMerge, SPIRV_OC_OpSelectionMerge,
       SPIRV_OC_OpLabel, SPIRV_OC_OpBranch, SPIRV_OC_OpBranchConditional,
-      SPIRV_OC_OpReturn, SPIRV_OC_OpReturnValue, SPIRV_OC_OpUnreachable,
+      SPIRV_OC_OpKill, SPIRV_OC_OpReturn, SPIRV_OC_OpReturnValue, SPIRV_OC_OpUnreachable,
       SPIRV_OC_OpGroupBroadcast, SPIRV_OC_OpGroupIAdd, SPIRV_OC_OpGroupFAdd,
       SPIRV_OC_OpGroupFMin, SPIRV_OC_OpGroupUMin, SPIRV_OC_OpGroupSMin,
       SPIRV_OC_OpGroupFMax, SPIRV_OC_OpGroupUMax, SPIRV_OC_OpGroupSMax,
diff --git a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
index cc2f0e4962d8a8a..dad693f2d28cbfd 100644
--- a/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
+++ b/mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td
@@ -242,6 +242,50 @@ def SPIRV_FunctionCallOp : SPIRV_Op<"FunctionCall", [
 
 // -----
 
+def SPIRV_KillOp : SPIRV_Op<"Kill", [Terminator]>,
+                   CppDeprecated<"use 'spirv.TerminateInvocation' or 'spirv.DemoteToHelperInvocation' instead">
+{
+  let summary = [{
+    Deprecated (use OpTerminateInvocation or OpDemoteToHelperInvocation).
+  }];
+
+  let description = [{
+    Fragment-shader discard.
+
+    Ceases all further processing in any invocation that executes it: Only
+    instructions these invocations executed before OpKill have observable
+    side effects. If this instruction is executed in non-uniform control
+    flow, all subsequent control flow is non-uniform (for invocations that
+    continue to execute).
+
+    This instruction must be the last instruction in a block.
+
+    This instruction is only valid in the Fragment Execution Model.
+
+    <!-- End of AutoGen section -->
+
+    #### Example:
+
+    ```mlir
+    spirv.Kill
+    ```
+  }];
+
+  let availability = [
+    MinVersion<SPIRV_V_1_0>,
+    MaxVersion<SPIRV_V_1_6>,
+    Extension<[]>,
+    Capability<[SPIRV_C_Shader]>
+  ];
+
+  let arguments = (ins);
+  let results = (outs);
+  let assemblyFormat = "attr-dict";
+  let hasVerifier = 0;
+}
+
+// -----
+
 def SPIRV_LoopOp : SPIRV_Op<"mlir.loop", [InFunctionScope]> {
   let summary = "Define a structured loop.";
 
diff --git a/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir b/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir
index 8496448759f0c99..1d1e2840a579a62 100644
--- a/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/control-flow-ops.mlir
@@ -789,3 +789,15 @@ func.func @unreachable() {
   // expected-error @+1 {{cannot be used in reachable block}}
   spirv.Unreachable
 }
+
+// -----
+
+//===----------------------------------------------------------------------===//
+// spirv.Kill
+//===----------------------------------------------------------------------===//
+
+// CHECK-LABEL: func @kill
+func.func @kill() {
+  // CHECK: spirv.Kill
+  spirv.Kill
+}
diff --git a/mlir/test/Target/SPIRV/terminator.mlir b/mlir/test/Target/SPIRV/terminator.mlir
index 065b68b9bdfbb80..8338a575681f129 100644
--- a/mlir/test/Target/SPIRV/terminator.mlir
+++ b/mlir/test/Target/SPIRV/terminator.mlir
@@ -24,4 +24,10 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
     // CHECK-NOT: spirv.Unreachable
     spirv.Unreachable
   }
+
+  // CHECK-LABEL: @kill
+  spirv.func @kill() -> () "None" {
+    // CHECK: spirv.Kill
+    spirv.Kill
+  }
 }

kuhar · 2025-02-11T09:48:18Z

mlir/include/mlir/Dialect/SPIRV/IR/SPIRVControlFlowOps.td

@@ -242,6 +242,50 @@ def SPIRV_FunctionCallOp : SPIRV_Op<"FunctionCall", [

 // -----

+def SPIRV_KillOp : SPIRV_Op<"Kill", [Terminator]>,
+                   CppDeprecated<"use 'spirv.TerminateInvocation' or 'spirv.DemoteToHelperInvocation' instead">


Does this result in deprecation warnings in any C++ code that references this Op? If yes, I don't think this is desired despite the op being deprecated at the SPIR-V level, since our support is not deprecated per se.

So, I haven't seen any warnings when building (unless I missed it) or running mlir-opt and mlir-translate however I looked again at the documentation for CppDeprecated and I agree it is not correctly used in here. I'll remove it and push an update.

IgWod-IMG · 2025-02-11T10:46:39Z

I have just pushed an updated patch. Please merge if happy (still waiting for committer rights).

kuhar

LGTM. I'm going to wait for @andfau-amd to review before merging.

andfau-amd · 2025-02-17T10:48:07Z

OpKill is quite unusual in that it's a block terminator, like OpReturn, so I decided to look extra closely at that aspect. I see that it has the Terminator thing in the tablegen definition, which is good. However, while I don't know if it has to be addressed in this PR, there's this existing TODO related to it that now becomes relevant:

llvm-project/mlir/lib/Dialect/SPIRV/IR/SPIRVDialect.cpp

Lines 84 to 87 in f09fd94

    
           // TODO: we need to filter OpKill here to avoid inlining it to 
        
           // a loop continue construct: 
        
           // https://github.com/KhronosGroup/SPIRV-Headers/issues/86 
        
           // However OpKill is fragment shader specific and we don't support it yet.

IgWod-IMG · 2025-02-17T14:25:06Z

So, I wasn't aware of the TODO and the issues. Looking at it, it seems that handling it shouldn't be too bad, due to the way SPIR-V MLIR is structured, i.e., we can identify entry, header, continue, merge within spirv.mlir.loop. Unless I missed something crucial here.

Nevertheless, there will probably be some details to work out, so I propose in this PR to have a blanket rule disallowing inlining OpKill in any context and then I'll work on a separate PR handling the issue properly. Although this will limit the inliner when OpKill is involved, it will also stop incorrect code being generated. I guess all that needs to be done is to add in isLegalToInline:

if (isa<spirv::KillOp>(op))
  return false;

Does it sound reasonable?

andfau-amd · 2025-02-17T14:45:30Z

Making the simplest possible correct change seems fine to me.

Although the operation is deprecated in the most recent version of the SPIR-V spec, it is still used by older shaders, so having it defined is valuable and incurs negligible maintenance overhead, due to op simplicity.

IgWod-IMG · 2025-02-17T16:16:21Z

I have disallowed inlining OpKill and added a test for that. Please merge if you're happy with it.

andfau-amd · 2025-02-17T16:45:22Z

Is "Please merge if you're happy with it." meaning you don't have commit access? I'll click the button once the tests pass in that case.

IgWod-IMG · 2025-02-17T16:53:38Z

Is "Please merge if you're happy with it." meaning you don't have commit access?

Correct! I don’t have commit access. I have submitted the request, but it hasn't yet been approved (#125728).

I'll click the button once the tests pass in that case.

Thank you!

Although the operation is deprecated in the most recent version of the SPIR-V spec, it is still used by older shaders, so having it defined is valuable and incurs negligible maintenance overhead, due to op simplicity.

IgWod-IMG requested review from antiagainst and kuhar as code owners February 10, 2025 17:40

llvmbot added mlir:spirv mlir labels Feb 10, 2025

kuhar reviewed Feb 11, 2025

View reviewed changes

IgWod-IMG force-pushed the img_kill branch from 9109766 to 7d5a942 Compare February 11, 2025 10:41

kuhar approved these changes Feb 11, 2025

View reviewed changes

[mlir][spirv] Add definition for OpKill

960ca47

Although the operation is deprecated in the most recent version of the SPIR-V spec, it is still used by older shaders, so having it defined is valuable and incurs negligible maintenance overhead, due to op simplicity.

IgWod-IMG force-pushed the img_kill branch from 7d5a942 to 960ca47 Compare February 17, 2025 16:13

andfau-amd approved these changes Feb 17, 2025

View reviewed changes

andfau-amd merged commit 93d3e20 into llvm:main Feb 18, 2025
8 checks passed

IgWod-IMG deleted the img_kill branch February 18, 2025 13:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[mlir][spirv] Add definition for OpKill #126554

[mlir][spirv] Add definition for OpKill #126554

Uh oh!

IgWod-IMG commented Feb 10, 2025

Uh oh!

llvmbot commented Feb 10, 2025

Uh oh!

llvmbot commented Feb 10, 2025

Uh oh!

kuhar Feb 11, 2025

Uh oh!

IgWod-IMG Feb 11, 2025

Uh oh!

IgWod-IMG commented Feb 11, 2025

Uh oh!

kuhar left a comment

Uh oh!

andfau-amd commented Feb 17, 2025

Uh oh!

IgWod-IMG commented Feb 17, 2025

Uh oh!

andfau-amd commented Feb 17, 2025

Uh oh!

IgWod-IMG commented Feb 17, 2025

Uh oh!

andfau-amd commented Feb 17, 2025

Uh oh!

IgWod-IMG commented Feb 17, 2025

Uh oh!

Uh oh!

Uh oh!

[mlir][spirv] Add definition for OpKill #126554

[mlir][spirv] Add definition for OpKill #126554

Uh oh!

Conversation

IgWod-IMG commented Feb 10, 2025

Uh oh!

llvmbot commented Feb 10, 2025

Uh oh!

llvmbot commented Feb 10, 2025

Uh oh!

kuhar Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

IgWod-IMG Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

IgWod-IMG commented Feb 11, 2025

Uh oh!

kuhar left a comment

Choose a reason for hiding this comment

Uh oh!

andfau-amd commented Feb 17, 2025

Uh oh!

IgWod-IMG commented Feb 17, 2025

Uh oh!

andfau-amd commented Feb 17, 2025

Uh oh!

IgWod-IMG commented Feb 17, 2025

Uh oh!

andfau-amd commented Feb 17, 2025

Uh oh!

IgWod-IMG commented Feb 17, 2025

Uh oh!

Uh oh!

Uh oh!