google / xls

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

Improve `for` loop syntax #1671

Open mikex-oss opened 1 month ago

mikex-oss commented 1 month ago

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

for loops are quite cumbersome compared to other languages like C++ or SystemVerilog.

This largely stems from the lack of mut, probably, but it is still a pain every time I need to write a for loop.

Consider an egregious case of building 2 related 2D arrays. In other languages, this would otherwise be a trivial nested for loop with adjacent inner loops. Let's take SystemVerilog as an example, since the focus is on representing synthesizable hardware:

baz_t [J-1:0][I-1:0] foo;
baz_t [K-1:0][I-1:0] bar;

 always_comb begin
    for (int i = 0; i < I; i++) begin
      for (int j = 0; j < J; j++) begin
        foo[j][i] = baz_t'(...);
      end

    for (int k = 0; k < K; k++) begin
      bar[k][i] = baz_t'(...);
    end
  end 
end

This takes significantly more thought to write in DSLX (see below).

Current best alternative workaround (limit 100 words)

Two approaches come to mind (just sketched out, may have errors). Neither seems as easy to get right without thinking as the code above.

  1. Do all the updates in the inner loop and keep track of multiple layers of the full array accumulator. Something like:

        let (foo, bar) = for(i, (foo, bar)): (u32, (baz_t[I][J], baz_t[I][K])) in range(u32:0, I) {
            let foo_new = for(j, foo): (u32, baz_t[I][J]) in range(u32:0, J) {
                let foo_j = update(foo[j], i, ...);
                update(foo, j, foo_j)
            }(foo);
    
            let bar_new = for(k, bar): (u32, baz_t[I][K]) in range(u32:0, K) {
                let bar_k = update(bar[k], i, ...);
                update(bar, k, bar_k)
            }(bar);
    
            (foo_new, bar_new)
        }((zero!<baz_t[I][J]>(), zero!<baz_t[I][K]>()));
  2. Split this into two outer loops, so you can do the array updates in the same direction as the nested looping:

        let foo = for(j, foo): (u32, baz_t[I][J]) in range(u32:0, J) {
          let foo_j_new = for (i, foo_j): (u32, baz_t[I]) in range(u32:0, I) {
            update(foo_j, i, ...)
          }(foo[j]);
          update(foo, j, foo_j_new)
        }(zero!<baz_t[I][J]>());
    
        let bar = for(k, bar): (u32, baz_t[I][K]) in range(u32:0, K) {
          let bar_k_new = for (i, bar_k): (u32, baz_t[I]) in range(u32:0, I) {
            update(bar_k, i, ...)
          }(bar[k]);
          update(bar, k, bar_k_new)
        }(zero!<baz_t[I][K]>());

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

Make it easier to write for loops.

cdleary commented 4 weeks ago

Yeah any more imperative syntax will be dependent on let mut and there are also "data flow clarity" tradeoffs around let mut. I'd also like the for loops to be friendlier to newcomers but there's a bit of an inherent challenge in then quickly teaching them stuff that obscures the dataflow.

For your use case with multidim arrays, in a short term solve, I think we can probably make a cartesian product helper (like a shorthand for an array builder along the lines of itertools.product(range(), range()) -- would probably dovetail reasonably with the multidimensional update index.

proppy commented 1 week ago

Related: it would be nice to have a for syntax variant to loop over all the possible value of an enum; currently it can involve casting the bounds to the uN[std::sizeof(EnumType) + u32:1] to be able to encode the "excluded" upper bound (eg: if the the last enum value saturate all the bits of the underlying uN type).