-
Notifications
You must be signed in to change notification settings - Fork 14.6k
[mlir][PDL] PDLToPDLInterp: Use the pdl pattern name in the generated rewriter. #149481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
@llvm/pr-subscribers-mlir Author: None (jumerckx) ChangesBefore, the rewriters generated by Full diff: https://github.com/llvm/llvm-project/pull/149481.diff 1 Files Affected:
diff --git a/mlir/lib/Conversion/PDLToPDLInterp/PDLToPDLInterp.cpp b/mlir/lib/Conversion/PDLToPDLInterp/PDLToPDLInterp.cpp
index 7d20109b3db59..3a1c1fd2d57af 100644
--- a/mlir/lib/Conversion/PDLToPDLInterp/PDLToPDLInterp.cpp
+++ b/mlir/lib/Conversion/PDLToPDLInterp/PDLToPDLInterp.cpp
@@ -632,8 +632,14 @@ void PatternLowering::generate(SuccessNode *successNode, Block *¤tBlock) {
SymbolRefAttr PatternLowering::generateRewriter(
pdl::PatternOp pattern, SmallVectorImpl<Position *> &usedMatchValues) {
builder.setInsertionPointToEnd(rewriterModule.getBody());
+ // Get the pattern name if available, otherwise use default
+ StringRef rewriterName = "pdl_generated_rewriter";
+ if (auto symName = pattern.getSymName()) {
+ rewriterName = symName.value();
+ }
+
auto rewriterFunc = builder.create<pdl_interp::FuncOp>(
- pattern.getLoc(), "pdl_generated_rewriter",
+ pattern.getLoc(), rewriterName,
builder.getFunctionType({}, {}));
rewriterSymbolTable.insert(rewriterFunc);
|
I can't add reviewers because I'm lacking permissions. |
if (auto symName = pattern.getSymName()) { | ||
rewriterName = symName.value(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (auto symName = pattern.getSymName()) { | |
rewriterName = symName.value(); | |
} | |
if (auto symName = pattern.getSymName()) | |
rewriterName = symName.value(); |
Can this be tested? |
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions cpp -- mlir/lib/Conversion/PDLToPDLInterp/PDLToPDLInterp.cpp View the diff from clang-format here.diff --git a/mlir/lib/Conversion/PDLToPDLInterp/PDLToPDLInterp.cpp b/mlir/lib/Conversion/PDLToPDLInterp/PDLToPDLInterp.cpp
index fd2c02f02..86afdfdb2 100644
--- a/mlir/lib/Conversion/PDLToPDLInterp/PDLToPDLInterp.cpp
+++ b/mlir/lib/Conversion/PDLToPDLInterp/PDLToPDLInterp.cpp
@@ -645,8 +645,7 @@ SymbolRefAttr PatternLowering::generateRewriter(
rewriterName = symName.value();
}
auto rewriterFunc = pdl_interp::FuncOp::create(
- builder, pattern.getLoc(), rewriterName,
- builder.getFunctionType({}, {}));
+ builder, pattern.getLoc(), rewriterName, builder.getFunctionType({}, {}));
rewriterSymbolTable.insert(rewriterFunc);
// Generate the rewriter function body.
|
Before, the rewriters generated by
convert-pdl-to-pdl-interp
had a generic"pdl_generated_rewriter"
name. With this change, the name from the original pdl pattern is kept for the rewriter. This makes it easier to relate the two.