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

alumacc: alternative cmp unification implementation #4000

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adrianparvino
Copy link

@adrianparvino adrianparvino commented Oct 10, 2023

$lt and related are sometimes inhibited from unifying with $sub as when B < A, then the $sub cell is unable to swap inputs, but $lt does. This resolves this problem by instead considering sig_alu as an unordered pair mapping.

Addendum:
$add and $sub might still be unable to unify with share -aggressive but changing the behavior [1] causes [2] to fail, so I'm keeping it to cmp only.

[1] ded586c#diff-f5a16f8ab735a667cabcdd605b428eeaa92f223f044ef01a91d52141d404633bL334-L336
[2] https://github.com/YosysHQ/yosys/blob/ded586c/tests/opt/opt_expr.ys#L27

@adrianparvino
Copy link
Author

This fails as reordering no longer happens. https://github.com/YosysHQ/yosys/blob/ded586c/tests/opt/opt_expr.ys#L27

@@ -331,9 +331,6 @@ struct AlumaccWorker
if (GetSize(C) > 1)
goto next_macc;

if (!subtract_b && B < A && GetSize(B))
std::swap(A, B);

Copy link
Author

Choose a reason for hiding this comment

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

In hindsight, this can remain, as cmp unification now is order-independent. This also allows the tests to pass.

@adrianparvino adrianparvino force-pushed the alumacc-cmp-contravariance branch from ded586c to de50379 Compare October 11, 2023 13:41
@adrianparvino adrianparvino force-pushed the alumacc-cmp-contravariance branch from de50379 to 1bf1d92 Compare October 11, 2023 13:41
@adrianparvino adrianparvino marked this pull request as ready for review October 11, 2023 13:41
@adrianparvino adrianparvino changed the title alumacc: alternative alumacc cmp contravariance implementation alumacc: alternative cmp unification implementation Oct 11, 2023
@povik povik self-assigned this Oct 23, 2023
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.

2 participants