google / xls

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

Verilog codegen for tuple outputs should retain their structure #394

Open ted-xie opened 3 years ago

ted-xie commented 3 years ago

I have a DSLX function that returns a tuple of type (u1, u8). The tuple fields are called (result, state). I would expect the emitted verilog to look like this:

output wire result,
output wire [7:0] state

Instead, I get a packed bit vector called output wire [8:0] out.

This is still usable, but generally it would be easier to integrate the generated code if the fields were unpacked into separate output ports.

vincent-mirian-google commented 3 years ago

A brief comment: perhaps, a feature request for multiple return values may be a more suited as a general solution, each return value can be assigned to a port.

cdleary commented 3 years ago

cc @meheff I think the reworking he's doing of codegen pipeline could tie in with how tuples or records (named tuples) end up presented on the module signature. Mark thoughts on where this one will fit? (I thought we had another issue open for this but can't find it!) This relates to google/xls#195

allight commented 3 months ago

We might want to build off of the work done on https://github.com/google/xls/issues/1239 to support this.

We could have an annotation on structs/etc #[verilog_export("name")].

We would need to decide how to pass this down to the codegen. The easiest way might be to have the ir_convert do the verilog generation of the struct and pass it down in a field in the ir-interface proto. This is sort of ugly however.

@proppy any ideas?

proppy commented 3 months ago

@allight I was thinking of something like what was suggested in #1433 (as a follow-up to #1429) where we have fn annotations to name the return arguments, ex:

#[output_port_name(foo, bar, hop)]
fn user_module() -> (u1, u2, u3) {
}

or

#[named_return]
fn user_module() -> (foo: u1, bar: u2, hop: u3) {
}

would codegen:

module user_module(
    output wire foo,
    output wire [1:0] bar,
    output wire [2:0] hop
);