-
Notifications
You must be signed in to change notification settings - Fork 10
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
PR for llvm/llvm-project#64553 #653
Conversation
…es` to `operandSegmentSizes` This renaming started with the native ODS support for properties, this is completing it. A mass automated textual rename seems safe for most codebases. Drop also the ods prefix to keep the accessors the same as they were before this change: properties.odsOperandSegmentSizes reverts back to: properties.operandSegementSizes The ODS prefix was creating divergence between all the places and make it harder to be consistent. Reviewed By: jpienaar Differential Revision: https://reviews.llvm.org/D157173
…ptionalParseResult' Fix #63072
Using properties would break when a dialect isn't in the mlir namespace
…t when initialization is needed The current logic hashes the context to detect registration changes and re-run the pass initialization. However it wasn't checking for changes to the pipeline, so a pass that would get added after a first run would not be initialized during subsequent runs. Reviewed By: Mogball Differential Revision: https://reviews.llvm.org/D158377
…elected fields It is surprising for the user that only some fields were honored. Also make the FrozenRewritePatternSet a shared_ptr<const T>. Fixes #64543 Differential Revision: https://reviews.llvm.org/D157469
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.
LGTM. Merging these changes in makes sense
@joker-eph needs a rebase. |
@tru I updated the branch in my fork, is the bot gonna update this automatically? |
No, you need to comment with the I don't think it's that well-documented, unfortunately. But it will also have to change for LLVM 18.x since we should be using PRs natively then. |
Somehow it does not pick it up: llvm/llvm-project#64553 (comment) ? |
Seems to hit some kind of snag in the automation. Want me to manually merge this @joker-eph ? |
Sure, whatever is easier for you! |
This was merged manually. closing. |
resolves llvm/llvm-project#64553