Skip to content

[mlir] add overload createDIScopeForLLVMFuncOp function #111689

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

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

Observer007
Copy link
Contributor

follow up work of #106229, add create pass overload function to create pass.

@llvmbot
Copy link
Member

llvmbot commented Oct 9, 2024

@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-mlir

Author: None (Observer007)

Changes

follow up work of #106229, add create pass overload function to create pass.


Full diff: https://github.com/llvm/llvm-project/pull/111689.diff

2 Files Affected:

  • (modified) mlir/include/mlir/Dialect/LLVMIR/Transforms/Passes.h (+4)
  • (modified) mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp (+9)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/Transforms/Passes.h b/mlir/include/mlir/Dialect/LLVMIR/Transforms/Passes.h
index 078c7d12f6f22b..c988ffe1a9b3ff 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/Transforms/Passes.h
+++ b/mlir/include/mlir/Dialect/LLVMIR/Transforms/Passes.h
@@ -23,6 +23,10 @@ namespace LLVM {
 /// Create a pass to add DIScope to LLVMFuncOp that are missing it.
 std::unique_ptr<Pass> createDIScopeForLLVMFuncOpPass();
 
+struct DIScopeForLLVMFuncOpOptions;
+std::unique_ptr<Pass>
+createDIScopeForLLVMFuncOpPass(const DIScopeForLLVMFuncOpOptions &options);
+
 /// Generate the code for registering conversion passes.
 #define GEN_PASS_DECL
 #define GEN_PASS_REGISTRATION
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
index 052e98ea8b8d48..91cc65d528559f 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
@@ -86,6 +86,10 @@ namespace {
 /// Add a debug info scope to LLVMFuncOp that are missing it.
 struct DIScopeForLLVMFuncOp
     : public LLVM::impl::DIScopeForLLVMFuncOpBase<DIScopeForLLVMFuncOp> {
+  DIScopeForLLVMFuncOp() = default;
+  DIScopeForLLVMFuncOp(const mlir::LLVM::DIScopeForLLVMFuncOpOptions &options)
+      : DIScopeForLLVMFuncOpBase(options) {}
+
   void runOnOperation() override {
     ModuleOp module = getOperation();
     Location loc = module.getLoc();
@@ -135,3 +139,8 @@ struct DIScopeForLLVMFuncOp
 std::unique_ptr<Pass> mlir::LLVM::createDIScopeForLLVMFuncOpPass() {
   return std::make_unique<DIScopeForLLVMFuncOp>();
 }
+
+std::unique_ptr<Pass> mlir::LLVM::createDIScopeForLLVMFuncOpPass(
+    const mlir::LLVM::DIScopeForLLVMFuncOpOptions &options) {
+  return std::make_unique<DIScopeForLLVMFuncOp>(options);
+}

struct DIScopeForLLVMFuncOpOptions;
std::unique_ptr<Pass>
createDIScopeForLLVMFuncOpPass(const DIScopeForLLVMFuncOpOptions &options);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that removing the let constructor = from the TableGen declaration for this pass will see the createXXX method auto-generated. Can you try this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, then the changes become more concise.

Copy link
Contributor

@Dinistro Dinistro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small drive by comment

@@ -86,6 +86,10 @@ namespace {
/// Add a debug info scope to LLVMFuncOp that are missing it.
struct DIScopeForLLVMFuncOp
: public LLVM::impl::DIScopeForLLVMFuncOpBase<DIScopeForLLVMFuncOp> {
DIScopeForLLVMFuncOp() = default;
DIScopeForLLVMFuncOp(const mlir::LLVM::DIScopeForLLVMFuncOpOptions &options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DIScopeForLLVMFuncOp(const mlir::LLVM::DIScopeForLLVMFuncOpOptions &options)
explicit DIScopeForLLVMFuncOp(const mlir::LLVM::DIScopeForLLVMFuncOpOptions &options)

Except you want implicit conversion, ofc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, using using Base::Base instead.

Copy link
Contributor

@Dinistro Dinistro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this generates all the pass creation function you require, this is LGTM!
Please make sure that you update the commit message accordingly, as it no longer matches what this change does.

Copy link
Collaborator

@joker-eph joker-eph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG, thanks!

@Observer007 Observer007 merged commit d124b98 into llvm:main Oct 10, 2024
8 checks passed
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
follow up work of llvm#106229, add
create pass overload function to create pass.

---------

Co-authored-by: jingzec <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants