[Mlir-commits] [mlir] [mlir][spirv] Use `AttrTypeReplacer` in map-memref-storage-class. NFC. (PR #80055)

[Lei Zhang]

================
@@ -335,23 +287,25 @@ class MapMemRefStorageClassPass final
     MLIRContext *context = &getContext();
     Operation *op = getOperation();
 
+    spirv::MemorySpaceToStorageClassMap spaceToStorage = memorySpaceMap;
     if (spirv::TargetEnvAttr attr = spirv::lookupTargetEnv(op)) {
       spirv::TargetEnv targetEnv(attr);
       if (targetEnv.allows(spirv::Capability::Kernel)) {
-        memorySpaceMap = spirv::mapMemorySpaceToOpenCLStorageClass;
+        spaceToStorage = spirv::mapMemorySpaceToOpenCLStorageClass;
       } else if (targetEnv.allows(spirv::Capability::Shader)) {
-        memorySpaceMap = spirv::mapMemorySpaceToVulkanStorageClass;
+        spaceToStorage = spirv::mapMemorySpaceToVulkanStorageClass;
       }
     }
 
-    std::unique_ptr<ConversionTarget> target =
-        spirv::getMemorySpaceToStorageClassTarget(*context);
-    spirv::MemorySpaceToStorageClassConverter converter(memorySpaceMap);
-
-    RewritePatternSet patterns(context);
-    spirv::populateMemorySpaceToStorageClassPatterns(converter, patterns);
+    spirv::MemorySpaceToStorageClassConverter converter(spaceToStorage);
+    // Perform the replacement.
+    spirv::convertMemRefTypesAndAttrs(op, converter);
 
-    if (failed(applyFullConversion(op, *target, std::move(patterns))))
+    // Only perform the conversion to check that there are no illegal ops
+    // remaining. Do not attempt to convert anything.
+    if (failed(applyFullConversion(
+            op, *spirv::getMemorySpaceToStorageClassTarget(*context),
+            RewritePatternSet{context})))
----------------
antiagainst wrote:

Probably not much in net effect given this is effectively using the degenerated case of the conversion framework. Though it anchors on some internal impl there--we don't know how conversion framework would change in the future and whether the degenerated case would be more expensive. It's coupling a bit there--conversion framwork isn't designed for the particular use here. would be better to use a simpler impl here to avoid that. 

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