[mlir] share argument attributes interface between calls and callables #123176

jeanPerier · 2025-01-16T10:03:09Z

First patch of this RFC to add argument and result attributes on llvm.call (and llvm.invoke).

This patch shares core interface methods dealing with argument and result attributes from CallableOpInterface with the CallOpInterface and makes them mandatory to gives more consistent guarantees about concrete operations using these interfaces.

Add optional arg_attrs and res_attrs attributes to operations using these interfaces that did not have that already.
They can then re-use the common "rich function signature" printing/parsing helpers if they want (for the LLVM dialect, this is done in the next patch).

[edit: change the inheritance level after RFC feedback]
[edit 2: remove extra interface and use listconcat instead to share methods. Make them mandatory].

llvmbot · 2025-01-16T10:03:45Z

@llvm/pr-subscribers-flang-codegen
@llvm/pr-subscribers-flang-fir-hlfir
@llvm/pr-subscribers-mlir-core
@llvm/pr-subscribers-mlir-func
@llvm/pr-subscribers-mlir-shape
@llvm/pr-subscribers-mlir-llvm
@llvm/pr-subscribers-mlir-gpu
@llvm/pr-subscribers-mlir-emitc

@llvm/pr-subscribers-mlir

Author: None (jeanPerier)

Changes

First patch of this RFC to add argument and result attributes on llvm.call (and llvm.invoke).

This patch extracts the core interface methods dealing with argument and result attributes from CallOpInterface in a new interface inherited by both CallOpInterface and CallableInterface.

This allows dialects to "opt-in" having parameter attributes on call arguments by adding arg_attrs and res_attrs OptionalAttr<DictArrayAttr> to the operation definition. They can then re-use the common "rich function signature" printing/parsing helpers if they want.

Patch is 34.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/123176.diff

14 Files Affected:

(modified) mlir/docs/Interfaces.md (+9-7)
(added) mlir/include/mlir/Interfaces/CallImplementation.h (+91)
(modified) mlir/include/mlir/Interfaces/CallInterfaces.td (+56-43)
(modified) mlir/include/mlir/Interfaces/FunctionImplementation.h (+8-16)
(modified) mlir/lib/Dialect/Async/IR/Async.cpp (+1-1)
(modified) mlir/lib/Dialect/EmitC/IR/EmitC.cpp (+1-1)
(modified) mlir/lib/Dialect/Func/IR/FuncOps.cpp (+1-1)
(modified) mlir/lib/Dialect/GPU/IR/GPUDialect.cpp (+1-1)
(modified) mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (+2-2)
(modified) mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp (+1-1)
(modified) mlir/lib/Dialect/Shape/IR/Shape.cpp (+1-1)
(modified) mlir/lib/Interfaces/CMakeLists.txt (+5-16)
(added) mlir/lib/Interfaces/CallImplementation.cpp (+178)
(modified) mlir/lib/Interfaces/FunctionImplementation.cpp (+4-144)

diff --git a/mlir/docs/Interfaces.md b/mlir/docs/Interfaces.md
index 51747db546bb76..195f58dfbc23b3 100644
--- a/mlir/docs/Interfaces.md
+++ b/mlir/docs/Interfaces.md
@@ -753,7 +753,15 @@ interface section goes as follows:
     -   (`C++ class` -- `ODS class`(if applicable))
 
 ##### CallInterfaces
-
+*   `OpWithArgumentAttributesInterface`  - Used to represent operations that may
+    carry argument and result attributes. It is inherited by both
+    CallOpInterface and CallableOpInterface.
+    -   `ArrayAttr getArgAttrsAttr()`
+    -   `ArrayAttr getResAttrsAttr()`
+    -   `void setArgAttrsAttr(ArrayAttr)`
+    -   `void setResAttrsAttr(ArrayAttr)`
+    -   `Attribute removeArgAttrsAttr()`
+    -   `Attribute removeResAttrsAttr()`
 *   `CallOpInterface` - Used to represent operations like 'call'
     -   `CallInterfaceCallable getCallableForCallee()`
     -   `void setCalleeFromCallable(CallInterfaceCallable)`
@@ -761,12 +769,6 @@ interface section goes as follows:
     -   `Region * getCallableRegion()`
     -   `ArrayRef<Type> getArgumentTypes()`
     -   `ArrayRef<Type> getResultsTypes()`
-    -   `ArrayAttr getArgAttrsAttr()`
-    -   `ArrayAttr getResAttrsAttr()`
-    -   `void setArgAttrsAttr(ArrayAttr)`
-    -   `void setResAttrsAttr(ArrayAttr)`
-    -   `Attribute removeArgAttrsAttr()`
-    -   `Attribute removeResAttrsAttr()`
 
 ##### RegionKindInterfaces
 
