[MLIR][FuncToLLVM] Propagate no_inline attribute #143809
Conversation
|
@llvm/pr-subscribers-mlir Author: None (junfengd-nv) ChangesThis commit fixes the no_inline attribute propagation from func::FuncOp to LLVM::LLVMFuncOp and adds corresponding test coverage. After the change, functions marked with no_inline in the func dialect maintain this optimization hint when lowered to LLVM IR. Full diff: https://github.com/llvm/llvm-project/pull/143809.diff 2 Files Affected:
diff --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index 328c605add65c..35228fb9c9e72 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -62,6 +62,7 @@ using namespace mlir;
static constexpr StringRef varargsAttrName = "func.varargs";
static constexpr StringRef linkageAttrName = "llvm.linkage";
static constexpr StringRef barePtrAttrName = "llvm.bareptr";
+static constexpr StringRef noInlineAttr = "no_inline";
/// Return `true` if the `op` should use bare pointer calling convention.
static bool shouldUseBarePtrCallConv(Operation *op,
@@ -381,6 +382,11 @@ mlir::convertFuncOpToLLVMFuncOp(FunctionOpInterface funcOp,
newFuncOp.setMemoryEffectsAttr(memoryAttr);
}
+ // Propagate no_inline attributes
+ if (funcOp->hasAttr(noInlineAttr)) {
+ newFuncOp->setAttr(noInlineAttr, rewriter.getUnitAttr());
+ }
+
// Propagate argument/result attributes to all converted arguments/result
// obtained after converting a given original argument/result.
if (ArrayAttr resAttrDicts = funcOp.getAllResultAttrs()) {
diff --git a/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir b/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir
index ae1dc70d0686b..b4e6cdffb7640 100644
--- a/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir
+++ b/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir
@@ -96,3 +96,11 @@ func.func private @badllvmlinkage(i32) attributes { "llvm.linkage" = 3 : i64 } /
func.func @variadic_func(%arg0: i32) attributes { "func.varargs" = true, "llvm.emit_c_interface" } {
return
}
+
+// -----
+
+// Check that no_inline attribute is propagated from func::FuncOp to LLVM::LLVMFuncOp
+// CHECK-LABEL: llvm.func @func_with_noinline(%arg0: i32) attributes {no_inline}
+func.func @func_with_noinline(%arg0: i32) attributes { no_inline } {
+ return
+}
|
ashermancinelli
requested review from
ashermancinelli,
matthias-springer and
vzakhari
|
Thank you, Junfeng! It looks good to me. Please give Matthias time to also review this. |
| static constexpr StringRef varargsAttrName = "func.varargs"; | ||
| static constexpr StringRef linkageAttrName = "llvm.linkage"; | ||
| static constexpr StringRef barePtrAttrName = "llvm.bareptr"; | ||
| static constexpr StringRef noInlineAttrName = "no_inline"; |
There was a problem hiding this comment.
I'd rather not rely on magic constants in local files and get this somehow from the interface or from specific op classes.
There was a problem hiding this comment.
Thank you for the comment, @ftynse!
Would that be acceptable to add the get/set interface methods into FunctionOpInterface and then implement it for func.func, llvm.func, etc.? Or did you mean to suggest something else?
There was a problem hiding this comment.
I didn't have a specific suggestion, just observed that such action-at-a-distance is dangerous. The op definition may change the name of the attribute to, e.g., noinline without the underscore, and this code will silently break. Adding interface methods would make sense to be, doubly so if these methods are then used in the inliner logic.
There was a problem hiding this comment.
doubly so if these methods are then used in the inliner logic.
Thanks! It sounds quite reasonable to add such interface methods in FunctionOpInterface.
4 participants
This commit fixes the no_inline attribute propagation from func::FuncOp to LLVM::LLVMFuncOp and adds corresponding test coverage. After the change, functions marked with no_inline in the func dialect maintain this optimization hint when lowered to LLVM IR.