[FIRRTL] Add create vector/bundle ops

[fabianschuiki]

As discussed in #1304, constructing a vector or bundle currently requires a temporary firrtl.wire, with subfield/subindex ops and connects. This makes it generally difficult to find out if a vector/bundle is a constant, among other things.

It might make sense to introduce create_vector and create_bundle ops, similar to HW's create_array and create_struct, to generaten aggregate value from fields. It might also make sense to have a create_vector_uniform (after LLHD's array_uniform) which constructs a vector where all elements are the same.

[youngar]

Earlier I had prototyped a BundleOp, but we decided not to add it because we only had one limited use-case, and it was slightly "scary" in that it let you create bundles with flipped elements. The last comment also mentions that it's similar to just using a wire #1127 (comment). If we have more use-cases then it makes sense to add these ops :)

I didn't look too closely, but for the InferResets use-case it doesn't look like you would need any flips. I wonder if it would make sense to support aggregate typed constants.

[fabianschuiki]

Thanks for pointing #1127 out, @youngar! The issue regarding flips didn't really cross my mind, but it's a pretty good one. My specific use case was the following in InferResets:

// Goal: add a reset to %x = firrtl.reg %clk : !firrtl.bundle{a: uint<8>, b: uint<8>}
%c0_ui8 = firrtl.constant 0 : !firrtl.uint<8>
%0 = !firrtl.wire : !firrtl.bundle{a: uint<8>, b: uint<8>}  // <-- zero value for reg
%1 = !firrtl.subfield %0["a"] : !firrtl.uint<8>
%2 = !firrtl.subfield %0["b"] : !firrtl.uint<8>
firrtl.connect %1, %c0_ui8 : !firrtl.uint<8>, !firrtl.uint<8>
firrtl.connect %2, %c0_ui8 : !firrtl.uint<8>, !firrtl.uint<8>
%x = firrtl.reg %clk, %reset, %0 : !firrtl.bundle{a: uint<8>, b: uint<8>}

A bundle op could simplify this to the following, which may become relevant at some point because it's easier to figure out if %0 is a constant (a requirement for async reset values). Doing this through a wire is quite complicated, having to resolve last-connect semantics on possibly indirect drives through subfield accesses.

// Goal: add a reset to %x = firrtl.reg %clk : !firrtl.bundle{a: uint<8>, b: uint<8>}
%c0_ui8 = firrtl.constant 0 : !firrtl.uint<8>
%0 = firrtl.bundle %c0_ui8, %c0_ui8 : !firrtl.bundle{a: uint<8>, b: uint<8>}
%x = firrtl.reg %clk, %reset, %0 : !firrtl.bundle{a: uint<8>, b: uint<8>}

This use case is very simple since there are no flips in the zero value for a reset. But if we add a bundle op, we might want to consider flips. Or put differently: I'd love for the result of firrtl.bundle to have simple value semantics, but that seems to be at odds with flips. I have no idea what a flip does on something that is decidedly not a wire. And what would people expect to happen if you connect a firrtl.bundle result to a firrtl.wire where there is a flip involved. Should the flipped fields be just skipped, because the firrtl.bundle cannot be modified? That seems a bit awkward to me.

@seldridge's comment on #1127 is interesting where he proposes that firrtl.bundle could just support passive bundles, and explicitly force the user of the op to handle the flips, e.g. by skipping those fields and using a partial connect. That would make the use case clearer for InferResets as well: there you'd want to strip all flips from reg, because the reset value applies to all fields, regardless of how connects to the register flip things around (if it even allows for that).

[youngar]

And what would people expect to happen if you connect a firrtl.bundle result to a firrtl.wire where there is a flip involved. Should the flipped fields be just skipped, because the firrtl.bundle cannot be modified? That seems a bit awkward to me.

When we talked about this before, we decided a bulk-connect was equivalent to the expanded connect statements, and that a constant on the LHS is illegal and not just skipped.

there you'd want to strip all flips from reg, because the reset value applies to all fields, regardless of how connects to the register flip things around (if it even allows for that).

We have that problem with Invalid; it sets all fields of a duplex value regardless of their flip. We can't just create an a giant invalid bundle and bulk connect it to a wire, so it's handled at parse time :'( Luckily registers don't allow flips.

I have no idea what a flip does on something that is decidedly not a wire

I think you have to look at the flow of the thing you're bundling up and verify no flows get reversed. If you're just bundling up constant values, it should be illegal for them to be flip in the bundle. The old bundle op was being used to bundle up all the result values of an instance op, so instance outputs were non-flip, while the instance inputs were flipped.

[fabianschuiki]

Seems like there could be room for a "unflipped" connect, which just discards flips, for things like invalidation of wires. But maybe it's overkill to have such an op for only one use case. Also, if that reset value construction is the only use true case for a bundle op, it might not be worthwhile to add 🤔.

[lattner]

I think it makes sense for this to exist - it will make a lot of code simpler, e.g. the code in the parser that is handling the implicitly-flattened-into-multiple-results operation. I'm not sure the best way to model this w.r.t. flow, but treating it equivalent to a wire with connects of the elements and a use of the whole thing makes a lot of sense.

This is will also make "wire promotion" simpler.

[fabianschuiki]

This sounds interesting. How would we handle flipped fields on structs in this case? Since the op itself would imply a connect to each field, do these connects get flipped on fields?

And there's still the problem that it's challenging to figure out whether the wire is a constant: you'd have to look at all uses -- looking through subaccesses -- and ensure that there are no connects. That could be a helper function, but it's not that much better than doing the same on a wire and checking that the corresponding last connect was from a constant.

Do we need this to have wire semantics? Or could it have value semantics, like a node, allowing for easy const-ness checks?

[lattner]

I'm suggesting that this have value semantics not connect semantics. We already have a thing with connect semantics: wire! :)

This would allow eliminating wires and lead to more modular transformations:

And there's still the problem that it's challenging to figure out whether the wire is a constant: you'd have to look at all uses -- looking through subaccesses -- and ensure that there are no connects.

Right. What I'm suggesting is that we can/should promote all uses of wires to their value semantic input (deleting the wire if we're allowed to), which separates out the promotion from all the transformations. This is similar to how LLVM uses mem2reg/SROA to promote allocas to value semantic domain independently of all the xforms that happen on ssa values (like const prop)

[dtzSiFive]

Added in #4177 .