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

[Comb][Fold] Fix idemp n^2 and bugfix + more optimization. #7514

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

dtzSiFive
Copy link
Contributor

  • Fix n^2 behavior with canonicalizeIdempotentInputs. Add arbitrary "depth" check to bound the search.
  • Don't allow flattening or idempotent to search into operations defined in other blocks.
  • Support removing duplicates even when operands come from other blocks. (or(x, y, x) -> or(x, x) regardless of their origin).
  • Don't walk into operations with different two-state-ness in the idempotent operand canonicalizer.
  • When creating new operation in idempotent canonicalizer, create it with matching two-state-ness.

Add tests for functional changes above.

* Fix n^2 behavior with canonicalizeIdempotentInputs.
  Add arbitrary "depth" check to bound the search.
* Don't allow flattening or idempotent to search into operations
  defined in other blocks.
* Support removing duplicates even when operands come from other
  blocks. (or(x, y, x) -> or(x, x) regardless of their origin).
* Don't walk into operations with different two-state-ness in the
  idempotent operand canonicalizer.
* When creating new operation in idempotent canonicalizer,
  create it with matching two-state-ness.

Add tests for functional changes above.
Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

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

It looks reasonable to me to set the limit. LGTM!

@dtzSiFive
Copy link
Contributor Author

Thanks!

@dtzSiFive dtzSiFive merged commit ebb2429 into llvm:main Aug 13, 2024
4 checks passed
@dtzSiFive dtzSiFive deleted the feature/idem-depth branch August 13, 2024 14:15
@dtzSiFive
Copy link
Contributor Author

(This was tested on various internal designs, and looks good, FWIW!)

@fabianschuiki
Copy link
Contributor

Really nice fix! 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants