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

macc: Stop using the B port #4817

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

macc: Stop using the B port #4817

wants to merge 1 commit into from

Conversation

povik
Copy link
Member

@povik povik commented Dec 13, 2024

What are the reasons/motivation for this change?
Explain how this is achieved.

The B port is for single-bit summands. These can just as well be represented as an additional summand on the A port (which supports summands of arbitrary width). An upcoming $macc_v2 cell won't be special-casing single-bit summands in any way.

In preparation, make the following changes:

  • remove the bit_ports field from the Macc helper (instead add any single-bit summands to ports next to other summands)

  • leave B empty on cells emitted from Macc::to_cell

If applicable, please suggest to reviewers how they can test the change.

An included test makes sure we interpret the B port on existing RTLIL exports.

The B port is for single-bit summands. These can just as well be
represented as an additional summand on the A port (which supports
summands of arbitrary width). An upcoming `$macc_v2` cell won't be
special-casing single-bit summands in any way.

In preparation, make the following changes:

 * remove the `bit_ports` field from the `Macc` helper (instead add any
   single-bit summands to `ports` next to other summands)

 * leave `B` empty on cells emitted from `Macc::to_cell`
@povik povik mentioned this pull request Dec 13, 2024
14 tasks
@widlarizer
Copy link
Collaborator

I think some theoretical merit was mentioned in the past to the B port when techmapping to primitives that only support single-bit summands. Since I have no actual evidence of that being used by anyone, it's just something to keep in mind. Otherwise I'm in favor of the idea

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