[mlir] allow function type cloning to fail #137130
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`FunctionOpInterface` assumed the fact that the function type (attribute of the operation) can be cloned with arbirary lists of function arguments and results to support argument and result list mutation. This is not always correct, in particular, LLVM dialect functions require exactly one result making it impossible to erase the result. Allow function type cloning to fail and propagate this failure through various APIs that use it. The common assumption is that existing IR has not been modified. Fixes llvm#131142. Reland a8c7ecd / llvm#136300.
llvmbot
added
mlir:core
|
@llvm/pr-subscribers-mlir-gpu @llvm/pr-subscribers-mlir Author: Oleksandr "Alex" Zinenko (ftynse) Changes
Allow function type cloning to fail and propagate this failure through various APIs that use it. The common assumption is that existing IR has not been modified. Fixes #131142. Reland a8c7ecd / #136300. Full diff: https://github.com/llvm/llvm-project/pull/137130.diff 11 Files Affected:
diff --git a/flang/lib/Optimizer/Transforms/AbstractResult.cpp b/flang/lib/Optimizer/Transforms/AbstractResult.cpp
index f8badfa639f94..59e2eeb76c715 100644
--- a/flang/lib/Optimizer/Transforms/AbstractResult.cpp
+++ b/flang/lib/Optimizer/Transforms/AbstractResult.cpp
@@ -387,8 +387,12 @@ class AbstractResultOpt
mlir::OpBuilder rewriter(context);
auto resultType = funcTy.getResult(0);
auto argTy = getResultArgumentType(resultType, shouldBoxResult);
- func.insertArgument(0u, argTy, {}, loc);
- func.eraseResult(0u);
+ llvm::LogicalResult res = func.insertArgument(0u, argTy, {}, loc);
+ (void)res;
+ assert(llvm::succeeded(res) && "failed to insert function argument");
+ res = func.eraseResult(0u);
+ (void)res;
+ assert(llvm::succeeded(res) && "failed to erase function result");
mlir::Value newArg = func.getArgument(0u);
if (mustEmboxResult(resultType, shouldBoxResult)) {
auto bufferType = fir::ReferenceType::get(resultType);
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
index e9f01ba5cd4e4..072fc7be07dd4 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
@@ -104,7 +104,8 @@ def LLVMFunctionType : LLVMType<"LLVMFunction", "func"> {
bool isVarArg() const { return getVarArg(); }
/// Returns a clone of this function type with the given argument
- /// and result types.
+ /// and result types. Returns null if the resulting function type would
+ /// not verify.
LLVMFunctionType clone(TypeRange inputs, TypeRange results) const;
/// Returns the result type of the function as an ArrayRef, enabling better
diff --git a/mlir/include/mlir/Interfaces/FunctionInterfaces.td b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
index 1e2ca9a8b4490..cad0e790fdd97 100644
--- a/mlir/include/mlir/Interfaces/FunctionInterfaces.td
+++ b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
@@ -255,79 +255,105 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
BlockArgListType getArguments() { return getFunctionBody().getArguments(); }
/// Insert a single argument of type `argType` with attributes `argAttrs` and
- /// location `argLoc` at `argIndex`.
- void insertArgument(unsigned argIndex, ::mlir::Type argType, ::mlir::DictionaryAttr argAttrs,
- ::mlir::Location argLoc) {
- insertArguments({argIndex}, {argType}, {argAttrs}, {argLoc});
+ /// location `argLoc` at `argIndex`. Returns failure if the function cannot be
+ /// updated to have the new signature.
+ ::llvm::LogicalResult insertArgument(
+ unsigned argIndex, ::mlir::Type argType, ::mlir::DictionaryAttr argAttrs,
+ ::mlir::Location argLoc) {
+ return insertArguments({argIndex}, {argType}, {argAttrs}, {argLoc});
}
/// Inserts arguments with the listed types, attributes, and locations at the
/// listed indices. `argIndices` must be sorted. Arguments are inserted in the
/// order they are listed, such that arguments with identical index will
- /// appear in the same order that they were listed here.
- void insertArguments(::llvm::ArrayRef<unsigned> argIndices, ::mlir::TypeRange argTypes,
- ::llvm::ArrayRef<::mlir::DictionaryAttr> argAttrs,
- ::llvm::ArrayRef<::mlir::Location> argLocs) {
+ /// appear in the same order that they were listed here. Returns failure if
+ /// the function cannot be updated to have the new signature.
+ ::llvm::LogicalResult insertArguments(
+ ::llvm::ArrayRef<unsigned> argIndices, ::mlir::TypeRange argTypes,
+ ::llvm::ArrayRef<::mlir::DictionaryAttr> argAttrs,
+ ::llvm::ArrayRef<::mlir::Location> argLocs) {
unsigned originalNumArgs = $_op.getNumArguments();
::mlir::Type newType = $_op.getTypeWithArgsAndResults(
argIndices, argTypes, /*resultIndices=*/{}, /*resultTypes=*/{});
+ if (!newType)
+ return ::llvm::failure();
::mlir::function_interface_impl::insertFunctionArguments(
$_op, argIndices, argTypes, argAttrs, argLocs,
originalNumArgs, newType);
+ return ::llvm::success();
}
- /// Insert a single result of type `resultType` at `resultIndex`.
- void insertResult(unsigned resultIndex, ::mlir::Type resultType,
- ::mlir::DictionaryAttr resultAttrs) {
- insertResults({resultIndex}, {resultType}, {resultAttrs});
+ /// Insert a single result of type `resultType` at `resultIndex`.Returns
+ /// failure if the function cannot be updated to have the new signature.
+ ::llvm::LogicalResult insertResult(
+ unsigned resultIndex, ::mlir::Type resultType,
+ ::mlir::DictionaryAttr resultAttrs) {
+ return insertResults({resultIndex}, {resultType}, {resultAttrs});
}
/// Inserts results with the listed types at the listed indices.
/// `resultIndices` must be sorted. Results are inserted in the order they are
/// listed, such that results with identical index will appear in the same
- /// order that they were listed here.
- void insertResults(::llvm::ArrayRef<unsigned> resultIndices, ::mlir::TypeRange resultTypes,
- ::llvm::ArrayRef<::mlir::DictionaryAttr> resultAttrs) {
+ /// order that they were listed here. Returns failure if the function
+ /// cannot be updated to have the new signature.
+ ::llvm::LogicalResult insertResults(
+ ::llvm::ArrayRef<unsigned> resultIndices,
+ ::mlir::TypeRange resultTypes,
+ ::llvm::ArrayRef<::mlir::DictionaryAttr> resultAttrs) {
unsigned originalNumResults = $_op.getNumResults();
::mlir::Type newType = $_op.getTypeWithArgsAndResults(
/*argIndices=*/{}, /*argTypes=*/{}, resultIndices, resultTypes);
+ if (!newType)
+ return ::llvm::failure();
::mlir::function_interface_impl::insertFunctionResults(
$_op, resultIndices, resultTypes, resultAttrs,
originalNumResults, newType);
+ return ::llvm::success();
}
- /// Erase a single argument at `argIndex`.
- void eraseArgument(unsigned argIndex) {
+ /// Erase a single argument at `argIndex`. Returns failure if the function
+ /// cannot be updated to have the new signature.
+ ::llvm::LogicalResult eraseArgument(unsigned argIndex) {
::llvm::BitVector argsToErase($_op.getNumArguments());
argsToErase.set(argIndex);
- eraseArguments(argsToErase);
+ return eraseArguments(argsToErase);
}
- /// Erases the arguments listed in `argIndices`.
- void eraseArguments(const ::llvm::BitVector &argIndices) {
+ /// Erases the arguments listed in `argIndices`. Returns failure if the
+ /// function cannot be updated to have the new signature.
+ ::llvm::LogicalResult eraseArguments(const ::llvm::BitVector &argIndices) {
::mlir::Type newType = $_op.getTypeWithoutArgs(argIndices);
+ if (!newType)
+ return ::llvm::failure();
::mlir::function_interface_impl::eraseFunctionArguments(
$_op, argIndices, newType);
+ return ::llvm::success();
}
- /// Erase a single result at `resultIndex`.
- void eraseResult(unsigned resultIndex) {
+ /// Erase a single result at `resultIndex`. Returns failure if the function
+ /// cannot be updated to have the new signature.
+ LogicalResult eraseResult(unsigned resultIndex) {
::llvm::BitVector resultsToErase($_op.getNumResults());
resultsToErase.set(resultIndex);
- eraseResults(resultsToErase);
+ return eraseResults(resultsToErase);
}
- /// Erases the results listed in `resultIndices`.
- void eraseResults(const ::llvm::BitVector &resultIndices) {
+ /// Erases the results listed in `resultIndices`. Returns failure if the
+ /// function cannot be updated to have the new signature.
+ ::llvm::LogicalResult eraseResults(const ::llvm::BitVector &resultIndices) {
::mlir::Type newType = $_op.getTypeWithoutResults(resultIndices);
+ if (!newType)
+ return ::llvm::failure();
::mlir::function_interface_impl::eraseFunctionResults(
$_op, resultIndices, newType);
+ return ::llvm::success();
}
/// Return the type of this function with the specified arguments and
/// results inserted. This is used to update the function's signature in
/// the `insertArguments` and `insertResults` methods. The arrays must be
- /// sorted by increasing index.
+ /// sorted by increasing index. Return nullptr if the updated type would
+ /// not be valid.
::mlir::Type getTypeWithArgsAndResults(
::llvm::ArrayRef<unsigned> argIndices, ::mlir::TypeRange argTypes,
::llvm::ArrayRef<unsigned> resultIndices, ::mlir::TypeRange resultTypes) {
@@ -341,7 +367,8 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
/// Return the type of this function without the specified arguments and
/// results. This is used to update the function's signature in the
- /// `eraseArguments` and `eraseResults` methods.
+ /// `eraseArguments` and `eraseResults` methods. Return nullptr if the
+ /// updated type would not be valid.
::mlir::Type getTypeWithoutArgsAndResults(
const ::llvm::BitVector &argIndices, const ::llvm::BitVector &resultIndices) {
::llvm::SmallVector<::mlir::Type> argStorage, resultStorage;
diff --git a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
index f22ad1fd70db2..dea3d99704d9b 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
@@ -125,8 +125,12 @@ GPUFuncOpLowering::matchAndRewrite(gpu::GPUFuncOp gpuFuncOp, OpAdaptor adaptor,
// Perform signature modification
rewriter.modifyOpInPlace(
gpuFuncOp, [gpuFuncOp, &argIndices, &argTypes, &argAttrs, &argLocs]() {
- static_cast<FunctionOpInterface>(gpuFuncOp).insertArguments(
- argIndices, argTypes, argAttrs, argLocs);
+ LogicalResult inserted =
+ static_cast<FunctionOpInterface>(gpuFuncOp).insertArguments(
+ argIndices, argTypes, argAttrs, argLocs);
+ (void)inserted;
+ assert(succeeded(inserted) &&
+ "expected GPU funcs to support inserting any argument");
});
} else {
workgroupBuffers.reserve(gpuFuncOp.getNumWorkgroupAttributions());
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp b/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
index 1c95ab77b9f33..acf5f7767d12a 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
@@ -92,7 +92,8 @@ updateFuncOp(func::FuncOp func,
}
// Erase the results.
- func.eraseResults(erasedResultIndices);
+ if (failed(func.eraseResults(erasedResultIndices)))
+ return failure();
// Add the new arguments to the entry block if the function is not external.
if (func.isExternal())
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp b/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp
index 9bc75267e70e4..ae011904cb972 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp
@@ -113,7 +113,8 @@ mlir::bufferization::dropEquivalentBufferResults(ModuleOp module) {
}
// Update function.
- funcOp.eraseResults(erasedResultIndices);
+ if (failed(funcOp.eraseResults(erasedResultIndices)))
+ return failure();
returnOp.getOperandsMutable().assign(newReturnValues);
// Update function calls.
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
index 29cf38c1fefea..e2bccf7f6493e 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
@@ -232,7 +232,10 @@ LLVMFunctionType::getChecked(function_ref<InFlightDiagnostic()> emitError,
LLVMFunctionType LLVMFunctionType::clone(TypeRange inputs,
TypeRange results) const {
- assert(results.size() == 1 && "expected a single result type");
+ if (results.size() != 1 || !isValidResultType(results[0]))
+ return {};
+ if (!llvm::all_of(inputs, isValidArgumentType))
+ return {};
return get(results[0], llvm::to_vector(inputs), isVarArg());
}
diff --git a/mlir/lib/Query/Query.cpp b/mlir/lib/Query/Query.cpp
index 869ee8f2ae1dc..73f313cd37fd0 100644
--- a/mlir/lib/Query/Query.cpp
+++ b/mlir/lib/Query/Query.cpp
@@ -88,10 +88,11 @@ static Operation *extractFunction(std::vector<Operation *> &ops,
// Remove unused function arguments
size_t currentIndex = 0;
while (currentIndex < funcOp.getNumArguments()) {
+ // Erase if possible.
if (funcOp.getArgument(currentIndex).use_empty())
- funcOp.eraseArgument(currentIndex);
- else
- ++currentIndex;
+ if (succeeded(funcOp.eraseArgument(currentIndex)))
+ continue;
+ ++currentIndex;
}
return funcOp;
diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index 92c05d87a002d..4939dda4a20f9 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -698,8 +698,11 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) {
// 3. Functions
for (auto &f : list.functions) {
- f.funcOp.eraseArguments(f.nonLiveArgs);
- f.funcOp.eraseResults(f.nonLiveRets);
+ // Some functions may not allow erasing arguments or results. These calls
+ // return failure in such cases without modifying the function, so it's okay
+ // to proceed.
+ (void)f.funcOp.eraseArguments(f.nonLiveArgs);
+ (void)f.funcOp.eraseResults(f.nonLiveRets);
}
// 4. Operands
diff --git a/mlir/test/IR/test-func-erase-result.mlir b/mlir/test/IR/test-func-erase-result.mlir
index 8fe40b2fdfcbc..a32866227547b 100644
--- a/mlir/test/IR/test-func-erase-result.mlir
+++ b/mlir/test/IR/test-func-erase-result.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s -test-func-erase-result -split-input-file | FileCheck %s
+// RUN: mlir-opt %s -test-func-erase-result -split-input-file -verify-diagnostics | FileCheck %s
// CHECK: func private @f(){{$}}
// CHECK-NOT: attributes{{.*}}result
@@ -66,3 +66,8 @@ func.func private @f() -> (
f32 {test.erase_this_result},
tensor<3xf32>
)
+
+// -----
+
+// expected-error @below {{failed to erase results}}
+llvm.func @llvm_func(!llvm.ptr, i64)
diff --git a/mlir/test/lib/IR/TestFunc.cpp b/mlir/test/lib/IR/TestFunc.cpp
index 2ade47249863c..94a4610365863 100644
--- a/mlir/test/lib/IR/TestFunc.cpp
+++ b/mlir/test/lib/IR/TestFunc.cpp
@@ -45,8 +45,12 @@ struct TestFuncInsertArg
: unknownLoc);
}
func->removeAttr("test.insert_args");
- func.insertArguments(indicesToInsert, typesToInsert, attrsToInsert,
- locsToInsert);
+ if (succeeded(func.insertArguments(indicesToInsert, typesToInsert,
+ attrsToInsert, locsToInsert)))
+ continue;
+
+ emitError(func->getLoc()) << "failed to insert arguments";
+ return signalPassFailure();
}
}
};
@@ -79,7 +83,12 @@ struct TestFuncInsertResult
: DictionaryAttr::get(&getContext()));
}
func->removeAttr("test.insert_results");
- func.insertResults(indicesToInsert, typesToInsert, attrsToInsert);
+ if (succeeded(func.insertResults(indicesToInsert, typesToInsert,
+ attrsToInsert)))
+ continue;
+
+ emitError(func->getLoc()) << "failed to insert results";
+ return signalPassFailure();
}
}
};
@@ -100,7 +109,10 @@ struct TestFuncEraseArg
for (auto argIndex : llvm::seq<int>(0, func.getNumArguments()))
if (func.getArgAttr(argIndex, "test.erase_this_arg"))
indicesToErase.set(argIndex);
- func.eraseArguments(indicesToErase);
+ if (succeeded(func.eraseArguments(indicesToErase)))
+ continue;
+ emitError(func->getLoc()) << "failed to erase arguments";
+ return signalPassFailure();
}
}
};
@@ -122,7 +134,10 @@ struct TestFuncEraseResult
for (auto resultIndex : llvm::seq<int>(0, func.getNumResults()))
if (func.getResultAttr(resultIndex, "test.erase_this_result"))
indicesToErase.set(resultIndex);
- func.eraseResults(indicesToErase);
+ if (succeeded(func.eraseResults(indicesToErase)))
+ continue;
+ emitError(func->getLoc()) << "failed to erase results";
+ return signalPassFailure();
}
}
};
|
|
@llvm/pr-subscribers-mlir-llvm Author: Oleksandr "Alex" Zinenko (ftynse) Changes
Allow function type cloning to fail and propagate this failure through various APIs that use it. The common assumption is that existing IR has not been modified. Fixes #131142. Reland a8c7ecd / #136300. Full diff: https://github.com/llvm/llvm-project/pull/137130.diff 11 Files Affected:
diff --git a/flang/lib/Optimizer/Transforms/AbstractResult.cpp b/flang/lib/Optimizer/Transforms/AbstractResult.cpp
index f8badfa639f94..59e2eeb76c715 100644
--- a/flang/lib/Optimizer/Transforms/AbstractResult.cpp
+++ b/flang/lib/Optimizer/Transforms/AbstractResult.cpp
@@ -387,8 +387,12 @@ class AbstractResultOpt
mlir::OpBuilder rewriter(context);
auto resultType = funcTy.getResult(0);
auto argTy = getResultArgumentType(resultType, shouldBoxResult);
- func.insertArgument(0u, argTy, {}, loc);
- func.eraseResult(0u);
+ llvm::LogicalResult res = func.insertArgument(0u, argTy, {}, loc);
+ (void)res;
+ assert(llvm::succeeded(res) && "failed to insert function argument");
+ res = func.eraseResult(0u);
+ (void)res;
+ assert(llvm::succeeded(res) && "failed to erase function result");
mlir::Value newArg = func.getArgument(0u);
if (mustEmboxResult(resultType, shouldBoxResult)) {
auto bufferType = fir::ReferenceType::get(resultType);
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
index e9f01ba5cd4e4..072fc7be07dd4 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
@@ -104,7 +104,8 @@ def LLVMFunctionType : LLVMType<"LLVMFunction", "func"> {
bool isVarArg() const { return getVarArg(); }
/// Returns a clone of this function type with the given argument
- /// and result types.
+ /// and result types. Returns null if the resulting function type would
+ /// not verify.
LLVMFunctionType clone(TypeRange inputs, TypeRange results) const;
/// Returns the result type of the function as an ArrayRef, enabling better
diff --git a/mlir/include/mlir/Interfaces/FunctionInterfaces.td b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
index 1e2ca9a8b4490..cad0e790fdd97 100644
--- a/mlir/include/mlir/Interfaces/FunctionInterfaces.td
+++ b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
@@ -255,79 +255,105 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
BlockArgListType getArguments() { return getFunctionBody().getArguments(); }
/// Insert a single argument of type `argType` with attributes `argAttrs` and
- /// location `argLoc` at `argIndex`.
- void insertArgument(unsigned argIndex, ::mlir::Type argType, ::mlir::DictionaryAttr argAttrs,
- ::mlir::Location argLoc) {
- insertArguments({argIndex}, {argType}, {argAttrs}, {argLoc});
+ /// location `argLoc` at `argIndex`. Returns failure if the function cannot be
+ /// updated to have the new signature.
+ ::llvm::LogicalResult insertArgument(
+ unsigned argIndex, ::mlir::Type argType, ::mlir::DictionaryAttr argAttrs,
+ ::mlir::Location argLoc) {
+ return insertArguments({argIndex}, {argType}, {argAttrs}, {argLoc});
}
/// Inserts arguments with the listed types, attributes, and locations at the
/// listed indices. `argIndices` must be sorted. Arguments are inserted in the
/// order they are listed, such that arguments with identical index will
- /// appear in the same order that they were listed here.
- void insertArguments(::llvm::ArrayRef<unsigned> argIndices, ::mlir::TypeRange argTypes,
- ::llvm::ArrayRef<::mlir::DictionaryAttr> argAttrs,
- ::llvm::ArrayRef<::mlir::Location> argLocs) {
+ /// appear in the same order that they were listed here. Returns failure if
+ /// the function cannot be updated to have the new signature.
+ ::llvm::LogicalResult insertArguments(
+ ::llvm::ArrayRef<unsigned> argIndices, ::mlir::TypeRange argTypes,
+ ::llvm::ArrayRef<::mlir::DictionaryAttr> argAttrs,
+ ::llvm::ArrayRef<::mlir::Location> argLocs) {
unsigned originalNumArgs = $_op.getNumArguments();
::mlir::Type newType = $_op.getTypeWithArgsAndResults(
argIndices, argTypes, /*resultIndices=*/{}, /*resultTypes=*/{});
+ if (!newType)
+ return ::llvm::failure();
::mlir::function_interface_impl::insertFunctionArguments(
$_op, argIndices, argTypes, argAttrs, argLocs,
originalNumArgs, newType);
+ return ::llvm::success();
}
- /// Insert a single result of type `resultType` at `resultIndex`.
- void insertResult(unsigned resultIndex, ::mlir::Type resultType,
- ::mlir::DictionaryAttr resultAttrs) {
- insertResults({resultIndex}, {resultType}, {resultAttrs});
+ /// Insert a single result of type `resultType` at `resultIndex`.Returns
+ /// failure if the function cannot be updated to have the new signature.
+ ::llvm::LogicalResult insertResult(
+ unsigned resultIndex, ::mlir::Type resultType,
+ ::mlir::DictionaryAttr resultAttrs) {
+ return insertResults({resultIndex}, {resultType}, {resultAttrs});
}
/// Inserts results with the listed types at the listed indices.
/// `resultIndices` must be sorted. Results are inserted in the order they are
/// listed, such that results with identical index will appear in the same
- /// order that they were listed here.
- void insertResults(::llvm::ArrayRef<unsigned> resultIndices, ::mlir::TypeRange resultTypes,
- ::llvm::ArrayRef<::mlir::DictionaryAttr> resultAttrs) {
+ /// order that they were listed here. Returns failure if the function
+ /// cannot be updated to have the new signature.
+ ::llvm::LogicalResult insertResults(
+ ::llvm::ArrayRef<unsigned> resultIndices,
+ ::mlir::TypeRange resultTypes,
+ ::llvm::ArrayRef<::mlir::DictionaryAttr> resultAttrs) {
unsigned originalNumResults = $_op.getNumResults();
::mlir::Type newType = $_op.getTypeWithArgsAndResults(
/*argIndices=*/{}, /*argTypes=*/{}, resultIndices, resultTypes);
+ if (!newType)
+ return ::llvm::failure();
::mlir::function_interface_impl::insertFunctionResults(
$_op, resultIndices, resultTypes, resultAttrs,
originalNumResults, newType);
+ return ::llvm::success();
}
- /// Erase a single argument at `argIndex`.
- void eraseArgument(unsigned argIndex) {
+ /// Erase a single argument at `argIndex`. Returns failure if the function
+ /// cannot be updated to have the new signature.
+ ::llvm::LogicalResult eraseArgument(unsigned argIndex) {
::llvm::BitVector argsToErase($_op.getNumArguments());
argsToErase.set(argIndex);
- eraseArguments(argsToErase);
+ return eraseArguments(argsToErase);
}
- /// Erases the arguments listed in `argIndices`.
- void eraseArguments(const ::llvm::BitVector &argIndices) {
+ /// Erases the arguments listed in `argIndices`. Returns failure if the
+ /// function cannot be updated to have the new signature.
+ ::llvm::LogicalResult eraseArguments(const ::llvm::BitVector &argIndices) {
::mlir::Type newType = $_op.getTypeWithoutArgs(argIndices);
+ if (!newType)
+ return ::llvm::failure();
::mlir::function_interface_impl::eraseFunctionArguments(
$_op, argIndices, newType);
+ return ::llvm::success();
}
- /// Erase a single result at `resultIndex`.
- void eraseResult(unsigned resultIndex) {
+ /// Erase a single result at `resultIndex`. Returns failure if the function
+ /// cannot be updated to have the new signature.
+ LogicalResult eraseResult(unsigned resultIndex) {
::llvm::BitVector resultsToErase($_op.getNumResults());
resultsToErase.set(resultIndex);
- eraseResults(resultsToErase);
+ return eraseResults(resultsToErase);
}
- /// Erases the results listed in `resultIndices`.
- void eraseResults(const ::llvm::BitVector &resultIndices) {
+ /// Erases the results listed in `resultIndices`. Returns failure if the
+ /// function cannot be updated to have the new signature.
+ ::llvm::LogicalResult eraseResults(const ::llvm::BitVector &resultIndices) {
::mlir::Type newType = $_op.getTypeWithoutResults(resultIndices);
+ if (!newType)
+ return ::llvm::failure();
::mlir::function_interface_impl::eraseFunctionResults(
$_op, resultIndices, newType);
+ return ::llvm::success();
}
/// Return the type of this function with the specified arguments and
/// results inserted. This is used to update the function's signature in
/// the `insertArguments` and `insertResults` methods. The arrays must be
- /// sorted by increasing index.
+ /// sorted by increasing index. Return nullptr if the updated type would
+ /// not be valid.
::mlir::Type getTypeWithArgsAndResults(
::llvm::ArrayRef<unsigned> argIndices, ::mlir::TypeRange argTypes,
::llvm::ArrayRef<unsigned> resultIndices, ::mlir::TypeRange resultTypes) {
@@ -341,7 +367,8 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
/// Return the type of this function without the specified arguments and
/// results. This is used to update the function's signature in the
- /// `eraseArguments` and `eraseResults` methods.
+ /// `eraseArguments` and `eraseResults` methods. Return nullptr if the
+ /// updated type would not be valid.
::mlir::Type getTypeWithoutArgsAndResults(
const ::llvm::BitVector &argIndices, const ::llvm::BitVector &resultIndices) {
::llvm::SmallVector<::mlir::Type> argStorage, resultStorage;
diff --git a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
index f22ad1fd70db2..dea3d99704d9b 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
@@ -125,8 +125,12 @@ GPUFuncOpLowering::matchAndRewrite(gpu::GPUFuncOp gpuFuncOp, OpAdaptor adaptor,
// Perform signature modification
rewriter.modifyOpInPlace(
gpuFuncOp, [gpuFuncOp, &argIndices, &argTypes, &argAttrs, &argLocs]() {
- static_cast<FunctionOpInterface>(gpuFuncOp).insertArguments(
- argIndices, argTypes, argAttrs, argLocs);
+ LogicalResult inserted =
+ static_cast<FunctionOpInterface>(gpuFuncOp).insertArguments(
+ argIndices, argTypes, argAttrs, argLocs);
+ (void)inserted;
+ assert(succeeded(inserted) &&
+ "expected GPU funcs to support inserting any argument");
});
} else {
workgroupBuffers.reserve(gpuFuncOp.getNumWorkgroupAttributions());
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp b/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
index 1c95ab77b9f33..acf5f7767d12a 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
@@ -92,7 +92,8 @@ updateFuncOp(func::FuncOp func,
}
// Erase the results.
- func.eraseResults(erasedResultIndices);
+ if (failed(func.eraseResults(erasedResultIndices)))
+ return failure();
// Add the new arguments to the entry block if the function is not external.
if (func.isExternal())
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp b/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp
index 9bc75267e70e4..ae011904cb972 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp
@@ -113,7 +113,8 @@ mlir::bufferization::dropEquivalentBufferResults(ModuleOp module) {
}
// Update function.
- funcOp.eraseResults(erasedResultIndices);
+ if (failed(funcOp.eraseResults(erasedResultIndices)))
+ return failure();
returnOp.getOperandsMutable().assign(newReturnValues);
// Update function calls.
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
index 29cf38c1fefea..e2bccf7f6493e 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
@@ -232,7 +232,10 @@ LLVMFunctionType::getChecked(function_ref<InFlightDiagnostic()> emitError,
LLVMFunctionType LLVMFunctionType::clone(TypeRange inputs,
TypeRange results) const {
- assert(results.size() == 1 && "expected a single result type");
+ if (results.size() != 1 || !isValidResultType(results[0]))
+ return {};
+ if (!llvm::all_of(inputs, isValidArgumentType))
+ return {};
return get(results[0], llvm::to_vector(inputs), isVarArg());
}
diff --git a/mlir/lib/Query/Query.cpp b/mlir/lib/Query/Query.cpp
index 869ee8f2ae1dc..73f313cd37fd0 100644
--- a/mlir/lib/Query/Query.cpp
+++ b/mlir/lib/Query/Query.cpp
@@ -88,10 +88,11 @@ static Operation *extractFunction(std::vector<Operation *> &ops,
// Remove unused function arguments
size_t currentIndex = 0;
while (currentIndex < funcOp.getNumArguments()) {
+ // Erase if possible.
if (funcOp.getArgument(currentIndex).use_empty())
- funcOp.eraseArgument(currentIndex);
- else
- ++currentIndex;
+ if (succeeded(funcOp.eraseArgument(currentIndex)))
+ continue;
+ ++currentIndex;
}
return funcOp;
diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index 92c05d87a002d..4939dda4a20f9 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -698,8 +698,11 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) {
// 3. Functions
for (auto &f : list.functions) {
- f.funcOp.eraseArguments(f.nonLiveArgs);
- f.funcOp.eraseResults(f.nonLiveRets);
+ // Some functions may not allow erasing arguments or results. These calls
+ // return failure in such cases without modifying the function, so it's okay
+ // to proceed.
+ (void)f.funcOp.eraseArguments(f.nonLiveArgs);
+ (void)f.funcOp.eraseResults(f.nonLiveRets);
}
// 4. Operands
diff --git a/mlir/test/IR/test-func-erase-result.mlir b/mlir/test/IR/test-func-erase-result.mlir
index 8fe40b2fdfcbc..a32866227547b 100644
--- a/mlir/test/IR/test-func-erase-result.mlir
+++ b/mlir/test/IR/test-func-erase-result.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s -test-func-erase-result -split-input-file | FileCheck %s
+// RUN: mlir-opt %s -test-func-erase-result -split-input-file -verify-diagnostics | FileCheck %s
// CHECK: func private @f(){{$}}
// CHECK-NOT: attributes{{.*}}result
@@ -66,3 +66,8 @@ func.func private @f() -> (
f32 {test.erase_this_result},
tensor<3xf32>
)
+
+// -----
+
+// expected-error @below {{failed to erase results}}
+llvm.func @llvm_func(!llvm.ptr, i64)
diff --git a/mlir/test/lib/IR/TestFunc.cpp b/mlir/test/lib/IR/TestFunc.cpp
index 2ade47249863c..94a4610365863 100644
--- a/mlir/test/lib/IR/TestFunc.cpp
+++ b/mlir/test/lib/IR/TestFunc.cpp
@@ -45,8 +45,12 @@ struct TestFuncInsertArg
: unknownLoc);
}
func->removeAttr("test.insert_args");
- func.insertArguments(indicesToInsert, typesToInsert, attrsToInsert,
- locsToInsert);
+ if (succeeded(func.insertArguments(indicesToInsert, typesToInsert,
+ attrsToInsert, locsToInsert)))
+ continue;
+
+ emitError(func->getLoc()) << "failed to insert arguments";
+ return signalPassFailure();
}
}
};
@@ -79,7 +83,12 @@ struct TestFuncInsertResult
: DictionaryAttr::get(&getContext()));
}
func->removeAttr("test.insert_results");
- func.insertResults(indicesToInsert, typesToInsert, attrsToInsert);
+ if (succeeded(func.insertResults(indicesToInsert, typesToInsert,
+ attrsToInsert)))
+ continue;
+
+ emitError(func->getLoc()) << "failed to insert results";
+ return signalPassFailure();
}
}
};
@@ -100,7 +109,10 @@ struct TestFuncEraseArg
for (auto argIndex : llvm::seq<int>(0, func.getNumArguments()))
if (func.getArgAttr(argIndex, "test.erase_this_arg"))
indicesToErase.set(argIndex);
- func.eraseArguments(indicesToErase);
+ if (succeeded(func.eraseArguments(indicesToErase)))
+ continue;
+ emitError(func->getLoc()) << "failed to erase arguments";
+ return signalPassFailure();
}
}
};
@@ -122,7 +134,10 @@ struct TestFuncEraseResult
for (auto resultIndex : llvm::seq<int>(0, func.getNumResults()))
if (func.getResultAttr(resultIndex, "test.erase_this_result"))
indicesToErase.set(resultIndex);
- func.eraseResults(indicesToErase);
+ if (succeeded(func.eraseResults(indicesToErase)))
+ continue;
+ emitError(func->getLoc()) << "failed to erase results";
+ return signalPassFailure();
}
}
};
|
|
@llvm/pr-subscribers-mlir-bufferization Author: Oleksandr "Alex" Zinenko (ftynse) Changes
Allow function type cloning to fail and propagate this failure through various APIs that use it. The common assumption is that existing IR has not been modified. Fixes #131142. Reland a8c7ecd / #136300. Full diff: https://github.com/llvm/llvm-project/pull/137130.diff 11 Files Affected:
diff --git a/flang/lib/Optimizer/Transforms/AbstractResult.cpp b/flang/lib/Optimizer/Transforms/AbstractResult.cpp
index f8badfa639f94..59e2eeb76c715 100644
--- a/flang/lib/Optimizer/Transforms/AbstractResult.cpp
+++ b/flang/lib/Optimizer/Transforms/AbstractResult.cpp
@@ -387,8 +387,12 @@ class AbstractResultOpt
mlir::OpBuilder rewriter(context);
auto resultType = funcTy.getResult(0);
auto argTy = getResultArgumentType(resultType, shouldBoxResult);
- func.insertArgument(0u, argTy, {}, loc);
- func.eraseResult(0u);
+ llvm::LogicalResult res = func.insertArgument(0u, argTy, {}, loc);
+ (void)res;
+ assert(llvm::succeeded(res) && "failed to insert function argument");
+ res = func.eraseResult(0u);
+ (void)res;
+ assert(llvm::succeeded(res) && "failed to erase function result");
mlir::Value newArg = func.getArgument(0u);
if (mustEmboxResult(resultType, shouldBoxResult)) {
auto bufferType = fir::ReferenceType::get(resultType);
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
index e9f01ba5cd4e4..072fc7be07dd4 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
@@ -104,7 +104,8 @@ def LLVMFunctionType : LLVMType<"LLVMFunction", "func"> {
bool isVarArg() const { return getVarArg(); }
/// Returns a clone of this function type with the given argument
- /// and result types.
+ /// and result types. Returns null if the resulting function type would
+ /// not verify.
LLVMFunctionType clone(TypeRange inputs, TypeRange results) const;
/// Returns the result type of the function as an ArrayRef, enabling better
diff --git a/mlir/include/mlir/Interfaces/FunctionInterfaces.td b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
index 1e2ca9a8b4490..cad0e790fdd97 100644
--- a/mlir/include/mlir/Interfaces/FunctionInterfaces.td
+++ b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
@@ -255,79 +255,105 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
BlockArgListType getArguments() { return getFunctionBody().getArguments(); }
/// Insert a single argument of type `argType` with attributes `argAttrs` and
- /// location `argLoc` at `argIndex`.
- void insertArgument(unsigned argIndex, ::mlir::Type argType, ::mlir::DictionaryAttr argAttrs,
- ::mlir::Location argLoc) {
- insertArguments({argIndex}, {argType}, {argAttrs}, {argLoc});
+ /// location `argLoc` at `argIndex`. Returns failure if the function cannot be
+ /// updated to have the new signature.
+ ::llvm::LogicalResult insertArgument(
+ unsigned argIndex, ::mlir::Type argType, ::mlir::DictionaryAttr argAttrs,
+ ::mlir::Location argLoc) {
+ return insertArguments({argIndex}, {argType}, {argAttrs}, {argLoc});
}
/// Inserts arguments with the listed types, attributes, and locations at the
/// listed indices. `argIndices` must be sorted. Arguments are inserted in the
/// order they are listed, such that arguments with identical index will
- /// appear in the same order that they were listed here.
- void insertArguments(::llvm::ArrayRef<unsigned> argIndices, ::mlir::TypeRange argTypes,
- ::llvm::ArrayRef<::mlir::DictionaryAttr> argAttrs,
- ::llvm::ArrayRef<::mlir::Location> argLocs) {
+ /// appear in the same order that they were listed here. Returns failure if
+ /// the function cannot be updated to have the new signature.
+ ::llvm::LogicalResult insertArguments(
+ ::llvm::ArrayRef<unsigned> argIndices, ::mlir::TypeRange argTypes,
+ ::llvm::ArrayRef<::mlir::DictionaryAttr> argAttrs,
+ ::llvm::ArrayRef<::mlir::Location> argLocs) {
unsigned originalNumArgs = $_op.getNumArguments();
::mlir::Type newType = $_op.getTypeWithArgsAndResults(
argIndices, argTypes, /*resultIndices=*/{}, /*resultTypes=*/{});
+ if (!newType)
+ return ::llvm::failure();
::mlir::function_interface_impl::insertFunctionArguments(
$_op, argIndices, argTypes, argAttrs, argLocs,
originalNumArgs, newType);
+ return ::llvm::success();
}
- /// Insert a single result of type `resultType` at `resultIndex`.
- void insertResult(unsigned resultIndex, ::mlir::Type resultType,
- ::mlir::DictionaryAttr resultAttrs) {
- insertResults({resultIndex}, {resultType}, {resultAttrs});
+ /// Insert a single result of type `resultType` at `resultIndex`.Returns
+ /// failure if the function cannot be updated to have the new signature.
+ ::llvm::LogicalResult insertResult(
+ unsigned resultIndex, ::mlir::Type resultType,
+ ::mlir::DictionaryAttr resultAttrs) {
+ return insertResults({resultIndex}, {resultType}, {resultAttrs});
}
/// Inserts results with the listed types at the listed indices.
/// `resultIndices` must be sorted. Results are inserted in the order they are
/// listed, such that results with identical index will appear in the same
- /// order that they were listed here.
- void insertResults(::llvm::ArrayRef<unsigned> resultIndices, ::mlir::TypeRange resultTypes,
- ::llvm::ArrayRef<::mlir::DictionaryAttr> resultAttrs) {
+ /// order that they were listed here. Returns failure if the function
+ /// cannot be updated to have the new signature.
+ ::llvm::LogicalResult insertResults(
+ ::llvm::ArrayRef<unsigned> resultIndices,
+ ::mlir::TypeRange resultTypes,
+ ::llvm::ArrayRef<::mlir::DictionaryAttr> resultAttrs) {
unsigned originalNumResults = $_op.getNumResults();
::mlir::Type newType = $_op.getTypeWithArgsAndResults(
/*argIndices=*/{}, /*argTypes=*/{}, resultIndices, resultTypes);
+ if (!newType)
+ return ::llvm::failure();
::mlir::function_interface_impl::insertFunctionResults(
$_op, resultIndices, resultTypes, resultAttrs,
originalNumResults, newType);
+ return ::llvm::success();
}
- /// Erase a single argument at `argIndex`.
- void eraseArgument(unsigned argIndex) {
+ /// Erase a single argument at `argIndex`. Returns failure if the function
+ /// cannot be updated to have the new signature.
+ ::llvm::LogicalResult eraseArgument(unsigned argIndex) {
::llvm::BitVector argsToErase($_op.getNumArguments());
argsToErase.set(argIndex);
- eraseArguments(argsToErase);
+ return eraseArguments(argsToErase);
}
- /// Erases the arguments listed in `argIndices`.
- void eraseArguments(const ::llvm::BitVector &argIndices) {
+ /// Erases the arguments listed in `argIndices`. Returns failure if the
+ /// function cannot be updated to have the new signature.
+ ::llvm::LogicalResult eraseArguments(const ::llvm::BitVector &argIndices) {
::mlir::Type newType = $_op.getTypeWithoutArgs(argIndices);
+ if (!newType)
+ return ::llvm::failure();
::mlir::function_interface_impl::eraseFunctionArguments(
$_op, argIndices, newType);
+ return ::llvm::success();
}
- /// Erase a single result at `resultIndex`.
- void eraseResult(unsigned resultIndex) {
+ /// Erase a single result at `resultIndex`. Returns failure if the function
+ /// cannot be updated to have the new signature.
+ LogicalResult eraseResult(unsigned resultIndex) {
::llvm::BitVector resultsToErase($_op.getNumResults());
resultsToErase.set(resultIndex);
- eraseResults(resultsToErase);
+ return eraseResults(resultsToErase);
}
- /// Erases the results listed in `resultIndices`.
- void eraseResults(const ::llvm::BitVector &resultIndices) {
+ /// Erases the results listed in `resultIndices`. Returns failure if the
+ /// function cannot be updated to have the new signature.
+ ::llvm::LogicalResult eraseResults(const ::llvm::BitVector &resultIndices) {
::mlir::Type newType = $_op.getTypeWithoutResults(resultIndices);
+ if (!newType)
+ return ::llvm::failure();
::mlir::function_interface_impl::eraseFunctionResults(
$_op, resultIndices, newType);
+ return ::llvm::success();
}
/// Return the type of this function with the specified arguments and
/// results inserted. This is used to update the function's signature in
/// the `insertArguments` and `insertResults` methods. The arrays must be
- /// sorted by increasing index.
+ /// sorted by increasing index. Return nullptr if the updated type would
+ /// not be valid.
::mlir::Type getTypeWithArgsAndResults(
::llvm::ArrayRef<unsigned> argIndices, ::mlir::TypeRange argTypes,
::llvm::ArrayRef<unsigned> resultIndices, ::mlir::TypeRange resultTypes) {
@@ -341,7 +367,8 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
/// Return the type of this function without the specified arguments and
/// results. This is used to update the function's signature in the
- /// `eraseArguments` and `eraseResults` methods.
+ /// `eraseArguments` and `eraseResults` methods. Return nullptr if the
+ /// updated type would not be valid.
::mlir::Type getTypeWithoutArgsAndResults(
const ::llvm::BitVector &argIndices, const ::llvm::BitVector &resultIndices) {
::llvm::SmallVector<::mlir::Type> argStorage, resultStorage;
diff --git a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
index f22ad1fd70db2..dea3d99704d9b 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
@@ -125,8 +125,12 @@ GPUFuncOpLowering::matchAndRewrite(gpu::GPUFuncOp gpuFuncOp, OpAdaptor adaptor,
// Perform signature modification
rewriter.modifyOpInPlace(
gpuFuncOp, [gpuFuncOp, &argIndices, &argTypes, &argAttrs, &argLocs]() {
- static_cast<FunctionOpInterface>(gpuFuncOp).insertArguments(
- argIndices, argTypes, argAttrs, argLocs);
+ LogicalResult inserted =
+ static_cast<FunctionOpInterface>(gpuFuncOp).insertArguments(
+ argIndices, argTypes, argAttrs, argLocs);
+ (void)inserted;
+ assert(succeeded(inserted) &&
+ "expected GPU funcs to support inserting any argument");
});
} else {
workgroupBuffers.reserve(gpuFuncOp.getNumWorkgroupAttributions());
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp b/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
index 1c95ab77b9f33..acf5f7767d12a 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
@@ -92,7 +92,8 @@ updateFuncOp(func::FuncOp func,
}
// Erase the results.
- func.eraseResults(erasedResultIndices);
+ if (failed(func.eraseResults(erasedResultIndices)))
+ return failure();
// Add the new arguments to the entry block if the function is not external.
if (func.isExternal())
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp b/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp
index 9bc75267e70e4..ae011904cb972 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp
@@ -113,7 +113,8 @@ mlir::bufferization::dropEquivalentBufferResults(ModuleOp module) {
}
// Update function.
- funcOp.eraseResults(erasedResultIndices);
+ if (failed(funcOp.eraseResults(erasedResultIndices)))
+ return failure();
returnOp.getOperandsMutable().assign(newReturnValues);
// Update function calls.
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
index 29cf38c1fefea..e2bccf7f6493e 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
@@ -232,7 +232,10 @@ LLVMFunctionType::getChecked(function_ref<InFlightDiagnostic()> emitError,
LLVMFunctionType LLVMFunctionType::clone(TypeRange inputs,
TypeRange results) const {
- assert(results.size() == 1 && "expected a single result type");
+ if (results.size() != 1 || !isValidResultType(results[0]))
+ return {};
+ if (!llvm::all_of(inputs, isValidArgumentType))
+ return {};
return get(results[0], llvm::to_vector(inputs), isVarArg());
}
diff --git a/mlir/lib/Query/Query.cpp b/mlir/lib/Query/Query.cpp
index 869ee8f2ae1dc..73f313cd37fd0 100644
--- a/mlir/lib/Query/Query.cpp
+++ b/mlir/lib/Query/Query.cpp
@@ -88,10 +88,11 @@ static Operation *extractFunction(std::vector<Operation *> &ops,
// Remove unused function arguments
size_t currentIndex = 0;
while (currentIndex < funcOp.getNumArguments()) {
+ // Erase if possible.
if (funcOp.getArgument(currentIndex).use_empty())
- funcOp.eraseArgument(currentIndex);
- else
- ++currentIndex;
+ if (succeeded(funcOp.eraseArgument(currentIndex)))
+ continue;
+ ++currentIndex;
}
return funcOp;
diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index 92c05d87a002d..4939dda4a20f9 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -698,8 +698,11 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) {
// 3. Functions
for (auto &f : list.functions) {
- f.funcOp.eraseArguments(f.nonLiveArgs);
- f.funcOp.eraseResults(f.nonLiveRets);
+ // Some functions may not allow erasing arguments or results. These calls
+ // return failure in such cases without modifying the function, so it's okay
+ // to proceed.
+ (void)f.funcOp.eraseArguments(f.nonLiveArgs);
+ (void)f.funcOp.eraseResults(f.nonLiveRets);
}
// 4. Operands
diff --git a/mlir/test/IR/test-func-erase-result.mlir b/mlir/test/IR/test-func-erase-result.mlir
index 8fe40b2fdfcbc..a32866227547b 100644
--- a/mlir/test/IR/test-func-erase-result.mlir
+++ b/mlir/test/IR/test-func-erase-result.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s -test-func-erase-result -split-input-file | FileCheck %s
+// RUN: mlir-opt %s -test-func-erase-result -split-input-file -verify-diagnostics | FileCheck %s
// CHECK: func private @f(){{$}}
// CHECK-NOT: attributes{{.*}}result
@@ -66,3 +66,8 @@ func.func private @f() -> (
f32 {test.erase_this_result},
tensor<3xf32>
)
+
+// -----
+
+// expected-error @below {{failed to erase results}}
+llvm.func @llvm_func(!llvm.ptr, i64)
diff --git a/mlir/test/lib/IR/TestFunc.cpp b/mlir/test/lib/IR/TestFunc.cpp
index 2ade47249863c..94a4610365863 100644
--- a/mlir/test/lib/IR/TestFunc.cpp
+++ b/mlir/test/lib/IR/TestFunc.cpp
@@ -45,8 +45,12 @@ struct TestFuncInsertArg
: unknownLoc);
}
func->removeAttr("test.insert_args");
- func.insertArguments(indicesToInsert, typesToInsert, attrsToInsert,
- locsToInsert);
+ if (succeeded(func.insertArguments(indicesToInsert, typesToInsert,
+ attrsToInsert, locsToInsert)))
+ continue;
+
+ emitError(func->getLoc()) << "failed to insert arguments";
+ return signalPassFailure();
}
}
};
@@ -79,7 +83,12 @@ struct TestFuncInsertResult
: DictionaryAttr::get(&getContext()));
}
func->removeAttr("test.insert_results");
- func.insertResults(indicesToInsert, typesToInsert, attrsToInsert);
+ if (succeeded(func.insertResults(indicesToInsert, typesToInsert,
+ attrsToInsert)))
+ continue;
+
+ emitError(func->getLoc()) << "failed to insert results";
+ return signalPassFailure();
}
}
};
@@ -100,7 +109,10 @@ struct TestFuncEraseArg
for (auto argIndex : llvm::seq<int>(0, func.getNumArguments()))
if (func.getArgAttr(argIndex, "test.erase_this_arg"))
indicesToErase.set(argIndex);
- func.eraseArguments(indicesToErase);
+ if (succeeded(func.eraseArguments(indicesToErase)))
+ continue;
+ emitError(func->getLoc()) << "failed to erase arguments";
+ return signalPassFailure();
}
}
};
@@ -122,7 +134,10 @@ struct TestFuncEraseResult
for (auto resultIndex : llvm::seq<int>(0, func.getNumResults()))
if (func.getResultAttr(resultIndex, "test.erase_this_result"))
indicesToErase.set(resultIndex);
- func.eraseResults(indicesToErase);
+ if (succeeded(func.eraseResults(indicesToErase)))
+ continue;
+ emitError(func->getLoc()) << "failed to erase results";
+ return signalPassFailure();
}
}
};
|
j2kun
approved these changes
Apr 24, 2025
joker-eph
reviewed
Apr 24, 2025
| if (results.size() != 1 || !isValidResultType(results[0])) | ||
| return {}; | ||
| if (!llvm::all_of(inputs, isValidArgumentType)) | ||
| return {}; | ||
| return get(results[0], llvm::to_vector(inputs), isVarArg()); |
There was a problem hiding this comment.
Can't we just use getChecked here?
That would match the doc of the API: "Returns null if the resulting function type would not verify."
There was a problem hiding this comment.
It emits a diagnostic that we don't want here. And we don't have an equivalent of llvm::nulls for diagnostics were we could just send it to the void. Overriding the handler locally sounds a bit too heavy here...
IanWood1
added a commit
to iree-org/llvm-project
that referenced
this pull request
May 5, 2025
This reverts commit 7318074.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
`FunctionOpInterface` assumed the fact that the function type (attribute of the operation) can be cloned with arbirary lists of function arguments and results to support argument and result list mutation. This is not always correct, in particular, LLVM dialect functions require exactly one result making it impossible to erase the result. Allow function type cloning to fail and propagate this failure through various APIs that use it. The common assumption is that existing IR has not been modified. Fixes llvm#131142. Reland a8c7ecd / llvm#136300.
GeorgeARM
pushed a commit
to GeorgeARM/llvm-project
that referenced
this pull request
May 7, 2025
`FunctionOpInterface` assumed the fact that the function type (attribute of the operation) can be cloned with arbirary lists of function arguments and results to support argument and result list mutation. This is not always correct, in particular, LLVM dialect functions require exactly one result making it impossible to erase the result. Allow function type cloning to fail and propagate this failure through various APIs that use it. The common assumption is that existing IR has not been modified. Fixes llvm#131142. Reland a8c7ecd / llvm#136300.
MaheshRavishankar
pushed a commit
to iree-org/llvm-project
that referenced
this pull request
May 8, 2025
This reverts commit 7318074.
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
`FunctionOpInterface` assumed the fact that the function type (attribute of the operation) can be cloned with arbirary lists of function arguments and results to support argument and result list mutation. This is not always correct, in particular, LLVM dialect functions require exactly one result making it impossible to erase the result. Allow function type cloning to fail and propagate this failure through various APIs that use it. The common assumption is that existing IR has not been modified. Fixes llvm#131142. Reland a8c7ecd / llvm#136300.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 14, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 14, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 14, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 14, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 14, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 14, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 14, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 14, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 14, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 15, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to Muzammiluddin-Syed-ECE/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to iree-org/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
Muzammiluddin-Syed-ECE
pushed a commit
to iree-org/llvm-project
that referenced
this pull request
May 16, 2025
This reverts commit 7318074.
paul0403
added a commit
to PennyLaneAI/catalyst
that referenced
this pull request
Jul 28, 2025
**Context:** Update llvm, mhlo and enzyme, 2025 Q3. The latest pair of good versions, indicated by mhlo, is tensorflow/mlir-hlo@1dd2e71 ``` mhlo=1dd2e71331014ae0373f6bf900ce6be393357190 llvm=f8cb7987c64dcffb72414a40560055cb717dbf74 ``` For Enzyme, we go to the latest release https://github.com/EnzymeAD/Enzyme/releases/tag/v0.0.186 ``` enzyme=v0.0.186 ``` with commit `8c1a596158f6194f10e8ffd56a1660a61c54337e` **Description of the Change:** Miscellaneous: 1. `GreedyRewriteConfig.stuff = blah` -> `GreedyRewriteConfig.setStuff(blah)` llvm/llvm-project#137122 2. llvm gep op `inbounds` attribute is subsumed under a gep sign wrap enum flag llvm/llvm-project#137272 3. `arith::Constant[Int, Float]Op` builders now have the same argument order as other ops (output type first, then arguments) llvm/llvm-project#144636 (note that Enzyme also noticed this EnzymeAD/Enzyme#2379 😆 ) 4. The `lookupOrCreateFn` functions now take in a builder instead of instantiating a new one llvm/llvm-project#136421 5. `getStridedElementPtr` now takes in `rewriter` as the first argument (instead of the last), like all the other utils llvm/llvm-project#138984 6. The following functions now return a `LogicalResult`, and will be caught by warnings as errors as `-Wunused-result`: - `func::FuncOp.[insert, erase]Argument(s)` llvm/llvm-project#137130 - `getBackwardSlice()` llvm/llvm-project#140961 Things related to `transform.apply_registered_pass` op: 1. It now takes in a `dynamic_options` llvm/llvm-project#142683. We don't need to use this as all our pass options are static. 2. The options it takes in are now dictionaries instead of strings llvm/llvm-project#143159 Bufferization: 1. `bufferization.to_memref` op is renamed to `bufferization.to_buffer` llvm/llvm-project#137180 3. `bufferization.to_tensor` op's builder now needs the result type to be explicit llvm/llvm-project#142986. This is also needed by a patched mhlo pass. 4. The `getBuffer()` methods take in a new arg for `BufferizationState` llvm/llvm-project#141019, llvm/llvm-project#141466 5. `UnknownTypeConverterFn` in bufferization options now takes in just a type instead of a full value llvm/llvm-project#144658 **Related GitHub Issues:** [sc-95176] [sc-95664] --------- Co-authored-by: Mehrdad Malek <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
FunctionOpInterfaceassumed the fact that the function type (attribute of the operation) can be cloned with arbirary lists of function arguments and results to support argument and result list mutation. This is not always correct, in particular, LLVM dialect functions require exactly one result making it impossible to erase the result.Allow function type cloning to fail and propagate this failure through various APIs that use it. The common assumption is that existing IR has not been modified.
Fixes #131142.
Reland a8c7ecd / #136300.