Skip to content
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

Confusing error reporting while parsing a nested pass pipeline #59151

Closed
shabalind opened this issue Nov 23, 2022 · 4 comments
Closed

Confusing error reporting while parsing a nested pass pipeline #59151

shabalind opened this issue Nov 23, 2022 · 4 comments
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior mlir:core MLIR Core Infrastructure

Comments

@shabalind
Copy link
Contributor

shabalind commented Nov 23, 2022

I'm currently working on a project that invokes an MLIR pass pipeline using Python bindings:

mypm = pm.PassManager.parse(f"""builtin.module(
    mypass1,
    func.func(mypass2,mypass3)
)""")

Somewhat surprisingly this pass pipeline is not valid according to the pipeline parser, it tells me to add , at the end after func.func(...). At the same time if you do add that comma, it errors as well since there is no valid pass name after the comma.

As a workaround I added a dummy pass that does nothing just to make it happy:

mypm = pm.PassManager.parse(f"""builtin.module(
    mypass1,
    func.func(mypass2,mypass3),
    dummy-do-nothing-pass
)""")

It would be great if this was not necessary in the first place, and parser accepted the initial snippet right away.

@shabalind shabalind added bug Indicates an unexpected problem or unintended behavior mlir:core MLIR Core Infrastructure labels Nov 23, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 23, 2022

@llvm/issue-subscribers-mlir-core

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 23, 2022

@llvm/issue-subscribers-bug

mshockwave pushed a commit that referenced this issue Jan 30, 2023
An user might want to add extra spaces for better readability, e.g:
```
mypm = pm.PassManager.parse(f"""builtin.module(
    mypass1,
        func.func(mypass2,mypass3)
)""")
```
GitHub issue #59151

The parser was not taking into account the possibility of spaces after
`)`or `}`

Differential Revision: https://reviews.llvm.org/D142821
@boschmitt
Copy link
Contributor

@River707 you can close this issue, the commit fixes it (Poor choice of wording on my part, but I was unsure how the info in the review process would land in the commit)

@shabalind
Copy link
Contributor Author

@boschmitt Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior mlir:core MLIR Core Infrastructure
Projects
None yet
Development

No branches or pull requests

5 participants