[Mlir-commits] [mlir] Update MLIR conversion to LLVMFunc to account better for properties (PR #67406)
Mehdi Amini
llvmlistbot at llvm.org
Mon Oct 9 00:39:50 PDT 2023
https://github.com/joker-eph updated https://github.com/llvm/llvm-project/pull/67406
>From 325f69f11475297b4938671fe5779aa277f7ff4a Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Tue, 26 Sep 2023 01:44:14 -0700
Subject: [PATCH] Update MLIR conversion to LLVMFunc to account better for
properties
Change the attribute propagation to handle only discardable attributes,
inherent attribute are handled directly and args/results through the
interface.
---
mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td | 1 +
mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp | 174 ++++++++----------
.../Conversion/FuncToLLVM/convert-funcs.mlir | 17 ++
3 files changed, 97 insertions(+), 95 deletions(-)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 25209ce4497455e..8745d14c8d48318 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -1395,6 +1395,7 @@ def LLVM_LLVMFuncOp : LLVM_Op<"func", [
let arguments = (ins
StrAttr:$sym_name,
+ OptionalAttr<StrAttr>:$sym_visibility,
TypeAttrOf<LLVM_FunctionType>:$function_type,
DefaultValuedAttr<Linkage, "Linkage::External">:$linkage,
UnitAttr:$dso_local,
diff --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index d52f01880282e1a..4aacb47a7fe9cc1 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -73,44 +73,40 @@ static bool shouldUseBarePtrCallConv(Operation *op,
}
/// Only retain those attributes that are not constructed by
-/// `LLVMFuncOp::build`. If `filterArgAttrs` is set, also filter out argument
-/// attributes.
-static void filterFuncAttributes(func::FuncOp func, bool filterArgAndResAttrs,
+/// `LLVMFuncOp::build`.
+static void filterFuncAttributes(func::FuncOp func,
SmallVectorImpl<NamedAttribute> &result) {
- for (const NamedAttribute &attr : func->getAttrs()) {
- if (attr.getName() == SymbolTable::getSymbolAttrName() ||
- attr.getName() == func.getFunctionTypeAttrName() ||
- attr.getName() == linkageAttrName ||
+ for (const NamedAttribute &attr : func->getDiscardableAttrs()) {
+ if (attr.getName() == linkageAttrName ||
attr.getName() == varargsAttrName ||
- attr.getName() == LLVM::LLVMDialect::getReadnoneAttrName() ||
- (filterArgAndResAttrs &&
- (attr.getName() == func.getArgAttrsAttrName() ||
- attr.getName() == func.getResAttrsAttrName())))
+ attr.getName() == LLVM::LLVMDialect::getReadnoneAttrName())
continue;
result.push_back(attr);
}
}
-/// Adds a an empty set of argument attributes for the newly added argument in
-/// front of the existing ones.
-static void prependEmptyArgAttr(OpBuilder &builder,
- SmallVectorImpl<NamedAttribute> &newFuncAttrs,
- func::FuncOp func) {
- auto argAttrs = func.getArgAttrs();
- // Nothing to do when there were no arg attrs beforehand.
- if (!argAttrs)
- return;
-
- size_t numArguments = func.getNumArguments();
- SmallVector<Attribute> newArgAttrs;
- newArgAttrs.reserve(numArguments + 1);
- // Insert empty dictionary for the new argument.
- newArgAttrs.push_back(builder.getDictionaryAttr({}));
-
- llvm::append_range(newArgAttrs, *argAttrs);
- auto newNamedAttr = builder.getNamedAttr(func.getArgAttrsAttrName(),
- builder.getArrayAttr(newArgAttrs));
- newFuncAttrs.push_back(newNamedAttr);
+/// Propagate argument/results attributes.
+static void propagateArgResAttrs(OpBuilder &builder, bool resultStructType,
+ func::FuncOp funcOp,
+ LLVM::LLVMFuncOp wrapperFuncOp) {
+ auto argAttrs = funcOp.getArgAttrs();
+ if (!resultStructType) {
+ if (auto resAttrs = funcOp.getAllResultAttrs())
+ wrapperFuncOp.setAllResultAttrs(resAttrs);
+ if (argAttrs)
+ wrapperFuncOp.setAllArgAttrs(*argAttrs);
+ } else {
+ SmallVector<Attribute> argAttributes;
+ // Only modify the argument and result attributes when the result is now
+ // an argument.
+ if (argAttrs) {
+ argAttributes.push_back(builder.getDictionaryAttr({}));
+ argAttributes.append(argAttrs->begin(), argAttrs->end());
+ wrapperFuncOp.setAllArgAttrs(argAttributes);
+ }
+ }
+ if (funcOp.getSymVisibilityAttr())
+ wrapperFuncOp.setSymVisibility(funcOp.getSymVisibilityAttr());
}
/// Creates an auxiliary function with pointer-to-memref-descriptor-struct
@@ -129,18 +125,14 @@ static void wrapForExternalCallers(OpBuilder &rewriter, Location loc,
auto [wrapperFuncType, resultStructType] =
typeConverter.convertFunctionTypeCWrapper(type);
- SmallVector<NamedAttribute, 4> attributes;
- // Only modify the argument and result attributes when the result is now an
- // argument.
- if (resultStructType)
- prependEmptyArgAttr(rewriter, attributes, funcOp);
- filterFuncAttributes(
- funcOp, /*filterArgAndResAttrs=*/static_cast<bool>(resultStructType),
- attributes);
+ SmallVector<NamedAttribute> attributes;
+ filterFuncAttributes(funcOp, attributes);
+
auto wrapperFuncOp = rewriter.create<LLVM::LLVMFuncOp>(
loc, llvm::formatv("_mlir_ciface_{0}", funcOp.getName()).str(),
wrapperFuncType, LLVM::Linkage::External, /*dsoLocal=*/false,
/*cconv=*/LLVM::CConv::C, /*comdat=*/nullptr, attributes);
+ propagateArgResAttrs(rewriter, !!resultStructType, funcOp, wrapperFuncOp);
OpBuilder::InsertionGuard guard(rewriter);
rewriter.setInsertionPointToStart(wrapperFuncOp.addEntryBlock());
@@ -199,19 +191,14 @@ static void wrapExternalFunction(OpBuilder &builder, Location loc,
assert(wrapperType && "unexpected type conversion failure");
SmallVector<NamedAttribute, 4> attributes;
- // Only modify the argument and result attributes when the result is now an
- // argument.
- if (resultStructType)
- prependEmptyArgAttr(builder, attributes, funcOp);
- filterFuncAttributes(
- funcOp, /*filterArgAndResAttrs=*/static_cast<bool>(resultStructType),
- attributes);
+ filterFuncAttributes(funcOp, attributes);
// Create the auxiliary function.
auto wrapperFunc = builder.create<LLVM::LLVMFuncOp>(
loc, llvm::formatv("_mlir_ciface_{0}", funcOp.getName()).str(),
wrapperType, LLVM::Linkage::External, /*dsoLocal=*/false,
/*cconv=*/LLVM::CConv::C, /*comdat=*/nullptr, attributes);
+ propagateArgResAttrs(builder, !!resultStructType, funcOp, wrapperFunc);
// The wrapper that we synthetize here should only be visible in this module.
newFuncOp.setLinkage(LLVM::Linkage::Private);
@@ -351,21 +338,56 @@ struct FuncOpConversionBase : public ConvertOpToLLVMPattern<func::FuncOp> {
if (!llvmType)
return rewriter.notifyMatchFailure(funcOp, "signature conversion failed");
+ // Create an LLVM function, use external linkage by default until MLIR
+ // functions have linkage.
+ LLVM::Linkage linkage = LLVM::Linkage::External;
+ if (funcOp->hasAttr(linkageAttrName)) {
+ auto attr =
+ dyn_cast<mlir::LLVM::LinkageAttr>(funcOp->getAttr(linkageAttrName));
+ if (!attr) {
+ funcOp->emitError() << "Contains " << linkageAttrName
+ << " attribute not of type LLVM::LinkageAttr";
+ return rewriter.notifyMatchFailure(
+ funcOp, "Contains linkage attribute not of type LLVM::LinkageAttr");
+ }
+ linkage = attr.getLinkage();
+ }
+
+ SmallVector<NamedAttribute, 4> attributes;
+ filterFuncAttributes(funcOp, attributes);
+ auto newFuncOp = rewriter.create<LLVM::LLVMFuncOp>(
+ funcOp.getLoc(), funcOp.getName(), llvmType, linkage,
+ /*dsoLocal=*/false, /*cconv=*/LLVM::CConv::C, /*comdat=*/nullptr,
+ attributes);
+ if (funcOp.getSymVisibilityAttr())
+ newFuncOp.setSymVisibility(funcOp.getSymVisibilityAttr());
+
+ // Create a memory effect attribute corresponding to readnone.
+ StringRef readnoneAttrName = LLVM::LLVMDialect::getReadnoneAttrName();
+ if (funcOp->hasAttr(readnoneAttrName)) {
+ auto attr = funcOp->getAttrOfType<UnitAttr>(readnoneAttrName);
+ if (!attr) {
+ funcOp->emitError() << "Contains " << readnoneAttrName
+ << " attribute not of type UnitAttr";
+ return rewriter.notifyMatchFailure(
+ funcOp, "Contains readnone attribute not of type UnitAttr");
+ }
+ auto memoryAttr = LLVM::MemoryEffectsAttr::get(
+ rewriter.getContext(),
+ {LLVM::ModRefInfo::NoModRef, LLVM::ModRefInfo::NoModRef,
+ LLVM::ModRefInfo::NoModRef});
+ newFuncOp.setMemoryAttr(memoryAttr);
+ }
+
// Propagate argument/result attributes to all converted arguments/result
// obtained after converting a given original argument/result.
- SmallVector<NamedAttribute, 4> attributes;
- filterFuncAttributes(funcOp, /*filterArgAndResAttrs=*/true, attributes);
if (ArrayAttr resAttrDicts = funcOp.getAllResultAttrs()) {
assert(!resAttrDicts.empty() && "expected array to be non-empty");
- auto newResAttrDicts =
- (funcOp.getNumResults() == 1)
- ? resAttrDicts
- : rewriter.getArrayAttr(rewriter.getDictionaryAttr({}));
- attributes.push_back(
- rewriter.getNamedAttr(funcOp.getResAttrsAttrName(), newResAttrDicts));
+ if (funcOp.getNumResults() == 1)
+ newFuncOp.setAllResultAttrs(resAttrDicts);
}
if (ArrayAttr argAttrDicts = funcOp.getAllArgAttrs()) {
- SmallVector<Attribute, 4> newArgAttrs(
+ SmallVector<Attribute> newArgAttrs(
cast<LLVM::LLVMFunctionType>(llvmType).getNumParams());
for (unsigned i = 0, e = funcOp.getNumArguments(); i < e; ++i) {
// Some LLVM IR attribute have a type attached to them. During FuncOp ->
@@ -415,48 +437,10 @@ struct FuncOpConversionBase : public ConvertOpToLLVMPattern<func::FuncOp> {
newArgAttrs[mapping->inputNo + j] =
DictionaryAttr::get(rewriter.getContext(), {});
}
- attributes.push_back(rewriter.getNamedAttr(
- funcOp.getArgAttrsAttrName(), rewriter.getArrayAttr(newArgAttrs)));
+ if (!newArgAttrs.empty())
+ newFuncOp.setAllArgAttrs(rewriter.getArrayAttr(newArgAttrs));
}
- // Create an LLVM function, use external linkage by default until MLIR
- // functions have linkage.
- LLVM::Linkage linkage = LLVM::Linkage::External;
- if (funcOp->hasAttr(linkageAttrName)) {
- auto attr =
- dyn_cast<mlir::LLVM::LinkageAttr>(funcOp->getAttr(linkageAttrName));
- if (!attr) {
- funcOp->emitError() << "Contains " << linkageAttrName
- << " attribute not of type LLVM::LinkageAttr";
- return rewriter.notifyMatchFailure(
- funcOp, "Contains linkage attribute not of type LLVM::LinkageAttr");
- }
- linkage = attr.getLinkage();
- }
-
- // Create a memory effect attribute corresponding to readnone.
- StringRef readnoneAttrName = LLVM::LLVMDialect::getReadnoneAttrName();
- LLVM::MemoryEffectsAttr memoryAttr = {};
- if (funcOp->hasAttr(readnoneAttrName)) {
- auto attr = funcOp->getAttrOfType<UnitAttr>(readnoneAttrName);
- if (!attr) {
- funcOp->emitError() << "Contains " << readnoneAttrName
- << " attribute not of type UnitAttr";
- return rewriter.notifyMatchFailure(
- funcOp, "Contains readnone attribute not of type UnitAttr");
- }
- memoryAttr = LLVM::MemoryEffectsAttr::get(rewriter.getContext(),
- {LLVM::ModRefInfo::NoModRef,
- LLVM::ModRefInfo::NoModRef,
- LLVM::ModRefInfo::NoModRef});
- }
- auto newFuncOp = rewriter.create<LLVM::LLVMFuncOp>(
- funcOp.getLoc(), funcOp.getName(), llvmType, linkage,
- /*dsoLocal=*/false, /*cconv=*/LLVM::CConv::C, /*comdat=*/nullptr,
- attributes);
- // If the memory attribute was created, add it to the function.
- if (memoryAttr)
- newFuncOp.setMemoryAttr(memoryAttr);
rewriter.inlineRegionBefore(funcOp.getBody(), newFuncOp.getBody(),
newFuncOp.end());
if (failed(rewriter.convertRegionTypes(&newFuncOp.getBody(), *typeConverter,
diff --git a/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir b/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir
index 3526af2b28af5da..9b576ef51866884 100644
--- a/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir
+++ b/mlir/test/Conversion/FuncToLLVM/convert-funcs.mlir
@@ -61,6 +61,23 @@ func.func @variadic_func(%arg0: i32) attributes { "func.varargs" = true } {
return
}
+// -----
+
+// CHECK-LABEL: llvm.func @private_callee
+// CHECK-SAME: sym_visibility = "private"
+func.func private @private_callee(%arg1: f32) -> i32 {
+ %0 = arith.constant 0 : i32
+ return %0 : i32
+}
+
+// CHECK-LABEL: llvm.func @caller_private_callee
+func.func @caller_private_callee(%arg1: f32) -> i32 {
+ %0 = call @private_callee(%arg1) : (f32) -> i32
+ return %0 : i32
+}
+
+
+
// -----
func.func private @badllvmlinkage(i32) attributes { "llvm.linkage" = 3 : i64 } // expected-error {{Contains llvm.linkage attribute not of type LLVM::LinkageAttr}}
More information about the Mlir-commits
mailing list