google / xls

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

[DSLX] Switch multidimensional arrays to Rustic syntax? #616

Open cdleary opened 2 years ago

cdleary commented 2 years ago

As mentioned in #526 there is an outstanding discrepancy between DSL multidimensional arrays and Rustic ones. I do think the DSL ones we formulated this way before we got serious about making it as close to Rust as possible (back then we were trying to make it look more like parameterized and concise IR), but some of the nice properties we were thinking at the time we around the inside-out nature of unwrapping the type definition with indexing, e.g. when mixing tuples and arrays:

let x: ((u2, u3)[4], (u6, u8)[2][5])[3]

In that sort of type definition you unravel the outermost entities to the innermost ones, e.g. x[0][1][4][1][1] gets you the u8 because you're always unwrapping the "rightmost" decorating dimension.

I /believe/ there's no difference in power or clarity in the Rust array syntax (Rust just nests the multidimensional array items more verbosely) but was discussing this with @RobSpringer recently so CC him for his thoughts as well.

meheff commented 3 months ago

The syntax of multidimensional arrays came up again recently. The current syntax looks like SystemVerilog and C declarations and indexing, but the indices are reversed. For example, in DSLX:

let a: u8[3][2] = ...;
// Index last element.
... = a[1][2];

While in SystemVerilog we have:

logic [2:0][1:0][7:0] a;
// Index last element
... = a[2][1];

And similar ordering in C.

I think the current syntax for DSLX makes sense at some level. In the declaration you are defining the type inside out. This is natural when building up compound types where the innermost type is on the left in the declaration and indexing is outside in:

let b: (u32[3], u8)[2] = ...;
... = b[1].1[2];

// Equivalent to (c[3])[2].
let c[3][2] = ...;
... = c[1][2];

However, the fact that this looks like SystemVerilog / C but indexes in the opposite direction makes this untenable IMO. A couple possibiilities are:

  1. Reverse the indexing order to match SystemVerilog / C.

  2. Use Rust multidimensional array syntax. For example:

    let a: [[u8; 3]; 2];

I think option 1 makes things inconsistent compared compound types like array of tuples (b example above), so I would prefer 2. The Rust syntax still has the dimension order with innermost on the left, but the nesting makes it clear which one is the outer dimension and which one is the inner dimension.