[Mlir-commits] [mlir] [mlir][SPIR-V] Deserialize all unit decorations on function parameters (PR #230611)

[Arseniy Obolenskiy]

https://github.com/aobolensk updated https://github.com/llvm/llvm-project/pull/230611

>From 4fa8fca8e06db12a02c07068a4f6df9a3a48d6ae Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Fri, 9 Oct 2026 20:10:16 +0200
Subject: [PATCH 1/3] [mlir][SPIR-V] Deserialize all unit decorations on
 function parameters

Previously only Aliased, Restrict and RelaxedPrecision were kept, so decorations like Volatile and Coherent were silently dropped
---
 .../SPIRV/Deserialization/Deserializer.cpp    | 42 +++++--------------
 .../Target/SPIRV/function-decorations.mlir    | 12 ++++++
 2 files changed, 23 insertions(+), 31 deletions(-)

diff --git a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
index 9df544d861767e..613cce14f277eb 100644
--- a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
+++ b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
@@ -492,39 +492,19 @@ LogicalResult spirv::Deserializer::setFunctionArgAttrs(
 
   spirv::DecorationAttr foundDecorationAttr;
   for (NamedAttribute decAttr : decorations[argID]) {
-    for (auto decoration :
-         {spirv::Decoration::Aliased, spirv::Decoration::Restrict,
-          spirv::Decoration::AliasedPointer,
-          spirv::Decoration::RestrictPointer}) {
-
-      if (decAttr.getName() !=
-          getSymbolDecoration(stringifyDecoration(decoration)))
-        continue;
-
-      if (foundDecorationAttr)
-        return emitError(unknownLoc,
-                         "more than one Aliased/Restrict decorations for "
-                         "function argument with result <id> ")
-               << argID;
+    if (!isa<UnitAttr>(decAttr.getValue()))
+      continue;
 
-      foundDecorationAttr = spirv::DecorationAttr::get(context, decoration);
-      break;
-    }
+    // TODO: Support multiple decorations per function parameter.
+    if (foundDecorationAttr)
+      return emitError(unknownLoc, "already found a decoration for function "
+                                   "argument with result <id> ")
+             << argID;
 
-    if (decAttr.getName() == getSymbolDecoration(stringifyDecoration(
-                                 spirv::Decoration::RelaxedPrecision))) {
-      // TODO: Current implementation supports only one decoration per function
-      // parameter so RelaxedPrecision cannot be applied at the same time as,
-      // for example, Aliased/Restrict/etc. This should be relaxed to allow any
-      // combination of decoration allowed by the spec to be supported.
-      if (foundDecorationAttr)
-        return emitError(unknownLoc, "already found a decoration for function "
-                                     "argument with result <id> ")
-               << argID;
-
-      foundDecorationAttr = spirv::DecorationAttr::get(
-          context, spirv::Decoration::RelaxedPrecision);
-    }
+    std::optional<spirv::Decoration> decoration = spirv::symbolizeDecoration(
+        llvm::convertToCamelFromSnakeCase(decAttr.getName(), true));
+    assert(decoration.has_value());
+    foundDecorationAttr = spirv::DecorationAttr::get(context, *decoration);
   }
 
   if (!foundDecorationAttr)
diff --git a/mlir/test/Target/SPIRV/function-decorations.mlir b/mlir/test/Target/SPIRV/function-decorations.mlir
index a47b39b0b93397..e7d9670c22137a 100644
--- a/mlir/test/Target/SPIRV/function-decorations.mlir
+++ b/mlir/test/Target/SPIRV/function-decorations.mlir
@@ -94,3 +94,15 @@ spirv.module PhysicalStorageBuffer64 GLSL450 requires #spirv.vce<v1.0,
     spirv.Return
   }
 }
