[Mlir-commits] [mlir] 6423c90 - [mlir][LLVM][NFC] Avoid rollback in FuncOp --> LLVM lowering (#136477)

[llvmlistbot at llvm.org]

Author: Matthias Springer
Date: 2025-04-21T10:38:49+02:00
New Revision: 6423c901fd5a5a8370a5326c5fbe16eb5a837374

URL: https://github.com/llvm/llvm-project/commit/6423c901fd5a5a8370a5326c5fbe16eb5a837374
DIFF: https://github.com/llvm/llvm-project/commit/6423c901fd5a5a8370a5326c5fbe16eb5a837374.diff

LOG: [mlir][LLVM][NFC] Avoid rollback in FuncOp --> LLVM lowering (#136477)

This pattern used to create an `llvm.func` op, then check additional
requirements and return "failure". This commit moves the checks before
the creation of the replacement op, so that no rollback is necessary
when one of the checks fails.

Note: This is in preparation of the One-Shot Dialect Conversion
refactoring, which removes the rollback functionality.

Added: 
    

Modified: 
    mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
index 55f0a9ac3bbb2..328c605add65c 100644
--- a/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
+++ b/mlir/lib/Conversion/FuncToLLVM/FuncToLLVM.cpp
@@ -320,13 +320,22 @@ mlir::convertFuncOpToLLVMFuncOp(FunctionOpInterface funcOp,
   // overriden with an LLVM pointer type for later processing.
   SmallVector<std::optional<NamedAttribute>> byValRefNonPtrAttrs;
   TypeConverter::SignatureConversion result(funcOp.getNumArguments());
-  auto llvmType = converter.convertFunctionSignature(
-      funcOp, varargsAttr && varargsAttr.getValue(),
-      shouldUseBarePtrCallConv(funcOp, &converter), result,
-      byValRefNonPtrAttrs);
+  auto llvmType = dyn_cast_or_null<LLVM::LLVMFunctionType>(
+      converter.convertFunctionSignature(
+          funcOp, varargsAttr && varargsAttr.getValue(),
+          shouldUseBarePtrCallConv(funcOp, &converter), result,
+          byValRefNonPtrAttrs));
   if (!llvmType)
     return rewriter.notifyMatchFailure(funcOp, "signature conversion failed");
 
+  // Check for unsupported variadic functions.
+  if (!shouldUseBarePtrCallConv(funcOp, &converter))
+    if (funcOp->getAttrOfType<UnitAttr>(
+            LLVM::LLVMDialect::getEmitCWrapperAttrName()))
+      if (llvmType.isVarArg())
+        return funcOp.emitError("C interface for variadic functions is not "
+                                "supported yet.");
+
   // Create an LLVM function, use external linkage by default until MLIR
   // functions have linkage.
   LLVM::Linkage linkage = LLVM::Linkage::External;
@@ -342,6 +351,18 @@ mlir::convertFuncOpToLLVMFuncOp(FunctionOpInterface funcOp,
     linkage = attr.getLinkage();
   }
 
+  // Check for invalid attributes.
+  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");
+    }
+  }
+
   SmallVector<NamedAttribute, 4> attributes;
   filterFuncAttributes(funcOp, attributes);
   auto newFuncOp = rewriter.create<LLVM::LLVMFuncOp>(
@@ -352,15 +373,7 @@ mlir::convertFuncOpToLLVMFuncOp(FunctionOpInterface funcOp,
       .setVisibility(funcOp.getVisibility());
 
   // 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,
@@ -447,10 +460,6 @@ mlir::convertFuncOpToLLVMFuncOp(FunctionOpInterface funcOp,
   if (!shouldUseBarePtrCallConv(funcOp, &converter)) {
     if (funcOp->getAttrOfType<UnitAttr>(
             LLVM::LLVMDialect::getEmitCWrapperAttrName())) {
-      if (newFuncOp.isVarArg())
-        return funcOp.emitError("C interface for variadic functions is not "
-                                "supported yet.");
-
       if (newFuncOp.isExternal())
         wrapExternalFunction(rewriter, funcOp->getLoc(), converter, funcOp,
                              newFuncOp);