[mlir] Fix FunctionOpInterface impl for external func#124693
Merged
ZenithalHourlyRate merged 1 commit intollvm:mainfrom Feb 19, 2025
Merged
Conversation
Member
|
@llvm/pr-subscribers-mlir Author: Hongren Zheng (ZenithalHourlyRate) ChangesFor function declarations (i.e. func op has no entry block), the FunctionOpInterface method An example can be seen in google/heir#1324 The segfault trace Full diff: https://github.com/llvm/llvm-project/pull/124693.diff 1 Files Affected:
diff --git a/mlir/lib/Interfaces/FunctionInterfaces.cpp b/mlir/lib/Interfaces/FunctionInterfaces.cpp
index 80f47a3f836768..1d565eac2aeb88 100644
--- a/mlir/lib/Interfaces/FunctionInterfaces.cpp
+++ b/mlir/lib/Interfaces/FunctionInterfaces.cpp
@@ -199,7 +199,7 @@ void function_interface_impl::insertFunctionArguments(
// There are 3 things that need to be updated:
// - Function type.
// - Arg attrs.
- // - Block arguments of entry block.
+ // - Block arguments of entry block, if not empty.
Block &entry = op->getRegion(0).front();
// Update the argument attributes of the function.
@@ -228,8 +228,11 @@ void function_interface_impl::insertFunctionArguments(
// Update the function type and any entry block arguments.
op.setFunctionTypeAttr(TypeAttr::get(newType));
- for (unsigned i = 0, e = argIndices.size(); i < e; ++i)
- entry.insertArgument(argIndices[i] + i, argTypes[i], argLocs[i]);
+
+ // Update the block arguments of the entry block when it is not external.
+ if (!op.isExternal())
+ for (unsigned i = 0, e = argIndices.size(); i < e; ++i)
+ entry.insertArgument(argIndices[i] + i, argTypes[i], argLocs[i]);
}
void function_interface_impl::insertFunctionResults(
@@ -279,7 +282,7 @@ void function_interface_impl::eraseFunctionArguments(
// There are 3 things that need to be updated:
// - Function type.
// - Arg attrs.
- // - Block arguments of entry block.
+ // - Block arguments of entry block, if not empty.
Block &entry = op->getRegion(0).front();
// Update the argument attributes of the function.
@@ -294,7 +297,10 @@ void function_interface_impl::eraseFunctionArguments(
// Update the function type and any entry block arguments.
op.setFunctionTypeAttr(TypeAttr::get(newType));
- entry.eraseArguments(argIndices);
+
+ // Update the block arguments of the entry block when it is not external.
+ if (!op.isExternal())
+ entry.eraseArguments(argIndices);
}
void function_interface_impl::eraseFunctionResults(
|
Member
Author
|
Cc @fabianschuiki, @River707 for review |
Member
Author
|
Ping for review |
River707
requested changes
Feb 3, 2025
ZenithalHourlyRate
force-pushed
the
mlir-function-interface-declaration
branch
from
2fa0334 to
77c9b37
Compare
Member
Author
|
Comments addressed and tested insertArgument for op declaration in downstream project. |
Member
Author
|
Ping for review |
1 similar comment
Member
Author
|
Ping for review |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
For function declarations (i.e. func op has no entry block), the FunctionOpInterface method
insertArgumentanderaseArgumentwill cause segfault. This PR guards against manipulation of empty entry block by checking whether func op is external.An example can be seen in google/heir#1324
The segfault trace