+
+// -----
+
+spirv.module Physical64 OpenCL requires #spirv.vce<v1.0, [Kernel, Addresses, Linkage, Int64], []> {
+  // CHECK-LABEL: spirv.func @func_arg_decoration_unit(%{{.*}}: !spirv.ptr<i32, CrossWorkgroup> {spirv.decoration = #spirv.decoration<Volatile>}, %{{.*}}: !spirv.ptr<i32, CrossWorkgroup> {spirv.decoration = #spirv.decoration<Coherent>})
+  spirv.func @func_arg_decoration_unit(
+      %arg0: !spirv.ptr<i32, CrossWorkgroup> { spirv.decoration = #spirv.decoration<Volatile> },
+      %arg1: !spirv.ptr<i32, CrossWorkgroup> { spirv.decoration = #spirv.decoration<Coherent> }
+  ) "None" {
+    spirv.Return
+  }
+}

>From 959ba9a9d1801253a88b30b937f9a8e044d88eab Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Wed, 14 Oct 2026 14:23:58 +0200
Subject: [PATCH 2/3] fix

---
 .../Dialect/SPIRV/DeserializationTest.cpp       | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/mlir/unittests/Dialect/SPIRV/DeserializationTest.cpp b/mlir/unittests/Dialect/SPIRV/DeserializationTest.cpp
index db19eb9e222c1b..c420162f951c9a 100644
--- a/mlir/unittests/Dialect/SPIRV/DeserializationTest.cpp
+++ b/mlir/unittests/Dialect/SPIRV/DeserializationTest.cpp
@@ -269,13 +269,8 @@ class UnsupportedParameterDecorationTest
 TEST_P(UnsupportedParameterDecorationTest, RejectWithSupportedDecoration) {
   addHeader();
   auto argID = nextID++;
-  SmallVector<uint32_t> operands = {argID, static_cast<uint32_t>(GetParam())};
-  if (GetParam() == spirv::Decoration::Alignment)
-    operands.push_back(16);
-  else if (GetParam() == spirv::Decoration::FuncParamAttr)
-    operands.push_back(
-        static_cast<uint32_t>(spirv::FunctionParameterAttribute::NoWrite));
-  addInstruction(spirv::Opcode::OpDecorate, operands);
+  addInstruction(spirv::Opcode::OpDecorate,
+                 {argID, static_cast<uint32_t>(GetParam())});
   addInstruction(spirv::Opcode::OpDecorate,
                  {argID, static_cast<uint32_t>(spirv::Decoration::Constant)});
 
@@ -294,15 +289,13 @@ TEST_P(UnsupportedParameterDecorationTest, RejectWithSupportedDecoration) {
   addFunctionEnd();
 
   ASSERT_FALSE(deserialize());
-  expectDiagnostic("unhandled decoration 'constant' on function argument with "
-                   "result <id> 1");
+  expectDiagnostic(
+      "already found a decoration for function argument with result <id> 1");
 }
 
 INSTANTIATE_TEST_SUITE_P(
     SPIRV, UnsupportedParameterDecorationTest,
-    ::testing::Values(spirv::Decoration::Alignment,
-                      spirv::Decoration::FuncParamAttr,
-                      spirv::Decoration::Restrict,
+    ::testing::Values(spirv::Decoration::Restrict,
                       spirv::Decoration::RelaxedPrecision));
 
 TEST_F(DeserializationTest, AlignmentMissingOperand) {

>From 75c3048caeffdfb7f28d31c176010e7bcf45e5d9 Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Thu, 15 Oct 2026 08:09:18 +0200
Subject: [PATCH 3/3] add error

---
 mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
index 589b077d564887..83d5be6765a010 100644
--- a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
+++ b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
@@ -535,7 +535,10 @@ LogicalResult spirv::Deserializer::setFunctionArgAttrs(
 
     std::optional<spirv::Decoration> decoration = spirv::symbolizeDecoration(
         llvm::convertToCamelFromSnakeCase(decAttr.getName(), true));
-    assert(decoration.has_value());
+    if (!decoration)
+      return emitError(unknownLoc, "unknown decoration '")
+             << decAttr.getName().getValue()
+             << "' on function argument with result <id> " << argID;
     foundDecorationAttr = spirv::DecorationAttr::get(context, *decoration);
   }