Skip to content

Conversation

@timnoack
Copy link
Contributor

This patch adds verification to the SymbolOpInterface to enforce the design constraint that symbol operations must not produce SSA results, as documented in Symbols and SymbolTables.

This is a follow-up of #168376

@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir mlir:ods labels Nov 17, 2025
@llvmbot
Copy link
Member

llvmbot commented Nov 17, 2025

@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-mlir-core

@llvm/pr-subscribers-mlir-ods

Author: Tim Noack (timnoack)

Changes

This patch adds verification to the SymbolOpInterface to enforce the design constraint that symbol operations must not produce SSA results, as documented in Symbols and SymbolTables.

This is a follow-up of #168376


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

3 Files Affected:

  • (modified) mlir/include/mlir/IR/SymbolInterfaces.td (+2)
  • (modified) mlir/test/IR/invalid-ops.mlir (+8)
  • (modified) mlir/test/lib/Dialect/Test/TestOps.td (+7)
diff --git a/mlir/include/mlir/IR/SymbolInterfaces.td b/mlir/include/mlir/IR/SymbolInterfaces.td
index bbfa30815bd4a..b3aafe063d376 100644
--- a/mlir/include/mlir/IR/SymbolInterfaces.td
+++ b/mlir/include/mlir/IR/SymbolInterfaces.td
@@ -171,6 +171,8 @@ def Symbol : OpInterface<"SymbolOpInterface"> {
     if (concreteOp.isDeclaration() && concreteOp.isPublic())
       return concreteOp.emitOpError("symbol declaration cannot have public "
              "visibility");
+    if ($_op->getNumResults() != 0)
+      return concreteOp.emitOpError("symbols must not have results");
     auto parent = $_op->getParentOp();
     if (parent && !parent->hasTrait<OpTrait::SymbolTable>() && parent->isRegistered()) {
       return concreteOp.emitOpError("symbol's parent must have the SymbolTable "
diff --git a/mlir/test/IR/invalid-ops.mlir b/mlir/test/IR/invalid-ops.mlir
index 0c5fec8c4055a..2f5dd28b51911 100644
--- a/mlir/test/IR/invalid-ops.mlir
+++ b/mlir/test/IR/invalid-ops.mlir
@@ -145,3 +145,11 @@ func.func @verify_fail_3() {
   %r = "arith.constant"() {value = -3 : si32} : () -> si32
   return
 }
+
+// -----
+
+// Verify that symbols with results are rejected
+module {
+  // expected-error@+1 {{'test.symbol_with_result' op symbols must not have results}}
+  %0 = "test.symbol_with_result"() <{sym_name = "test_symbol"}> : () -> i32
+}
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index 275025978a784..670223984fd95 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -120,6 +120,13 @@ def SymbolOp : TEST_Op<"symbol", [NoMemoryEffect, Symbol]> {
                        OptionalAttr<StrAttr>:$sym_visibility);
 }
 
+def SymbolWithResultOp : TEST_Op<"symbol_with_result", [Symbol]> {
+  let summary = "invalid symbol operation that produces an SSA result";
+  let arguments = (ins StrAttr:$sym_name,
+                       OptionalAttr<StrAttr>:$sym_visibility);
+  let results = (outs AnyType:$result);
+}
+
 def OverriddenSymbolVisibilityOp : TEST_Op<"overridden_symbol_visibility", [
   DeclareOpInterfaceMethods<Symbol, ["getVisibility", "setVisibility"]>,
 ]> {

@joker-eph joker-eph enabled auto-merge (squash) November 17, 2025 15:48
@joker-eph joker-eph merged commit ff7896e into llvm:main Nov 17, 2025
14 checks passed
@timnoack timnoack deleted the symbol-no-results-verification branch November 17, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mlir:core MLIR Core Infrastructure mlir:ods mlir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants