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

Better cleanup for functions sliding data in #1315

Merged
merged 2 commits into from
Jun 30, 2022

Conversation

jfalcou
Copy link
Owner

@jfalcou jfalcou commented Jun 30, 2022

Some functions used a bad cleanup and slide_left somehow forgot to get cleaned up properly.
This fix #667 and clear up the issue raised by the latest tests rewrite on unary slide_left.

@jfalcou
Copy link
Owner Author

jfalcou commented Jun 30, 2022

@DenisYaroshevskiy current implementation generates a logical mask from constexpr values.
Even if the code generation looks OK. the alternative is to use slide_right which doesn't require additional constant generation.
However slide_right is in core, and remove_garbage lives in detail so I am a bit reluctant to use it.

What's your opinion?

@jfalcou
Copy link
Owner Author

jfalcou commented Jun 30, 2022

Owner

@DenisYaroshevskiy current implementation generates a logical mask from constexpr values. Even if the code generation looks OK. the alternative is to use slide_right which doesn't require additional constant generation. However slide_right is in core, and remove_garbage lives in detail so I am a bit reluctant to use it.

What's your opinion?

After discussion, we can use slide_right

@jfalcou jfalcou force-pushed the fix-667/better-cleanup branch from 3093978 to c50e4f5 Compare June 30, 2022 17:19
@DenisYaroshevskiy DenisYaroshevskiy merged commit 4c92c1c into main Jun 30, 2022
@DenisYaroshevskiy DenisYaroshevskiy deleted the fix-667/better-cleanup branch June 30, 2022 19:34
jtlap pushed a commit that referenced this pull request May 12, 2024
* Better cleanup for slide_left

* Factorize sum style cleanup into remove_garbage
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.

[FEATURE] clean up with known at compile time ignore
2 participants