[mlir][test] Fix broken test pass on SymbolOpInterface

[sotoshigoto]

Summary

When running the opt-tool with a pass pipeline that does not include
-pass-pipeline="builtin.func(..)", such as -test-print-liveness, a
crash occurs during casting if this pass operates on an operation that
does not have a SymbolOpInterface. For example, a ModuleOp that lacks a
symbol attribute. In a previous change at https://reviews.llvm.org/D121191,
FuncOp was replaced with SymbolOpInterface to reduce the dependency on
FuncOp. This patch adds a check to determine whether an operation can be
cast to SymbolOpInterface before performing the operation.

Fixes #61870

[mehdi_amini]

FYI: we are using GitHub Pull-Requests now (for future revisions)

mlir/test/lib/Analysis/TestLiveness.cpp

23

This is supposed to handle the filtering already, can you provide a test case where it does not?
We should fix it in the framework, it's not the role of the individual passes to do so.

[mehdi_amini]

mlir/test/lib/Analysis/TestLiveness.cpp

23

I looked at the linked GitHub issue which has a reproducer, so got it now.
We still should fix this in the framework itself.

[sotoshigoto]

@mehdi_amini Thank you for your comment. I will try to address it on the framework side.

mlir/test/lib/Analysis/TestLiveness.cpp

 #include "mlir/IR/SymbolTable.h"
 #include "mlir/Pass/Pass.h"
 
 using namespace mlir;
 
 namespace {
 
 struct TestLivenessPass
     : public PassWrapper<TestLivenessPass, InterfacePass<SymbolOpInterface>> {
   MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestLivenessPass)
 
   StringRef getArgument() const final { return "test-print-liveness"; }
   StringRef getDescription() const final {
     return "Print the contents of a constructed liveness information.";
   }
   void runOnOperation() override {
+    if (!isa<SymbolOpInterface>(Pass::getOperation())) {
+      auto passName = getArgument();
+      Pass::getOperation()->emitError()
+          << passName
+          << " pass can operate on an operation with SymbolOpInteface.\n"
+          << "NOTE: The option may need to be fixed to `-pass-pipeline="
+          << "\"builtin.module(func.func(" << passName << "))\"`\n";
+      return signalPassFailure();
+    }
     llvm::errs() << "Testing : " << getOperation().getName() << "\n";
     getAnalysis<Liveness>().print(llvm::errs());
   }
 };
 
 } // namespace
 
 namespace mlir {
 namespace test {
 void registerTestLivenessPass() { PassRegistration<TestLivenessPass>(); }
 } // namespace test
 } // namespace mlir

mlir/test/lib/Analysis/TestMemRefStrideCalculation.cpp

StringRef getDescription() const final {
     return "Test operation constant folding";
   }
   void runOnOperation() override;
 };
 } // namespace
 
 /// Traverse AllocOp and compute strides of each MemRefType independently.
 void TestMemRefStrideCalculation::runOnOperation() {
+  if (!isa<SymbolOpInterface>(Pass::getOperation())) {
+    auto passName = getArgument();
+    Pass::getOperation()->emitError()
+        << passName
+        << " pass can operate on an operation with SymbolOpInteface.\n"
+        << "NOTE: The option may need to be fixed to `-pass-pipeline="
+        << "\"builtin.module(func.func(" << passName << "))\"`\n";
+    return signalPassFailure();
+  }
+
   llvm::outs() << "Testing: " << getOperation().getName() << "\n";
   getOperation().walk([&](memref::AllocOp allocOp) {
     auto memrefType = cast<MemRefType>(allocOp.getResult().getType());
     int64_t offset;
     SmallVector<int64_t, 4> strides;
     if (failed(getStridesAndOffset(memrefType, strides, offset))) {
       llvm::outs() << "MemRefType " << memrefType << " cannot be converted to "
                    << "strided form\n";

mlir/test/lib/Analysis/TestSlice.cpp

: public PassWrapper<TestTopologicalSortPass,
                          InterfacePass<SymbolOpInterface>> {
   MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestTopologicalSortPass)
 
   StringRef getArgument() const final { return "test-print-topological-sort"; }
   StringRef getDescription() const final {
     return "Print operations in topological order";
   }
   void runOnOperation() override {
+    if (!isa<SymbolOpInterface>(Pass::getOperation())) {
+      auto passName = getArgument();
+      Pass::getOperation()->emitError()
+          << passName
+          << " pass can operate on an operation with SymbolOpInteface.\n"
+          << "NOTE: The option may need to be fixed to `-pass-pipeline="
+          << "\"builtin.module(func.func(" << passName << "))\"`\n";
+      return signalPassFailure();
+    }
+
     std::map<int, Operation *> ops;
     getOperation().walk([&ops](Operation *op) {
       if (auto originalOrderAttr = op->getAttrOfType<IntegerAttr>(kOrderMarker))
         ops[originalOrderAttr.getInt()] = op;
     });
     SetVector<Operation *> sortedOp;
     for (auto op : ops)
       sortedOp.insert(op.second);

mlir/test/lib/IR/TestDiagnostics.cpp

struct TestDiagnosticFilterPass
   StringRef getArgument() const final { return "test-diagnostic-filter"; }
   StringRef getDescription() const final {
     return "Test diagnostic filtering support.";
   }
   TestDiagnosticFilterPass() = default;
   TestDiagnosticFilterPass(const TestDiagnosticFilterPass &) {}
 
   void runOnOperation() override {
+    if (!isa<SymbolOpInterface>(Pass::getOperation())) {
+      auto passName = getArgument();
+      Pass::getOperation()->emitError()
+          << passName
+          << " pass can operate on an operation with SymbolOpInteface.\n"
+          << "NOTE: The option may need to be fixed to `-pass-pipeline="
+          << "\"builtin.module(func.func(" << passName << "{filters=..}))\"`\n";
+      return signalPassFailure();
+    }
+
     llvm::errs() << "Test '" << getOperation().getName() << "'\n";
 
     // Build a diagnostic handler that has filtering capabilities.
     auto filterFn = [&](Location loc) {
       // Ignore non-file locations.
       FileLineColLoc fileLoc = dyn_cast<FileLineColLoc>(loc);
       if (!fileLoc)
         return true;

mlir/test/lib/IR/TestDominance.cpp

struct TestDominancePass
   MLIR_DEFINE_EXPLICIT_INTERNAL_INLINE_TYPE_ID(TestDominancePass)
 
   StringRef getArgument() const final { return "test-print-dominance"; }
   StringRef getDescription() const final {
     return "Print the dominance information for multiple regions.";
   }
 
   void runOnOperation() override {
+    if (!isa<SymbolOpInterface>(Pass::getOperation())) {
+      auto passName = getArgument();
+      Pass::getOperation()->emitError()
+          << passName
+          << " pass can operate on an operation with SymbolOpInteface.\n"
+          << "NOTE: The option may need to be fixed to `-pass-pipeline="
+          << "\"builtin.module(func.func(" << passName << "))\"`\n";
+      return signalPassFailure();
+    }
+
     llvm::errs() << "Testing : " << getOperation().getName() << "\n";
     DominanceTest dominanceTest(getOperation());
 
     // Print dominance information.
     llvm::errs() << "--- DominanceInfo ---\n";
     dominanceTest.printDominance(getAnalysis<DominanceInfo>(),
                                  /*printCommonDominatorInfo=*/true);