diff --git a/mlir/include/mlir/Interfaces/CallImplementation.h b/mlir/include/mlir/Interfaces/CallImplementation.h
new file mode 100644
index 00000000000000..85e47f6b3dbbb9
--- /dev/null
+++ b/mlir/include/mlir/Interfaces/CallImplementation.h
@@ -0,0 +1,91 @@
+//===- CallImplementation.h - Call and Callable Op utilities ----*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file provides utility functions for implementing call-like and
+// callable-like operations, in particular, parsing, printing and verification
+// components common to these operations.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_INTERFACES_CALLIMPLEMENTATION_H
+#define MLIR_INTERFACES_CALLIMPLEMENTATION_H
+
+#include "mlir/IR/OpImplementation.h"
+#include "mlir/Interfaces/CallInterfaces.h"
+
+namespace mlir {
+
+class OpWithArgumentAttributesInterface;
+
+namespace call_interface_impl {
+
+/// Parse a function or call result list.
+///
+///   function-result-list ::= function-result-list-parens
+///                          | non-function-type
+///   function-result-list-parens ::= `(` `)`
+///                                 | `(` function-result-list-no-parens `)`
+///   function-result-list-no-parens ::= function-result (`,` function-result)*
+///   function-result ::= type attribute-dict?
+///
+ParseResult
+parseFunctionResultList(OpAsmParser &parser, SmallVectorImpl<Type> &resultTypes,
+                        SmallVectorImpl<DictionaryAttr> &resultAttrs);
+
+/// Parses a function signature using `parser`. This does not deal with function
+/// signatures containing SSA region arguments (to parse these signatures, use
+/// function_interface_impl::parseFunctionSignature). When
+/// `mustParseEmptyResult`, `-> ()` is expected when there is no result type.
+///
+///   no-ssa-function-signature ::= `(` no-ssa-function-arg-list `)`
+///                               -> function-result-list
+///   no-ssa-function-arg-list  ::= no-ssa-function-arg
+///                               (`,` no-ssa-function-arg)*
+///   no-ssa-function-arg       ::= type attribute-dict?
+ParseResult parseFunctionSignature(OpAsmParser &parser,
+                                   SmallVectorImpl<Type> &argTypes,
+                                   SmallVectorImpl<DictionaryAttr> &argAttrs,
+                                   SmallVectorImpl<Type> &resultTypes,
+                                   SmallVectorImpl<DictionaryAttr> &resultAttrs,
+                                   bool mustParseEmptyResult = true);
+
+/// Print a function signature for a call or callable operation. If a body
+/// region is provided, the SSA arguments are printed in the signature. When
+/// `printEmptyResult` is false, `-> function-result-list` is omitted when
+/// `resultTypes` is empty.
+///
+///   function-signature     ::= ssa-function-signature
+///                            | no-ssa-function-signature
+///   ssa-function-signature ::= `(` ssa-function-arg-list `)`
+///                            -> function-result-list
+///   ssa-function-arg-list  ::= ssa-function-arg (`,` ssa-function-arg)*
+///   ssa-function-arg       ::= `%`name `:` type attribute-dict?
+void printFunctionSignature(OpAsmPrinter &p,
+                            OpWithArgumentAttributesInterface op,
+                            TypeRange argTypes, bool isVariadic,
+                            TypeRange resultTypes, Region *body = nullptr,
+                            bool printEmptyResult = true);
+
+/// Adds argument and result attributes, provided as `argAttrs` and
+/// `resultAttrs` arguments, to the list of operation attributes in `result`.
+/// Internally, argument and result attributes are stored as dict attributes
+/// with special names given by getResultAttrName, getArgumentAttrName.
+void addArgAndResultAttrs(Builder &builder, OperationState &result,
+                          ArrayRef<DictionaryAttr> argAttrs,
+                          ArrayRef<DictionaryAttr> resultAttrs,
+                          StringAttr argAttrsName, StringAttr resAttrsName);
+void addArgAndResultAttrs(Builder &builder, OperationState &result,
+                          ArrayRef<OpAsmParser::Argument> args,
+                          ArrayRef<DictionaryAttr> resultAttrs,
+                          StringAttr argAttrsName, StringAttr resAttrsName);
+
+} // namespace call_interface_impl
+
+} // namespace mlir
+
+#endif // MLIR_INTERFACES_CALLIMPLEMENTATION_H
diff --git a/mlir/include/mlir/Interfaces/CallInterfaces.td b/mlir/include/mlir/Interfaces/CallInterfaces.td
index c6002da0d491ce..80912a9762187e 100644
--- a/mlir/include/mlir/Interfaces/CallInterfaces.td
+++ b/mlir/include/mlir/Interfaces/CallInterfaces.td
@@ -17,12 +17,65 @@
 
 include "mlir/IR/OpBase.td"
 
+
+/// Interface for operations with arguments attributes (both call-like
+/// and callable operations).
+def OpWithArgumentAttributesInterface : OpInterface<"OpWithArgumentAttributesInterface"> {
+  let description = [{
+    A call-like or callable operation that may define attributes for its arguments. 
+  }];
+  let cppNamespace = "::mlir";
+  let methods = [
+    InterfaceMethod<[{
+        Get the array of argument attribute dictionaries. The method should
+        return an array attribute containing only dictionary attributes equal in
+        number to the number of arguments. Alternatively, the method can
+        return null to indicate that the region has no argument attributes.
+      }],
+      "::mlir::ArrayAttr", "getArgAttrsAttr", (ins),
+      /*methodBody=*/[{}], /*defaultImplementation=*/[{ return nullptr; }]>,
+    InterfaceMethod<[{
+        Get the array of result attribute dictionaries. The method should return
+        an array attribute containing only dictionary attributes equal in number
+        to the number of results. Alternatively, the method can return
+        null to indicate that the region has no result attributes.
+      }],
+      "::mlir::ArrayAttr", "getResAttrsAttr", (ins),
+      /*methodBody=*/[{}], /*defaultImplementation=*/[{ return nullptr; }]>,
+    InterfaceMethod<[{
+      Set the array of argument attribute dictionaries.
+    }],
+    "void", "setArgAttrsAttr", (ins "::mlir::ArrayAttr":$attrs),
+      /*methodBody=*/[{}], /*defaultImplementation=*/[{ return; }]>,
+    InterfaceMethod<[{
+      Set the array of result attribute dictionaries.
+    }],
+    "void", "setResAttrsAttr", (ins "::mlir::ArrayAttr":$attrs),
+      /*methodBody=*/[{}], /*defaultImplementation=*/[{ return; }]>,
+    InterfaceMethod<[{
+      Remove the array of argument attribute dictionaries. This is the same as
+      setting all argument attributes to an empty dictionary. The method should
+      return the removed attribute.
+    }],
+    "::mlir::Attribute", "removeArgAttrsAttr", (ins),
+      /*methodBody=*/[{}], /*defaultImplementation=*/[{ return nullptr; }]>,
+    InterfaceMethod<[{
+      Remove the array of result attribute dictionaries. This is the same as
+      setting all result attributes to an empty dictionary. The method should
+      return the removed attribute.
+    }],
+    "::mlir::Attribute", "removeResAttrsAttr", (ins),
+      /*methodBody=*/[{}], /*defaultImplementation=*/[{ return nullptr; }]>,
+  ];
+}
+
 // `CallInterfaceCallable`: This is a type used to represent a single callable
 // region. A callable is either a symbol, or an SSA value, that is referenced by
 // a call-like operation. This represents the destination of the call.
 
 /// Interface for call-like operations.
