[Mlir-commits] [mlir] [mlir][SPIR-V] Deserialize Volatile function parameter decorations (PR #232195)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Alexander Shaposhnikov (alexander-shaposhnikov)

<details>
<summary>Changes</summary>

The serializer already supports Volatile function parameter decorations, but the deserializer rejects them. Accept Volatile using the existing spirv.decoration argument attribute, allowing these parameters to survive binary round trips.

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


3 Files Affected:

- (modified) mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp (+12-9) 
- (modified) mlir/test/Target/SPIRV/function-parameter-attributes.mlir (+30) 
- (modified) mlir/unittests/Dialect/SPIRV/DeserializationTest.cpp (+6-6) 


``````````diff
diff --git a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
index 1cdc5686adfeb2..02c76515a5fed8 100644
--- a/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
+++ b/mlir/lib/Target/SPIRV/Deserialization/Deserializer.cpp
@@ -547,21 +547,24 @@ LogicalResult spirv::Deserializer::setFunctionArgAttrs(
     if (handled)
       continue;
 
-    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.
+    for (auto decoration :
+         {spirv::Decoration::RelaxedPrecision, spirv::Decoration::Volatile}) {
+      if (decAttr.getName() !=
+          getSymbolDecoration(stringifyDecoration(decoration)))
+        continue;
+
+      // TODO: Support multiple decorations per function parameter.
       if (foundDecorationAttr)
         return emitError(unknownLoc, "already found a decoration for function "
                                      "argument with result <id> ")
                << argID;
 
-      foundDecorationAttr = spirv::DecorationAttr::get(
-          context, spirv::Decoration::RelaxedPrecision);
-      continue;
+      foundDecorationAttr = spirv::DecorationAttr::get(context, decoration);
+      handled = true;
+      break;
     }
+    if (handled)
+      continue;
 
     return emitError(unknownLoc, "unhandled decoration '")
            << decAttr.getName().getValue()
diff --git a/mlir/test/Target/SPIRV/function-parameter-attributes.mlir b/mlir/test/Target/SPIRV/function-parameter-attributes.mlir
index 3559aef561601b..31b89fd29aed3f 100644
--- a/mlir/test/Target/SPIRV/function-parameter-attributes.mlir
+++ b/mlir/test/Target/SPIRV/function-parameter-attributes.mlir
@@ -103,3 +103,33 @@ spirv.module Physical64 OpenCL requires #spirv.vce<v1.0, [Addresses, Kernel, Lin
     linkage_attributes = #spirv.linkage_attributes<linkage_name = "external", linkage_type = <Import>>
   }
 }
+
+// -----
+
+// `volatile` pointer parameters are decorated with Volatile; it may be combined
+// with Alignment and FuncParamAttr decorations.
+spirv.module Physical64 OpenCL requires #spirv.vce<v1.0, [Addresses, Kernel, Linkage], []> {
+  // CHECK-LABEL: spirv.func @volatile(
+  // CHECK-SAME: !spirv.ptr<i32, CrossWorkgroup> {spirv.decoration = #spirv.decoration<Volatile>}
+  // CHECK-SAME: !spirv.ptr<i32, CrossWorkgroup> {spirv.alignment = 4 : i32, spirv.decoration = #spirv.decoration<Volatile>, spirv.func_param_attr = [#spirv.function_parameter_attribute<NoCapture>]}
+  spirv.func @volatile(
+      %a: !spirv.ptr<i32, CrossWorkgroup> {spirv.decoration = #spirv.decoration<Volatile>},
+      %b: !spirv.ptr<i32, CrossWorkgroup> {
+        spirv.alignment = 4 : i32,
+        spirv.decoration = #spirv.decoration<Volatile>,
+        spirv.func_param_attr = [#spirv.function_parameter_attribute<NoCapture>]}
+  ) "None" {
+    spirv.Return
+  }
+
+}
+
+// -----
+
+spirv.module Physical64 OpenCL requires #spirv.vce<v1.0, [Addresses, Kernel, Linkage], []> {
+  // CHECK-LABEL: spirv.func @volatile_declaration(
+  // CHECK-SAME: !spirv.ptr<i32, CrossWorkgroup> {spirv.decoration = #spirv.decoration<Volatile>}
+  spirv.func @volatile_declaration(%arg: !spirv.ptr<i32, CrossWorkgroup> {spirv.decoration = #spirv.decoration<Volatile>}) "None" attributes {
+    linkage_attributes = #spirv.linkage_attributes<linkage_name = "volatile_declaration", linkage_type = <Import>>
+  }
+}
diff --git a/mlir/unittests/Dialect/SPIRV/DeserializationTest.cpp b/mlir/unittests/Dialect/SPIRV/DeserializationTest.cpp
index db19eb9e222c1b..3bb757ae93e651 100644
--- a/mlir/unittests/Dialect/SPIRV/DeserializationTest.cpp
+++ b/mlir/unittests/Dialect/SPIRV/DeserializationTest.cpp
@@ -298,12 +298,12 @@ TEST_P(UnsupportedParameterDecorationTest, RejectWithSupportedDecoration) {
                    "result <id> 1");
 }
 
-INSTANTIATE_TEST_SUITE_P(
-    SPIRV, UnsupportedParameterDecorationTest,
-    ::testing::Values(spirv::Decoration::Alignment,
-                      spirv::Decoration::FuncParamAttr,
-                      spirv::Decoration::Restrict,
-                      spirv::Decoration::RelaxedPrecision));
+INSTANTIATE_TEST_SUITE_P(SPIRV, UnsupportedParameterDecorationTest,
+                         ::testing::Values(spirv::Decoration::Alignment,
+                                           spirv::Decoration::FuncParamAttr,
+                                           spirv::Decoration::Restrict,
+                                           spirv::Decoration::RelaxedPrecision,
+                                           spirv::Decoration::Volatile));
 
 TEST_F(DeserializationTest, AlignmentMissingOperand) {
   addHeader();

``````````

</details>


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