Add fix for inlining slices

[leonardt]

See https://github.com/rdaly525/coreir/issues/895 for more context

This adds a step in the inlining process to insert intermediate wires for slice selects so the current inlining implementation works. There might be alternative solutions to this, but hopefully this fixes the functionality bug with minimal changes to the code. I played around with some other ideas but it seems pretty complicated to get this right in the actual inlining logic (need to map from indices of one slice to indices of another). I think this gets us a baseline working version with the least amount of code, but may introduce some overhead in the output (extra wire instances), but I think this might be better than blasting the slices into individual bits.

[rdaly525]

Unfortunately it is not just inline that is affected, but any code that uses the "addPassthrough" function. Grep-ing through the source code it looks like at least 8 passes use this function, so we need to a more general solution. Also, if addPassthrough is safe for slices, then inlining is safe for slices.

Similar to your code, we can do a similar tactic of adding coreir.wire during the PTTraverse function (used by addPassthrough). Basically in the first line of that function, when the other wireable is a slice, stamp out a coreir.wire instance.

Your code does an additional optimization where it does not add "unneeded" coreir.wire instances. This should be easy to detect and cleanup in a separate pass. In fact that pass would be analogous to copy propagation and would be generally useful as a normal optimization pass.

Let me know if this makes sense. Apologies for not being clearer in the issue.

[leonardt]

I'd personally like to avoid adding unnecessary things and cleaning them up later just for performance reasons. This doesn't mean that copy propogation wouldn't be useful, just that if it's easy enough to manage during the process, it's probably a better idea to handle it immediately (we already have scaling issues so we don't want to make them worse).

I'll see about generalizing the passthrough logic.

[leonardt]

Also, I found out that checking of already created wire instances is necessary, because if we're introducing a wire that connects up to an input, we can only have one driver, so various slices need to drive the same wire (we can't introduce a wire for each slice driving a portion of the input, else we'll get a multiple driver error)

[leonardt]

I've moved the logic into PTTraverse. I also removed the module def transformation cacheing logic since that seems like it should be managed externally to the passthrough logic (so transformers of modules should cache transformations when possible, rather having this built into passthrough which generically operates on an arbitrary context so it would have to manage the cacheing internally).

[leonardt]

I ended up only inserting a wire of to was a slice, which seems to work for our test case. If I did other, it didn't properly handle the slices. I can also insert the wires for to and other but this seemed to just insert an extra wire of indirection which didn't seem necessary since it seems to work when just inserting the wire for to

[leonardt]

The change uncovered two things: first, passthrough is used in the flatten types logic, this seemed to work before wit slices, so are we sure this is a general passthrough problem and not an inlining problem? Perhaps we can add a test case for the non-flattentypes passthrough usage to see if it works? For now, we can just insert the wires in flatten types, but ideally we'd avoid this if it's unnecessary just for code readability. Also, the new wire insertion logic uses the mantle wire for convenience (since it can generate the wire based on type), however the verilog backend is not set up to inline them. For now, I changed the code to guard against inlining mantle wires, we can add this feature in a separate change set.

[rdaly525]

I know that the inline_passthrough code would not handle these slices appropriately in all cases as it makes an assumption that the number of possible selects for a wireable is the same for a given type. This assumption is broken when using slices.

I think adding in these wires is a fine workaround.