AXI generator clean up

[nathanielnrn]

A few small issues were brought up that should be addressed. I find it helpful to keep these all in one place.

• Reduce code duplication by refactoring where appropriate.
• Closed by #1867. As mentioned here, in our channels we create base_addr and curr_addr registers. The naming is a bit vague and base_addr should be changed to something that reflects that this goes to our byte-addressed AXI addresses, while curr_addr should be changed to reflect that it indexes into our internal calyx memories based on the memories indexing.
• Closed by #1867. Assert in build that our yxi is well formed. i.e, powers of 2, width is byte aligned, etc.
• Closed by #1867. Talked about this synchronously with @sampsyo and have had a few more thoughts since then:
  Reevaluate xVALID_was_high signals based on our in sync discussion. In general the point of this is to denote we are not in the first "raising of xVALID" cycle and that a handshake has occurred. We need this because we want to ensure that in a single servicing of a transfer xVALID is only asserted once. Because we have no guarantees on when the servicing group will terminate, we look to xVALID_was_high to ensure xVALID is only high once. When we discussed this in person we assumed this would be in the first cycle, but I think this should be generalized to denote when a handshake_occurred. More concretely if we are in the group that services a transfer I believe we want the following (being a bit sloppy with notation):

//we are not currently in a handshake and haven't been high before
xVALID.in = !(xVALID & xREADY) & !(handshake_occurred) ? 1'b1;
//negate the above. Key point is that `handshake_occurred` stays high.
xVALID.in = (xVALID & xREADY) | handshake_occurred ? 1'b0;
xVALID..write_en = 1'b1;

//left side of `|` is a handshake, useful for when xVALID is high and xREADY is low
//right hand side is preemptive handshake when xREADY is high and we are about to drive xVALID high. Saves a cycle?
// actually might not be that useful                                            //guard for xVALID.in = 1'b1
handshake_occurred.in = (xVALID & xREADY) | (xREADY & (!(xVALID & xREADY) & !(handshake_occurred)))? 1'b1;
// okay to write to this as long as it is 0
handshake_occurred.write_en =  !(handshake_occurred) ? 1'b1;

I think we can also make

handshake_occurred.in = (xVALID & xREADY) | (xREADY & (!(xVALID & xREADY) & !(handshake_occurred)))? 1'b1;

simpler and just do

handshake_occurred.in = (xVALID & xREADY) ? 1'b1

because even if there is a cycle delay by the time we are worried about xVALID being set high again happening handshake_occurred will be high in that cycle and prevent a toggling of xVALID

[sampsyo]

This sounds perfect. That new definition for handshake_occurred crystalizes what's going on here: we want to keep track of the moment when the handshake happened, by that natural "valid && ready" definition, and use that to suppress future handshakes.

[nathanielnrn]

Closing this for now, leaving the refactoring item open as something that can be done with a later issue.