[Mlir-commits] [mlir] [mlir][spirv] Print and parse discardable attributes on spec constants (PR #227669)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Samarth Narang (snarang181)

<details>
<summary>Changes</summary>

The custom assembly formats of spirv.SpecConstant, spirv.SpecConstantComposite and spirv.EXT.SpecConstantCompositeReplicate never printed or parsed the discardable attribute dictionary, so any such attribute survived the generic form and bytecode but was silently dropped by a textual round trip. 

Fixes #<!-- -->225260

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


2 Files Affected:

- (modified) mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp (+10-3) 
- (modified) mlir/test/Dialect/SPIRV/IR/structure-ops.mlir (+14) 


``````````diff
diff --git a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
index 37ae6a942e491..f967ca61f3349 100644
--- a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
@@ -1902,7 +1902,7 @@ ParseResult spirv::SpecConstantOp::parse(OpAsmParser &parser,
       parser.parseAttribute(valueAttr, defaultValueAttrName, result.attributes))
     return failure();
 
-  return success();
+  return parser.parseOptionalAttrDict(result.attributes);
 }
 
 void spirv::SpecConstantOp::print(OpAsmPrinter &printer) {
@@ -1914,6 +1914,9 @@ void spirv::SpecConstantOp::print(OpAsmPrinter &printer) {
           (*this)->getDiscardableAttrOfType<IntegerAttr>(kSpecIdAttrName))
     printer << ' ' << kSpecIdAttrName << '(' << specID.getInt() << ')';
   printer << " = " << getDefaultValue();
+  printer.printOptionalAttrDict(
+      (*this)->getDiscardableAttrDictionary().getValue(),
+      /*elidedAttrs=*/{kSpecIdAttrName});
 }
 
 LogicalResult spirv::SpecConstantOp::verify() {
@@ -2009,7 +2012,7 @@ ParseResult spirv::SpecConstantCompositeOp::parse(OpAsmParser &parser,
       spirv::SpecConstantCompositeOp::getTypeAttrName(result.name);
   result.addAttribute(typeAttrName, TypeAttr::get(type));
 
-  return success();
+  return parser.parseOptionalAttrDict(result.attributes);
 }
 
 void spirv::SpecConstantCompositeOp::print(OpAsmPrinter &printer) {
@@ -2019,6 +2022,8 @@ void spirv::SpecConstantCompositeOp::print(OpAsmPrinter &printer) {
   printer.printSymbolName(getSymName());
   printer << " (" << llvm::interleaved(this->getConstituents().getValue())
           << ") : " << getType();
+  printer.printOptionalAttrDict(
+      (*this)->getDiscardableAttrDictionary().getValue());
 }
 
 LogicalResult spirv::SpecConstantCompositeOp::verify() {
@@ -2098,7 +2103,7 @@ spirv::EXTSpecConstantCompositeReplicateOp::parse(OpAsmParser &parser,
       spirv::EXTSpecConstantCompositeReplicateOp::getTypeAttrName(result.name);
   result.addAttribute(typeAttrName, TypeAttr::get(type));
 
-  return success();
+  return parser.parseOptionalAttrDict(result.attributes);
 }
 
 void spirv::EXTSpecConstantCompositeReplicateOp::print(OpAsmPrinter &printer) {
@@ -2107,6 +2112,8 @@ void spirv::EXTSpecConstantCompositeReplicateOp::print(OpAsmPrinter &printer) {
     printer << visibility.getValue() << ' ';
   printer.printSymbolName(getSymName());
   printer << " (" << this->getConstituent() << ") : " << getType();
+  printer.printOptionalAttrDict(
+      (*this)->getDiscardableAttrDictionary().getValue());
 }
 
 LogicalResult spirv::EXTSpecConstantCompositeReplicateOp::verify() {
diff --git a/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir b/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
index 4acaa417501ac..799bc9e0c82d2 100644
--- a/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
@@ -1372,3 +1372,17 @@ spirv.module Logical GLSL450 {
     spirv.Return
   }
 }
+
+// -----
+
+// Discardable attributes round-trip through the custom syntax.
+spirv.module Logical GLSL450 {
+  // CHECK: spirv.SpecConstant @sc_attrs spec_id(5) = 1 : i32 {test.marker = "keep"}
+  spirv.SpecConstant @sc_attrs spec_id(5) = 1 : i32 {test.marker = "keep"}
+  // CHECK: spirv.SpecConstant @sc = 1 : i32
+  spirv.SpecConstant @sc = 1 : i32
+  // CHECK: spirv.SpecConstantComposite @scc_attrs (@sc, @sc) : vector<2xi32> {test.marker = "keep"}
+  spirv.SpecConstantComposite @scc_attrs (@sc, @sc) : vector<2xi32> {test.marker = "keep"}
+  // CHECK: spirv.EXT.SpecConstantCompositeReplicate @sccr_attrs (@sc) : vector<2xi32> {test.marker = "keep"}
+  spirv.EXT.SpecConstantCompositeReplicate @sccr_attrs (@sc) : vector<2xi32> {test.marker = "keep"}
+}

``````````

</details>


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