google / xls

XLS: Accelerated HW Synthesis
http://google.github.io/xls/
Apache License 2.0
1.18k stars 173 forks source link

[enhancement] revise channel fifo depth syntax #1561

Open proppy opened 3 weeks ago

proppy commented 3 weeks ago

What's hard to do? (limit 100 words)

The way we set channel fifo depth doesn't feel coherent w/ the rest of DSLX syntax

Depth is currently set as the second arg of the channel type declaration, ex:

        let (data_s, data_r) = chan<u8, u32:1>("data");

But it doesn't need to be set on the type of the lhs let binding.

i.e: in this example: the type for data_r is chan<u8> in not chan<u8, u32:1> in.

Current best alternative workaround (limit 100 words)

None.

Your view of the "best case XLS enhancement" (limit 100 words)

proppy commented 3 weeks ago

@grebe you add an idea around using an attribute for this, could you expand?

grebe commented 3 weeks ago

Perhaps channels should have some configuration via attributes, e.g.

proc A {
  #[fifo(depth=1)]
  ch0: chan<u32>
  ...
}

This is a bit tricky in practice as a channel member is only one end of the channel, so you need some rules around how to resolve conflicts.

Another option is to have different chandecl syntaxes, e.g.

let (x, y): (chan<u32>, chan<u32>) = fifo("name", DEPTH);
let (y, z): (chan<u32>, chan<u32>) = fifo_flopped("name", DEPTH, REGISTER_PUSH_OUTPUTS, REGISTER_POP_OUTPUTS);
ericastor commented 2 weeks ago

FWIW, I like the second option suggested in @grebe's comment; I think it's nice if a chan is the thing that can be passed to send/receive, where only the interface is specified (which would include things like flow control) - and otherwise the details on the channel implementation are left to the point where we create the channel (e.g., an explicit fifo call rather than a chan constructor).