Skip to content

Conversation

loglav03
Copy link
Contributor

@loglav03 loglav03 commented Aug 29, 2025

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

  • [ x] Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

@loglav03 loglav03 marked this pull request as ready for review August 29, 2025 22:26
@loglav03
Copy link
Contributor Author

@vaughnbetz - Opened this PR for the next person to take over the issue.

@vaughnbetz
Copy link
Contributor

Thanks @loglav03 . @KennethKent : anyone to take on this bug fix?

@KennethKent
Copy link

KennethKent commented Sep 15, 2025 via email

@loglav03
Copy link
Contributor Author

loglav03 commented Oct 14, 2025

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 equalize_ports_size for nodes with the MINUS type. In the case of the provided test verilog, the original wide hard adder node had a port 'a' and 'b' size of 4 each and an out port size of 5. In this function, the original code had a line int new_out_size = port_a_size; which would create a new node, remap the input pins of the original node to the new node, and only remap the first 4 output pins to the new node and exclude the 5th. (See function starting at line 1365 in vtr-root/parmys/parmys-plugin/netlist/netlist_utils.cc)

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.

if (new_out_size < port_y_size) {
        for (int i = new_out_size; i < port_y_size; i++) {
            /* need to drive extra output pins with PAD */
            nnode_t *buf_node = make_1port_gate(BUF_NODE, 1, 1, node, traverse_mark_number);
            /* hook a pin from PAD node into the buf node */
            add_input_pin_to_node(buf_node, get_pad_pin(netlist), 0);
            /* remap the extra output pin to buf node */
            remap_pin_to_new_node(node->output_pins[i], buf_node, 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:
temp.parmys.txt

Old pre-vpr.blif:
temp.pre-vpr.txt

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:
temp.parmys.txt

New pre-vpr.blif:
temp.pre-vpr.txt

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.

@vaughnbetz
Copy link
Contributor

I'm a bit out of my depth in parmys operation here ... @KennethKent any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants