ice40_dsp: fix log_assert issue #4942
Conversation
| sigH.append(O[i]); | ||
| } | ||
| for (i = GetSize(O) - 1; i > 0 && nusers(O[i]) <= 1; i--) | ||
| ; | ||
| // This sigM could have no users if downstream sinks (e.g. $add) is | ||
| // narrower than $mul result, for example | ||
| if (i == 0) |
There was a problem hiding this comment.
Is this bit still correct?
There was a problem hiding this comment.
also this makes for a good opportunity to figure out what the comment referencing a non-existent sigM is actually supposed to say
There was a problem hiding this comment.
The point of original, pre-patch code was to cut unused bits from the end of output port. This code does the same but starts iterating from the end of the port. Seems like the outputs of SB_MAC16 needs to be connected with empty wire, so this piece is used to group all unused bits into one wire.
There was a problem hiding this comment.
So the weird comment actually points to this being copied from xilinx_dsp where this check is written pretty much the same way as of this PR
| wire_width++; | ||
| else { | ||
| if (wire_width) { // add empty wires for bit offset if needed | ||
| sigH.append(module->addWire(NEW_ID, wire_width)); |
There was a problem hiding this comment.
It seems odd to append to sigH a wire of wire_width width in each iteration. Is that the intended behavior?
There was a problem hiding this comment.
The reason for this is this line, as far as I understand, unused output ports on this type of cells should be connected with wires, so this code add them between wires of used ports.
There was a problem hiding this comment.
I actually misread this loop before, my bad, I get it now
What are the reasons/motivation for this change?
#4865
Explain how this is achieved.
This code iterates not from the beginning but from the end of output signal, leaving only bits that are currently in use with unused bits needed for offset.