[Mlir-commits] [mlir] [MLIR] Account for named attributes without dialect prefixes in `FuncToLLVM` lowering pass (PR #182987)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Ayokunle Amodu (ayokunle321)

<details>
<summary>Changes</summary>

Fixes https://github.com/llvm/llvm-project/issues/181464. 

Explanation of the issue:
In `convertFuncOpToLLVMFuncOp`, the function `filterFuncAttributes` is called to remove named attributes already handled by the `FuncOp`'s build function. This is done so that the `FuncOp`'s list of named attributes does not have duplicates when trying to create a sorted dictionary out of it. In the crash-inducing test below, the named attribute `linkage` slips through the filter because it does not have its dialect prefix, i.e., it is defined as `linkage` and not `llvm.linkage` (the pass was designed to handle only dialect-prefixed attributes, but MLIR does not enforce dialect prefixed attributes). Due to this, an assert that checks for duplicates in the attribute list fails.

Minimal IR:
```
module {
  func.func @<!-- -->host() attributes {linkage = #llvm.linkage<internal>} {
    return
  }
}
```

This patch updates the `FuncToLLVM` lowering pass to account for attribute names without dialect prefixes.

@<!-- -->matthias-springer I know the fix is funky 😔, please bare with me. Open to suggestions for a better workaround.


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


1 Files Affected:

- (modified) mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp (+22-7) 


``````````diff
diff --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index 2220f61ed8a07..d0b33bae6d0a5 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -52,10 +52,15 @@ static constexpr StringRef varargsAttrName = "func.varargs";
 static constexpr StringRef linkageAttrName = "llvm.linkage";
 static constexpr StringRef barePtrAttrName = "llvm.bareptr";
 
+static constexpr StringRef varargsAttrNameNoPrefix = "varargs";
+static constexpr StringRef linkageAttrNameNoPrefix = "linkage";
+static constexpr StringRef barePtrAttrNameNoPrefix = "bareptr";
+
 /// Return `true` if the `op` should use bare pointer calling convention.
 static bool shouldUseBarePtrCallConv(Operation *op,
                                      const LLVMTypeConverter *typeConverter) {
-  return (op && op->hasAttr(barePtrAttrName)) ||
+  return (op && (op->hasAttr(barePtrAttrName) ||
+                 op->hasAttr(barePtrAttrNameNoPrefix))) ||
          typeConverter->getOptions().useBarePtrCallConv;
 }
 
@@ -65,7 +70,9 @@ static void filterFuncAttributes(FunctionOpInterface func,
                                  SmallVectorImpl<NamedAttribute> &result) {
   for (const NamedAttribute &attr : func->getDiscardableAttrs()) {
     if (attr.getName() == linkageAttrName ||
+        attr.getName() == linkageAttrNameNoPrefix ||
         attr.getName() == varargsAttrName ||
+        attr.getName() == varargsAttrNameNoPrefix ||
         attr.getName() == LLVM::LLVMDialect::getReadnoneAttrName())
       continue;
     result.push_back(attr);
@@ -299,7 +306,10 @@ FailureOr<LLVM::LLVMFuncOp> mlir::convertFuncOpToLLVMFuncOp(
 
   // Convert the original function arguments. They are converted using the
   // LLVMTypeConverter provided to this legalization pattern.
-  auto varargsAttr = funcOp->getAttrOfType<BoolAttr>(varargsAttrName);
+  auto varargsAttr =
+      funcOp->getAttrOfType<BoolAttr>(varargsAttrName)
+          ? funcOp->getAttrOfType<BoolAttr>(varargsAttrName)
+          : funcOp->getAttrOfType<BoolAttr>(varargsAttrNameNoPrefix);
   // Gather `llvm.byval` and `llvm.byref` arguments whose type convertion was
   // overriden with an LLVM pointer type for later processing.
   SmallVector<std::optional<NamedAttribute>> byValRefNonPtrAttrs;
@@ -323,9 +333,12 @@ FailureOr<LLVM::LLVMFuncOp> mlir::convertFuncOpToLLVMFuncOp(
   // 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 (funcOp->hasAttr(linkageAttrName) ||
+      funcOp->hasAttr(linkageAttrNameNoPrefix)) {
+    auto attr = dyn_cast_or_null<mlir::LLVM::LinkageAttr>(
+        funcOp->getAttr(linkageAttrName)
+            ? funcOp->getAttr(linkageAttrName)
+            : funcOp->getAttr(linkageAttrNameNoPrefix));
     if (!attr) {
       funcOp->emitError() << "Contains " << linkageAttrName
                           << " attribute not of type LLVM::LinkageAttr";
@@ -651,13 +664,15 @@ class CallOpLowering : public CallOpInterfaceLowering<func::CallOp> {
       Operation *callee =
           symbolTables->lookupNearestSymbolFrom(callOp, callOp.getCalleeAttr());
       useBarePtrCallConv =
-          callee != nullptr && callee->hasAttr(barePtrAttrName);
+          callee != nullptr && (callee->hasAttr(barePtrAttrName) ||
+                                callee->hasAttr(barePtrAttrNameNoPrefix));
     } else {
       // Warning: This is a linear lookup.
       Operation *callee =
           SymbolTable::lookupNearestSymbolFrom(callOp, callOp.getCalleeAttr());
       useBarePtrCallConv =
-          callee != nullptr && callee->hasAttr(barePtrAttrName);
+          callee != nullptr && (callee->hasAttr(barePtrAttrName) ||
+                                callee->hasAttr(barePtrAttrNameNoPrefix));
     }
     return matchAndRewriteImpl(callOp, adaptor, rewriter, useBarePtrCallConv);
   }

``````````

</details>


https://github.com/llvm/llvm-project/pull/182987