[Handshake] Update `HandshakeToFIRRTL` to use low-level logic instead of `WhenOp` about circt HOT 6 CLOSED

Yes, I think that's correct!

from circt.

@hanchenye I think we can close this now.

from circt.

@hanchenye @stephenneuendorffer I started taking a look at this, and I have a question about the ops that implement the MergeLikeOpInterface. In these ops, it seems like we should apply the SameOperandsAndResultType trait. We don't have that right now, but does that constraint make sense here?

from circt.

@mikeurbach I think the select signal of MuxOp is not necessary to have the same type with other operands.

from circt.

Ah, true. I guess I am specifically referring to the data operands (as returned by the dataOperands method on MergeLikeOpInterface). These operands should always have the same type (and have the same type as the result), correct?

from circt.

@hanchenye I have been looking into MergeOp and ControlMergeOp next, and I just want to check in before digging too far into the implementation. I reviewed Section 3.4 and Figure 7 here. My understanding is Handshake's ControlMergeOp op is what they implement in Figure 7., i.e. it reports the index of the selected input as an output.

I had previously mentioned I would start with implementing MergeOp since it is simpler, but now I'm thinking it would be advantageous to start with the trickier ControlMergeOp. Then we can factor out the common part, and just leave off the unneeded aspects when implementing MergeOp. If that all sounds good I'll plan on sharing a PR for ControlMergeOp next.

from circt.