-
Notifications
You must be signed in to change notification settings - Fork 428
[WIP] Fixing Subtraction CIN Bug #3261
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
base: master
Are you sure you want to change the base?
Conversation
@vaughnbetz - Opened this PR for the next person to take over the issue. |
Thanks @loglav03 . @KennethKent : anyone to take on this bug fix? |
@vaughnbetz, @loglav03 will work on the bug for us.
Thanks for poking to remind me.
On Sep 14, 2025, at 1:52 AM, vaughnbetz ***@***.***> wrote:
⚠External message: Use caution.
[https://avatars.githubusercontent.com/u/5912487?s=20&v=4]vaughnbetz left a comment (verilog-to-routing/vtr-verilog-to-routing#3261)<#3261 (comment)>
Thanks @loglav03<https://github.com/loglav03> . @KennethKent<https://github.com/KennethKent> : anyone to take on this bug fix?
—
Reply to this email directly, view it on GitHub<#3261 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACQMJJTI4MQMVLUYXLMYCRL3STX7VAVCNFSM6AAAAACFE2V3FOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTEOBZGE4DONZQGM>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Currently, the newly updated code for splitting an adder for subtraction matches the structure depicted in the solution diagram provided by the related issue: #2266 However, even though the adder chain structure matches the provided solution, it is still failing vpr. I found that the old code was passing through vpr even though the subtraction adder chain was buggy. So, I investigated this to see what was causing it to pass, hoping it would give insight into what is causing the new code to fail. I found that in the parmys elaboration pass, it walks the netlist and calls the function At the end of this function, it has the conditional block shown below, which would remap the 5th output pin to a buffer node whose input pin is fed by the PAD node, which essentially makes the last output pin a constant 0.
The issue I'm seeing with the new code is that the output .names seem to be getting dropped from the pre-vpr.blif, which trips up the vpr stage because it can't see any primary outputs for the subtraction adder chain. The output .names appear in the parmys.blif so it looks like abc ends up dropping them before pre-vpr.blif is created. So, it seems that every primary output being driven by the sumout pin of each adder slice gets dropped at some point after yosys+parmys and before producing the pre-vpr.blif (likely in abc). I believe this is being caused by the adder chain being blackboxed so abc won't touch it (see .blackbox attribute in .blif files) and results in abc not being able to see the primary outputs since they're driven by the adder chain and drops them. The old code was able to produce a single primary output seen by abc and pass vpr because the final output pin of the original wide adder was remapped to a buffer node driven by the pad node, which later in the parmys techmap pass, the buffer node gets eliminated and the nets between the pad node-to-buf node and the buf node-to-primary output gets joined together (see function starting at line 545 in vtr-root/parmys/parmys-plugin/mapping/partial_map.cc), so the PO for the last output pin is driven by the pad node and not the adder chain itself. (See difference in .blif outputs below for old vs. new code for outputs .names) Old parmys.blif: Old pre-vpr.blif: Note: In the new code out files, the ordering is out of place for the adder sub circuits. The final adder sub circuit appears first in the sub circuit list. This doesn't affect anything, it's just the way they happened to be ordered. New parmys.blif: New pre-vpr.blif: Currently, we are looking for a solution to this. We tried connecting each sumout to buffer nodes so they drive the primary outputs instead of being directly driven from the adder chain. However, this gets blown up by that buffer elimination step in tech mapping and the POs still end up getting driven by the sumouts. |
I'm a bit out of my depth in parmys operation here ... @KennethKent any ideas? |
Description
When doing a 4-bit input a and b subtraction to get a 5 bit output, we must compute as 2's complement = (1's complement + 1)
See as a + (1's comp. of b) + 1 = a + (2's comp. of b).
The subtraction carry chain was originally computing 2's complement as 1's complement (a + (1's comp. of b)). The reason for this is that when the original wide hard adder was split into a 1-bit adder chain, the head adder of the chain was supposed to seed the rest of the chain with a carry-in of 1. Instead it computed GND (0) + VCC (1) + GND (0), which gives a cout of 0, seeding the rest of the chain with a carry-in of 0, resulting in a + (1's comp. of b). This was fixed to compute VCC + VCC + GND, which seeds a carry-in of 1 to the rest of the chain.
There is also an additional adder added to the tail end of the chain. This adder takes in GND and VCC and CIN as inputs and is supposed to output the correct sign bit out[4]. This is currently broken in the code as I was having trouble connecting the correct output back to the original wide hard adder.
See the issue below for a diagram of the bug and further explanation.
Related Issue
#2266
How Has This Been Tested?
No in depth testing done yet as the code still needs some fixing up.
Types of changes
Checklist: