[Mlir-commits] [mlir] [mlir] targeted verification for transform "inlining" (PR #192956)

[Oleksandr Alex Zinenko]

================
@@ -227,21 +263,57 @@ transform::detail::mergeSymbolsInto(Operation *target,
       assert(targetSymbolTable.lookup(funcOp.getName()) == collidingFuncOp);
 
       // Do the actual merging.
-      {
-        InFlightDiagnostic diag = mergeInto(funcOp, collidingFuncOp);
-        if (failed(diag))
-          return diag;
-      }
+      if (failed(mergeInto(funcOp, collidingFuncOp)))
+        return failure();
     }
   }
 
-  // Need full verification here because merging/inlining may have broken some
-  // nesting invariants that were not broken in the sources.
-  // TODO: implement and use InlinerDialectInterface to avoid this check.
-  if (failed(mlir::verify(target)))
-    return target->emitError()
-           << "failed to verify target op after merging symbols";
+  // Symbol merging only moves callable ops between symbol tables; it does not
+  // alter the bodies that were already valid in the source modules. The only
+  // invariants that may newly be violated after merging are:
+  //   1. a call now refers to a callee whose body is structurally not legal to
+  //      inline at the call site (caught by the transform dialect's
+  //      `DialectInlinerInterface` implementation), or
+  //   2. the merged call graph contains a recursive cycle, which is forbidden
+  //      for `transform.named_sequence` callables (caught by the shared
+  //      `verifyNoRecursionInCallGraph` helper).
+  // Use the inliner interface methods directly (without running the inlining
+  // pass) to validate (1), and reuse the dialect's call-graph verifier for
+  // (2). The call graph builder requires call/callable ops to be well-formed,
+  // so pre-verify them here without recursing into their bodies.
+  WalkResult preVerify = target->walk([](Operation *nested) {
+    if (!isa<CallableOpInterface, CallOpInterface>(nested))
+      return WalkResult::advance();
+    if (failed(mlir::verify(nested, /*verifyRecursively=*/false)))
+      return WalkResult::interrupt();
+    return WalkResult::advance();
+  });
+  if (preVerify.wasInterrupted())
+    return failure();
+
+  InlinerInterface inliner(target->getContext());
+  WalkResult inlineCheck = target->walk([&](CallOpInterface call) {
+    Operation *callable = nullptr;
+    CallInterfaceCallable callee = call.getCallableForCallee();
+    if (auto symRef = dyn_cast<SymbolRefAttr>(callee))
+      callable = targetSymbolTable.lookup(symRef.getLeafReference());
+    else if (auto value = dyn_cast<Value>(callee))
+      callable = value.getDefiningOp();
+
+    if (!callable)
+      return WalkResult::advance();
+    if (!inliner.isLegalToInline(call, callable, /*wouldBeCloned=*/true)) {
----------------
ftynse wrote:

Fair enough, I misremembered where the cloning happens. It was at the entire module and this code moves, I'll set to `false` instead.

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