-def CallOpInterface : OpInterface<"CallOpInterface"> {
+def CallOpInterface : OpInterface<"CallOpInterface",
+    [OpWithArgumentAttributesInterface]> {
   let description = [{
     A call-like operation is one that transfers control from one sub-routine to
     another. These operations may be traditional direct calls `call @foo`, or
@@ -85,7 +138,8 @@ def CallOpInterface : OpInterface<"CallOpInterface"> {
 }
 
 /// Interface for callable operations.
-def CallableOpInterface : OpInterface<"CallableOpInterface"> {
+def CallableOpInterface : OpInterface<"CallableOpInterface",
+    [OpWithArgumentAttributesInterface]> {
   let description = [{
     A callable operation is one who represents a potential sub-routine, and may
     be a target for a call-like operation (those providing the CallOpInterface
@@ -113,47 +167,6 @@ def CallableOpInterface : OpInterface<"CallableOpInterface"> {
       allow for this method may be called on function declarations).
     }],
     "::llvm::ArrayRef<::mlir::Type>", "getResultTypes">,
-
-    InterfaceMethod<[{
-        Get the array of argument attribute dictionaries. The method should
-        return an array attribute containing only dictionary attributes equal in
-        number to the number of region arguments. Alternatively, the method can
-        return null to indicate that the region has no argument attributes.
-      }],
-      "::mlir::ArrayAttr", "getArgAttrsAttr", (ins),
-      /*methodBody=*/[{}], /*defaultImplementation=*/[{ return nullptr; }]>,
-    InterfaceMethod<[{
-        Get the array of result attribute dictionaries. The method should return
-        an array attribute containing only dictionary attributes equal in number
-        to the number of region results. Alternatively, the method can return
-        null to indicate that the region has no result attributes.
-      }],
-      "::mlir::ArrayAttr", "getResAttrsAttr", (ins),
-      /*methodBody=*/[{}], /*defaultImplementation=*/[{ return nullptr; }]>,
-    InterfaceMethod<[{
-      Set the array of argument attribute dictionaries.
-    }],
-    "void", "setArgAttrsAttr", (ins "::mlir::ArrayAttr":$attrs),
-      /*methodBody=*/[{}], /*defaultImplementation=*/[{ return; }]>,
-    InterfaceMethod<[{
-      Set the array of result attribute dictionaries.
-    }],
-    "void", "setResAttrsAttr", (ins "::mlir::ArrayAttr":$attrs),
-      /*methodBody=*/[{}], /*defaultImplementation=*/[{ return; }]>,
-    InterfaceMethod<[{
-      Remove the array of argument attribute dictionaries. This is the same as
-      setting all argument attributes to an empty dictionary. The method should
-      return the removed attribute.
-    }],
-    "::mlir::Attribute", "removeArgAttrsAttr", (ins),
-      /*methodBody=*/[{}], /*defaultImplementation=*/[{ return nullptr; }]>,
-    InterfaceMethod<[{
-      Remove the array of result attribute dictionaries. This is the same as
-      setting all result attributes to an empty dictionary. The method should
-      return the removed attribute.
-    }],
-    "::mlir::Attribute", "removeResAttrsAttr", (ins),
-      /*methodBody=*/[{}], /*defaultImplementation=*/[{ return nullptr; }]>,
   ];
 }
 
diff --git a/mlir/include/mlir/Interfaces/FunctionImplementation.h b/mlir/include/mlir/Interfaces/FunctionImplementation.h
index a5e6963e4e666f..ae20533ef4b87c 100644
--- a/mlir/include/mlir/Interfaces/FunctionImplementation.h
+++ b/mlir/include/mlir/Interfaces/FunctionImplementation.h
@@ -16,6 +16,7 @@
 #define MLIR_IR_FUNCTIONIMPLEMENTATION_H_
 
 #include "mlir/IR/OpImplementation.h"
+#include "mlir/Interfaces/CallImplementation.h"
 #include "mlir/Interfaces/FunctionInterfaces.h"
 
 namespace mlir {
@@ -33,19 +34,6 @@ class VariadicFlag {
   bool variadic;
 };
 
-/// Adds argument and result attributes, provided as `argAttrs` and
-/// `resultAttrs` arguments, to the list of operation attributes in `result`.
-/// Internally, argument and result attributes are stored as dict attributes
-/// with special names given by getResultAttrName, getArgumentAttrName.
-void addArgAndResultAttrs(Builder &builder, OperationState &result,
-                          ArrayRef<DictionaryAttr> argAttrs,
-                          ArrayRef<DictionaryAttr> resultAttrs,
-                          StringAttr argAttrsName, StringAttr resAttrsName);
-void addArgAndResultAttrs(Builder &builder, OperationState &result,
-                          ArrayRef<OpAsmParser::Argument> args,
-                          ArrayRef<DictionaryAttr> resultAttrs,
-                          StringAttr argAttrsName, StringAttr resAttrsName);
-
 /// Callback type for `parseFunctionOp`, the callback should produce the
 /// type that will be associated with a function-like operation from lists of
 /// function arguments and results, VariadicFlag indicates whether the function
@@ -84,9 +72,13 @@ void printFunctionOp(OpAsmPrinter &p, FunctionOpInterface op, bool isVariadic,
 
 /// Prints the signature of the function-like operation `op`. Assumes `op` has
 /// is a FunctionOpInterface and has passed verification.
-void printFunctionSignature(OpAsmPrinter &p, FunctionOpInterface op,
-                            ArrayRef<Type> argTypes, bool isVariadic,
-                            ArrayRef<Type> resultTypes);
+inline void printFunctionSignature(OpAsmPrinter &p, FunctionOpInterface op,
+                                   ArrayRef<Type> argTypes, bool isVariadic,
+                                   ArrayRef<Type> resultTypes) {
+  call_interface_impl::printFunctionSignature(p, op, argTypes, isVariadic,
+                                              resultTypes, &op->getRegion(0),
+                                              /*printEmptyResult=*/false);
+}
 
 /// Prints the list of function prefixed with the "attributes" keyword. The
 /// attributes with names listed in "elided" as well as those used by the
diff --git a/mlir/lib/Dialect/Async/IR/Async.cpp b/mlir/lib/Dialect/Async/IR/Async.cpp
index a3e3f80954efce..d3bb250bb8ab9d 100644
--- a/mlir/lib/Dialect/Async/IR/Async.cpp
+++ b/mlir/lib/Dialect/Async/IR/Async.cpp
@@ -308,7 +308,7 @@ void FuncOp::build(OpBuilder &builder, OperationState &state, StringRef name,
   if (argAttrs.empty())
     return;
   assert(type.getNumInputs() == argAttrs.size());
-  function_interface_impl::addArgAndResultAttrs(
+  call_interface_impl::addArgAndResultAttrs(
       builder, state, argAttrs, /*resultAttrs=*/std::nullopt,
       getArgAttrsAttrName(state.name), getResAttrsAttrName(state.name));
 }
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index fdc21d6c6e24b9..6af17087ced968 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -529,7 +529,7 @@ void FuncOp::build(OpBuilder &builder, OperationState &state, StringRef name,
   if (argAttrs.empty())
     return;
   assert(type.getNumInputs() == argAttrs.size());
-  function_interface_impl::addArgAndResultAttrs(
+  call_interface_impl::addArgAndResultAttrs(
       builder, state, argAttrs, /*resultAttrs=*/std::nullopt,
       getArgAttrsAttrName(state.name), getResAttrsAttrName(state.name));
 }
diff --git a/mlir/lib/Dialect/Func/IR/FuncOps.cpp b/mlir/lib/Dialect/Func/IR/FuncOps.cpp
index a490b4c3c4ab43..ba7b84f27d6a8a 100644
--- a/mlir/lib/Dialect/Func/IR/FuncOps.cpp
+++ b/mlir/lib/Dialect/Func/IR/FuncOps.cpp
@@ -190,7 +190,7 @@ void FuncOp::build(OpBuilder &builder, OperationState &state, StringRef name,
   if (argAttrs.empty())
     return;
   assert(type.getNumInputs() == argAttrs.size());
-  function_interface_impl::addArgAndResultAttrs(
+  call_interface_impl::addArgAndResultAttrs(
       builder, state, argAttrs, /*resultAttrs=*/std::nullopt,
       getArgAttrsAttrName(state.name), getResAttrsAttrName(state.name));
 }
diff --git a/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp b/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp
index 49209229259a73..8b85c0829acfec 100644
--- a/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp
+++ b/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp
@@ -1487,7 +1487,7 @@ ParseResult GPUFuncOp::parse(OpAsmParser &parser, OperationState &result) {
   result.addAttribute(getFunctionTypeAttrName(result.name),
                       TypeAttr::get(type));
 
-  function_interface_impl::addArgAndResultAttrs(
+  call_interface_impl::addArgAndResultAttrs(
       builder, result, entryArgs, resultAttrs, getArgAttrsAttrName(result.name),
       getResAttrsAttrName(result.name));
 
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index ef5f1b069b40a3..ef1e0222e05f06 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2510,7 +2510,7 @@ void LLVMFuncOp::build(OpBuilder &builder, OperationState &result,
 
   assert(llvm::cast<LLVMFunctionType>(type).getNumParams() == argAttrs.size() &&
          "expected as many argument attribute lists as arguments");
-  function_interface_impl::addArgAndResultAttrs(
+  call_interface_impl::addArgAndResultAttrs(
       builder, result, argAttrs, /*resultAttrs=*/std::nullopt,
       getArgAttrsAttrName(result.name), getResAttrsAttrName(result.name));
 }
@@ -2636,7 +2636,7 @@ ParseResult LLVMFuncOp::parse(OpAsmParser &parser, OperationState &result) {
 
   if (failed(parser.parseOptionalAttrDictWithKeyword(result.attributes)))
     return failure();
-  function_interface_impl::addArgAndResultAttrs(
+  call_interface_impl::addArgAndResultAttrs(
       parser.getBuilder(), result, entryArgs, resultAttrs,
       getArgAttrsAttrName(result.name), getResAttrsAttrName(result.name));
 
diff --git a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
index 26559c1321db5e..870359ce55301c 100644
--- a/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
@@ -940,7 +940,7 @@ ParseResult spirv::FuncOp::parse(OpAsmParser &parser, OperationState &result) {
 
   // Add the attributes to the function arguments.
   assert(resultAttrs.size() == resultTypes.size());
-  function_interface_impl::addArgAndResultAttrs(
+  call_interface_impl::addArgAndResultAttrs(
       builder, result, entryArgs, resultAttrs, getArgAttrsAttrName(result.name),
       getResAttrsAttrName(result.name));
 
diff --git a/mlir/lib/Dialect/Shape/IR/Shape.cpp b/mlir/lib/Dialect/Shape/IR/Shape.cpp
index 65efc88e9c4033..2200af0f67a862 100644
--- a/mlir/lib/Dialect/Shape/IR/Shape.cpp
+++ b/mlir/lib/Dialect/Shape/IR/Shape.cpp
@@ -1297,7 +1297,7 @@ void FuncOp::build(OpBuilder &builder, OperationState &state, StringRef name,
   if (argAttrs.empty())
     return;
   assert(type.getNumInputs() == argAttrs.size());
-  function_interface_impl::addArgAndResultAttrs(
+  call_interface_impl::addArgAndResultAttrs(
       builder, state, argAttrs, /*resultAttrs=*/std::nullopt,
       getArgAttrsAttrName(state.name), getResAttrsAttrName(state.name));
 }
diff --git a/mlir/lib/Interfaces/CMakeLists.txt b/mlir/lib/Interfaces/CMakeLists.txt
index d3b7bf65ad3e73..76e2d921c2a9f5 100644
--- a/mlir/lib/Interfaces/CMakeLists.txt
+++ b/mlir/lib/Interfaces/CMakeLists.txt
@@ -1,4 +1,5 @@
 set(LLVM_OPTIONAL_SOURCES
+  CallImplementation.cpp
   CallInterfaces.cpp
   CastInterfaces.cpp
   ControlF...
[truncated]

River707

The general code movement seems good!

mlir/include/mlir/Interfaces/CallImplementation.h

mlir/include/mlir/Interfaces/CallInterfaces.td

mlir/lib/Interfaces/CallImplementation.cpp

Share methods via listconcat instead. Make methods mandatory (no default impl). Add arg_attrs and res_attrs to all concrete operations inheriting from CallOpInterface and CallableOpInterface.

jeanPerier · 2025-01-21T10:42:42Z

Thanks for the feedback @River707!

I updated this PR based on your comments and the RFC to make the methods mandatory in both CallOpInterafce and CallableOpInterface without using an extra OpInterface. This patch is now adding new optional arg_attrs and res_attrs attributes to all concrete operations inheriting these interfaces.

gysit

LGTM from my end.

Thanks for working on this feature! I added some nits. Please pick the ones that make sense.

mlir/examples/toy/Ch7/mlir/Dialect.cpp

mlir/include/mlir/Interfaces/CallInterfaces.td

mlir/lib/Interfaces/CallInterfaces.cpp

mlir/include/mlir/Interfaces/CallInterfaces.h

Co-authored-by: Tobias Gysi <tobias.gysi@nextsilicon.com>

jeanPerier · 2025-01-27T13:15:29Z

LGTM from my end.

Thanks for working on this feature! I added some nits. Please pick the ones that make sense.

Thanks for the review! The nits made sense to me. I applied them.

krzysz00 · 2025-01-27T19:00:09Z

Silly question: will this allow arg/result attributes on intrinsics?

River707 · 2025-01-27T19:03:02Z

mlir/include/mlir/Interfaces/CallInterfaces.h

@@ -14,6 +14,7 @@
 #ifndef MLIR_INTERFACES_CALLINTERFACES_H
 #define MLIR_INTERFACES_CALLINTERFACES_H

+#include "mlir/IR/OpImplementation.h"


OpImplementation.h pulls in a lot of extra junk, can you use forward declares here instead for whatever is needed?

No, because of OpAsmParser::Argument usage, I think it is not possible to forward declare nested types.

Either that class should be hoisted in a new OpAsmParserArgument class (a bit outside of the scope of this patch), or the helpers need to be in a different header (maybe that is why FunctionImplementation.h exists).

What do you think, is it worth splitting the header to avoid that include?

I plan to not split the header. Will likely merge on Monday, please tell me if you think splitting the header is better.

mlir/include/mlir/Interfaces/CallInterfaces.td

jeanPerier · 2025-01-28T15:11:15Z

Silly question: will this allow arg/result attributes on intrinsics?

Not through this interface change because llvm.call_intrinsic does not use the CallOpInterface.

So if this is desired, this should be added separately on the llvm.call_intrinsic.

…#123176

(#125475)

…callables (llvm#123176)" This reverts commit 327d627.

llvm#123176) This patch shares core interface methods dealing with argument and result attributes from CallableOpInterface with the CallOpInterface and makes them mandatory to gives more consistent guarantees about concrete operations using these interfaces. This allows adding argument attributes on call like operations, which is sometimes required to get proper ABI, like with llvm.call (and llvm.invoke). The patch adds optional `arg_attrs` and `res_attrs` attributes to operations using these interfaces that did not have that already. They can then re-use the common "rich function signature" printing/parsing helpers if they want (for the LLVM dialect, this is done in the next patch). Part of RFC: https://discourse.llvm.org/t/mlir-rfc-adding-argument-and-result-attributes-to-llvm-call/84107

…#123176 (llvm#125475)

Following #123176. Signed-off-by: Joshua James Venter <venter.joshua@gmail.com>

… and `res_attrs` llvm/llvm-project#123176

…141018) Following llvm#123176. Signed-off-by: Joshua James Venter <venter.joshua@gmail.com>

@mehrdad2m

**Context:** We update the llvm version tagged by jax 0.6.0: ``` mhlo=617a9361d186199480c080c9e8c474a5e30c22d1 llvm=179d30f8c3fddd3c85056fd2b8e877a4a8513158 ``` We also update Enzyme to the latest version, which is 0.0.180, at commit `db0181320d6e425ee963bd496ed0d8dbb615be18` **Description of the Change:** Firstly, jax recently moved from the Google github organization to its own jax-ml organization. This means the urls, and the retrieval method for the underlying llvm and mhlo git commit tags, needs to be updated. (Thanks @mehrdad2m !) Now on to the actual changes. I will list the changes in increasing complexity. 1. The new enzyme cmake target is `EnzymeStatic-21` (from 20) 2. Enzyme works with a later llvm then our target, so it has some llvm intrinsics unknown to the one we are targeting. We patch them away. They do not concern us since they are all intrinsics for nvidia backends. 3. `applyPatternsAndFoldGreedily` is removed. Drop-in replacement is `applyPatternsGreedily`. llvm/llvm-project#104649, llvm/llvm-project#126701 4. ops with `CallOpInterface` must have two new optional attributes `arg_attrs` and `res_attrs` llvm/llvm-project#123176 5. `CallInterfaceCallable` objects now must be directly casted to the callee `SymbolRefAttr`, i.e. `callee.get<SymbolRefAttr>()` -> `cast<SymbolRefAttr>(callee)` llvm/llvm-project@35e8989 6. The `lookupOrCreateFn` family of functions now return `FailureOr<funcop>` instead of just `funcop`, so a `.value()` needs to be used to retrieve the underlying `funcop`. llvm/llvm-project@e84f6b6 7. The cpp api for `OneShotBufferizePassOptions` no longer needs complicated lambdas for the type converter options. They can be set with the `mlir::bufferization::LayoutMapOption::IdentityLayoutMap` options directly. 8. The individual `match` and `rewrite` methods in pattern rewrites are removed. Use the two-in-one `matchAndRewrite` instead. llvm/llvm-project#129861 9. For rewrite patterns with 1-to-N convertions, a new `macthAndRewrite` overload with `OneToNOpAdaptor` must be used. For us, this is only the `catalyst.list*` ops. llvm/llvm-project#116470 10. The lowering of `cf::AssertOp` to llvm was split from the overall`--covert-cf-to-llvm` pass. We need to manually call this separate pattern for cf.assert duriing quantum to llvm dialect lowering, where we also convert cf to llvm. https://github.com/llvm/llvm-project/pull/120431/files 11. The new mhlo depends on a [shardy](https://github.com/openxla/shardy) dialect. Shardy is built with bazel, not cmake. Building shardy ourselves would be very difficult (not having bazel in our build ecosystem is a hard constraint, cc @mlxd ), and also not necessary (we just use mhlo for their "standard" passes). We thus patch out all shardy components. 12. Three necessary passes were removed in mhlo: `mhlo-legalize-control-flow`, `mhlo-legalize-to-std`, `hlo-legalize-sort` tensorflow/mlir-hlo@4a640be#diff-ef0d7e30da19a396ba036405a9ef636f8b1be194618b0a90f4602671fc2ef34d tensorflow/mlir-hlo@2a5e267#diff-f8c7cb07b43593403e00e0dbf9983f0186b4eb70368cc99af3b924061f1ea46f - Alongside the removal of `mhlo-legalize-to-std`, the cmake target `MhloToStandard` was removed too. We simply patch them back for now. **For the above two points, note that there will be an overall migration to the stablehlo repo, as mhlo is sunseting. Therefore, spending too much time on this isn't necessary, so we just patch.** 13. The new pattern applicator (`applyPatternsGreedily`) is more aggressive in dead code elimination, and is eliminating dead `Value`s in the adjoint gradient method. The `nodealloc` function we generate for adjoint gradient lowering used to only return the qreg, not the expval result. This causes the expval op to be eliminated since it has no users. This further causes wrong gradient results, since the entire program, all ops included (regardless of dead or not), impacts the gradient through chain rule. To avoid this, we return the expval result as well. In doing this, we implicitly assume that differentiated qnodes can only return expval. Although this assumption is true and also restricted by frontend, ideally we should not have it hard coded. We leave this as a TODO for a future feature. 14. The old `--buffer-deallocation` pass is removed. Intended replacement is `--buffer-deallocation-pipeline`. This migration is very complicated. We simply add back the old buffer deallocation pass in the catalyst dialect as a util for now. We will revisit this in #1778 . mlir lit test updates: 1. `bufferization.to_tensor/memref` updated assembly format 2. gradient adjoint lowering test returns both qreg and expval 3. Some inverse unrealized conversion cast pairs are canceled by the new pattern rewriter. 4. `llvm.mlir.undef` is deprecated, use `llvm.mlir.poison` instead. llvm/llvm-project#125629 **Benefits:** Up to date with upstream versions. [sc-92017] --------- Co-authored-by: Tzung-Han Juang <tzunghan.juang@gmail.com> Co-authored-by: Ritu Thombre <42207923+ritu-thombre99@users.noreply.github.com> Co-authored-by: Mehrdad Malekmohammadi <mehrdad.malek@xanadu.ai> Co-authored-by: Mehrdad Malek <39844030+mehrdad2m@users.noreply.github.com> Co-authored-by: David Ittah <dime10@users.noreply.github.com> Co-authored-by: Joey Carter <joseph.carter@xanadu.ai>

…141018) Following llvm#123176. Signed-off-by: Joshua James Venter <venter.joshua@gmail.com>

[mlir] share argument attributes interface between calls and callables

b652f02

jeanPerier requested review from ftynse, zero9178 and gysit January 16, 2025 10:03

jeanPerier requested review from antiagainst and kuhar as code owners January 16, 2025 10:03

llvmbot added mlir:llvm mlir:gpu mlir:spirv mlir mlir:async mlir:emitc mlir:func mlir:shape labels Jan 16, 2025

jeanPerier mentioned this pull request Jan 16, 2025

[mlir][LLVM] handle argument and result attributes in llvm.call and llvm.invoke #123177

Merged

rename and change inheritance level

3328f68

llvmbot added the mlir:core MLIR Core Infrastructure label Jan 16, 2025

River707 requested changes Jan 16, 2025

View reviewed changes

mlir/include/mlir/Interfaces/CallImplementation.h Outdated Show resolved Hide resolved

mlir/include/mlir/Interfaces/CallInterfaces.td Outdated Show resolved Hide resolved

mlir/lib/Interfaces/CallImplementation.cpp Outdated Show resolved Hide resolved

PR comments: remove new interface and added files

43cd704

Share methods via listconcat instead. Make methods mandatory (no default impl). Add arg_attrs and res_attrs to all concrete operations inheriting from CallOpInterface and CallableOpInterface.

jeanPerier requested a review from nicolasvasilache as a code owner January 21, 2025 10:39

llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:codegen labels Jan 21, 2025

jeanPerier requested a review from clementval January 21, 2025 10:40

update toy dialect too

803451d

jeanPerier requested a review from River707 January 23, 2025 09:46

gysit approved these changes Jan 27, 2025

View reviewed changes

Apply suggestions from code review

b48b0df

Co-authored-by: Tobias Gysi <tobias.gysi@nextsilicon.com>

rename parseFunctionSignature to parseFunctionSignatureWithArguments

9c7ee71

River707 approved these changes Jan 27, 2025

View reviewed changes

use concat operator instead of lisconcat

64dd585

jeanPerier merged commit 327d627 into main Feb 3, 2025
9 checks passed

jeanPerier deleted the users/jeanPerier/mlir-call-attrs-iface branch February 3, 2025 10:27

jeanPerier added a commit to jeanPerier/llvm-project that referenced this pull request Feb 3, 2025

[mlir] add missing dependency to libMLIRFunctionInterfaces after llvm…

25a4eb6

…#123176

jeanPerier added a commit that referenced this pull request Feb 3, 2025

[mlir] add missing dependency to libMLIRFunctionInterfaces after #123176

5fdf161

(#125475)

hanhanW added a commit to iree-org/llvm-project that referenced this pull request Feb 4, 2025

Revert "[mlir] share argument attributes interface between calls and …

1a708bd

…callables (llvm#123176)" This reverts commit 327d627.

Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025

[mlir] add missing dependency to libMLIRFunctionInterfaces after llvm…

2446573

…#123176 (llvm#125475)

makslevental mentioned this pull request Feb 20, 2025

[backend] Update LLVM version to https://github.com/llvm/llvm-project/commit/386af4a5c64ab75eaee2448dc38f2e34a40bfed0 triton-lang/triton#5974

Closed

jjvraw mentioned this pull request May 22, 2025

[mlir] Explain required attrs for CallOpInterface in Toy (NFC) #141018

Merged

joker-eph pushed a commit that referenced this pull request May 22, 2025

[mlir] Explain required attrs for CallOpInterface in Toy (NFC) (#141018)

d0fbfa6

Following #123176. Signed-off-by: Joshua James Venter <venter.joshua@gmail.com>

paul0403 added a commit to PennyLaneAI/catalyst that referenced this pull request May 27, 2025

ops with CallOpInterface must have two new optional attrs arg_attrs…

ad96d6e

… and `res_attrs` llvm/llvm-project#123176

paul0403 mentioned this pull request May 28, 2025

Update llvm version PennyLaneAI/catalyst#1752

Merged

sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025

[mlir] Explain required attrs for CallOpInterface in Toy (NFC) (llvm#…

43ff378

…141018) Following llvm#123176. Signed-off-by: Joshua James Venter <venter.joshua@gmail.com>

ajaden-codes pushed a commit to Jaddyen/llvm-project that referenced this pull request Jun 6, 2025

[mlir] Explain required attrs for CallOpInterface in Toy (NFC) (llvm#…

83da2b9

…141018) Following llvm#123176. Signed-off-by: Joshua James Venter <venter.joshua@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[mlir] share argument attributes interface between calls and callables #123176

[mlir] share argument attributes interface between calls and callables #123176

Uh oh!

jeanPerier commented Jan 16, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jan 16, 2025 •

edited

Loading

Uh oh!

River707 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeanPerier commented Jan 21, 2025

Uh oh!

gysit left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeanPerier commented Jan 27, 2025

Uh oh!

krzysz00 commented Jan 27, 2025

Uh oh!

River707 Jan 27, 2025

Uh oh!

jeanPerier Jan 28, 2025

Uh oh!

jeanPerier Jan 31, 2025

Uh oh!

Uh oh!

Uh oh!

jeanPerier commented Jan 28, 2025

Uh oh!

Uh oh!

Uh oh!

[mlir] share argument attributes interface between calls and callables #123176

[mlir] share argument attributes interface between calls and callables #123176

Uh oh!

Conversation

jeanPerier commented Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

River707 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeanPerier commented Jan 21, 2025

Uh oh!

gysit left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeanPerier commented Jan 27, 2025

Uh oh!

krzysz00 commented Jan 27, 2025

Uh oh!

River707 Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

jeanPerier Jan 28, 2025

Choose a reason for hiding this comment

Uh oh!

jeanPerier Jan 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jeanPerier commented Jan 28, 2025

Uh oh!

Uh oh!

Uh oh!

jeanPerier commented Jan 16, 2025 •

edited

Loading

llvmbot commented Jan 16, 2025 •

edited

Loading