[Mlir-commits] [mlir] 7da385d - Revert "[mlir] allow function type cloning to fail (#136300)"
Kazu Hirata
llvmlistbot at llvm.org
Fri Apr 18 09:52:34 PDT 2025
Author: Kazu Hirata
Date: 2025-04-18T09:52:28-07:00
New Revision: 7da385d79729bb7aa1bfc046d3a78e350b7a4f75
URL: https://github.com/llvm/llvm-project/commit/7da385d79729bb7aa1bfc046d3a78e350b7a4f75
DIFF: https://github.com/llvm/llvm-project/commit/7da385d79729bb7aa1bfc046d3a78e350b7a4f75.diff
LOG: Revert "[mlir] allow function type cloning to fail (#136300)"
This reverts commit 20a104a7d6423784dab04371a5ca728cc27a15a9.
Buildbot failure:
https://lab.llvm.org/buildbot/#/builders/157/builds/25688
I've reproduced the build failure.
Added:
Modified:
mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
mlir/include/mlir/Interfaces/FunctionInterfaces.td
mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp
mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
mlir/lib/Query/Query.cpp
mlir/lib/Transforms/RemoveDeadValues.cpp
mlir/test/IR/test-func-erase-result.mlir
mlir/test/lib/IR/TestFunc.cpp
Removed:
################################################################################
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
index 072fc7be07dd4..e9f01ba5cd4e4 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMTypes.td
@@ -104,8 +104,7 @@ def LLVMFunctionType : LLVMType<"LLVMFunction", "func"> {
bool isVarArg() const { return getVarArg(); }
/// Returns a clone of this function type with the given argument
- /// and result types. Returns null if the resulting function type would
- /// not verify.
+ /// and result types.
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 cad0e790fdd97..1e2ca9a8b4490 100644
--- a/mlir/include/mlir/Interfaces/FunctionInterfaces.td
+++ b/mlir/include/mlir/Interfaces/FunctionInterfaces.td
@@ -255,105 +255,79 @@ def FunctionOpInterface : OpInterface<"FunctionOpInterface", [
BlockArgListType getArguments() { return getFunctionBody().getArguments(); }
/// Insert a single argument of type `argType` with attributes `argAttrs` and
- /// 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});
+ /// location `argLoc` at `argIndex`.
+ void insertArgument(unsigned argIndex, ::mlir::Type argType, ::mlir::DictionaryAttr argAttrs,
+ ::mlir::Location argLoc) {
+ 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. 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) {
+ /// 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) {
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`.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});
+ /// Insert a single result of type `resultType` at `resultIndex`.
+ void insertResult(unsigned resultIndex, ::mlir::Type resultType,
+ ::mlir::DictionaryAttr resultAttrs) {
+ 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. 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) {
+ /// order that they were listed here.
+ void 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`. Returns failure if the function
- /// cannot be updated to have the new signature.
- ::llvm::LogicalResult eraseArgument(unsigned argIndex) {
+ /// Erase a single argument at `argIndex`.
+ void eraseArgument(unsigned argIndex) {
::llvm::BitVector argsToErase($_op.getNumArguments());
argsToErase.set(argIndex);
- return eraseArguments(argsToErase);
+ eraseArguments(argsToErase);
}
- /// 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) {
+ /// Erases the arguments listed in `argIndices`.
+ void 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`. Returns failure if the function
- /// cannot be updated to have the new signature.
- LogicalResult eraseResult(unsigned resultIndex) {
+ /// Erase a single result at `resultIndex`.
+ void eraseResult(unsigned resultIndex) {
::llvm::BitVector resultsToErase($_op.getNumResults());
resultsToErase.set(resultIndex);
- return eraseResults(resultsToErase);
+ eraseResults(resultsToErase);
}
- /// 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) {
+ /// Erases the results listed in `resultIndices`.
+ void 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. Return nullptr if the updated type would
- /// not be valid.
+ /// sorted by increasing index.
::mlir::Type getTypeWithArgsAndResults(
::llvm::ArrayRef<unsigned> argIndices, ::mlir::TypeRange argTypes,
::llvm::ArrayRef<unsigned> resultIndices, ::mlir::TypeRange resultTypes) {
@@ -367,8 +341,7 @@ 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. Return nullptr if the
- /// updated type would not be valid.
+ /// `eraseArguments` and `eraseResults` methods.
::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 dea3d99704d9b..f22ad1fd70db2 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
@@ -125,12 +125,8 @@ GPUFuncOpLowering::matchAndRewrite(gpu::GPUFuncOp gpuFuncOp, OpAdaptor adaptor,
// Perform signature modification
rewriter.modifyOpInPlace(
gpuFuncOp, [gpuFuncOp, &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");
+ static_cast<FunctionOpInterface>(gpuFuncOp).insertArguments(
+ argIndices, argTypes, argAttrs, argLocs);
});
} else {
workgroupBuffers.reserve(gpuFuncOp.getNumWorkgroupAttributions());
diff --git a/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp b/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
index acf5f7767d12a..1c95ab77b9f33 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/BufferResultsToOutParams.cpp
@@ -92,8 +92,7 @@ updateFuncOp(func::FuncOp func,
}
// Erase the results.
- if (failed(func.eraseResults(erasedResultIndices)))
- return failure();
+ func.eraseResults(erasedResultIndices);
// 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 ae011904cb972..9bc75267e70e4 100644
--- a/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp
+++ b/mlir/lib/Dialect/Bufferization/Transforms/DropEquivalentBufferResults.cpp
@@ -113,8 +113,7 @@ mlir::bufferization::dropEquivalentBufferResults(ModuleOp module) {
}
// Update function.
- if (failed(funcOp.eraseResults(erasedResultIndices)))
- return failure();
+ funcOp.eraseResults(erasedResultIndices);
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 e2bccf7f6493e..29cf38c1fefea 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMTypes.cpp
@@ -232,10 +232,7 @@ LLVMFunctionType::getChecked(function_ref<InFlightDiagnostic()> emitError,
LLVMFunctionType LLVMFunctionType::clone(TypeRange inputs,
TypeRange results) const {
- if (results.size() != 1 || !isValidResultType(results[0]))
- return {};
- if (!llvm::all_of(inputs, isValidArgumentType))
- return {};
+ assert(results.size() == 1 && "expected a single result type");
return get(results[0], llvm::to_vector(inputs), isVarArg());
}
diff --git a/mlir/lib/Query/Query.cpp b/mlir/lib/Query/Query.cpp
index 73f313cd37fd0..869ee8f2ae1dc 100644
--- a/mlir/lib/Query/Query.cpp
+++ b/mlir/lib/Query/Query.cpp
@@ -88,11 +88,10 @@ 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())
- if (succeeded(funcOp.eraseArgument(currentIndex)))
- continue;
- ++currentIndex;
+ funcOp.eraseArgument(currentIndex);
+ else
+ ++currentIndex;
}
return funcOp;
diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index 4939dda4a20f9..92c05d87a002d 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -698,11 +698,8 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) {
// 3. Functions
for (auto &f : list.functions) {
- // 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);
+ f.funcOp.eraseArguments(f.nonLiveArgs);
+ 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 a32866227547b..8fe40b2fdfcbc 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 -verify-diagnostics | FileCheck %s
+// RUN: mlir-opt %s -test-func-erase-result -split-input-file | FileCheck %s
// CHECK: func private @f(){{$}}
// CHECK-NOT: attributes{{.*}}result
@@ -66,8 +66,3 @@ 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 94a4610365863..2ade47249863c 100644
--- a/mlir/test/lib/IR/TestFunc.cpp
+++ b/mlir/test/lib/IR/TestFunc.cpp
@@ -45,12 +45,8 @@ struct TestFuncInsertArg
: unknownLoc);
}
func->removeAttr("test.insert_args");
- if (succeeded(func.insertArguments(indicesToInsert, typesToInsert,
- attrsToInsert, locsToInsert)))
- continue;
-
- emitError(func->getLoc()) << "failed to insert arguments";
- return signalPassFailure();
+ func.insertArguments(indicesToInsert, typesToInsert, attrsToInsert,
+ locsToInsert);
}
}
};
@@ -83,12 +79,7 @@ struct TestFuncInsertResult
: DictionaryAttr::get(&getContext()));
}
func->removeAttr("test.insert_results");
- if (succeeded(func.insertResults(indicesToInsert, typesToInsert,
- attrsToInsert)))
- continue;
-
- emitError(func->getLoc()) << "failed to insert results";
- return signalPassFailure();
+ func.insertResults(indicesToInsert, typesToInsert, attrsToInsert);
}
}
};
@@ -109,10 +100,7 @@ struct TestFuncEraseArg
for (auto argIndex : llvm::seq<int>(0, func.getNumArguments()))
if (func.getArgAttr(argIndex, "test.erase_this_arg"))
indicesToErase.set(argIndex);
- if (succeeded(func.eraseArguments(indicesToErase)))
- continue;
- emitError(func->getLoc()) << "failed to erase arguments";
- return signalPassFailure();
+ func.eraseArguments(indicesToErase);
}
}
};
@@ -134,10 +122,7 @@ struct TestFuncEraseResult
for (auto resultIndex : llvm::seq<int>(0, func.getNumResults()))
if (func.getResultAttr(resultIndex, "test.erase_this_result"))
indicesToErase.set(resultIndex);
- if (succeeded(func.eraseResults(indicesToErase)))
- continue;
- emitError(func->getLoc()) << "failed to erase results";
- return signalPassFailure();
+ func.eraseResults(indicesToErase);
}
}
};
More information about the Mlir-commits
mailing list