calyxir / calyx

Intermediate Language (IL) for Hardware Accelerator Generators
https://calyxir.org
MIT License
453 stars 45 forks source link

Support for arbitrary widths greater than 64 #1969

Closed matth2k closed 2 months ago

matth2k commented 2 months ago

A feature of the allo frontend is its use of arbitrarily precise types for all intermediate results. When the largest width is less than or equal to 64, Calyx is happy. However, evaluating a polynomial like x * x * x * x * x where x is 32b will cause a problem.

Here is a smaller toy program that has the issue:

import "primitives/core.futil";

component main (@go go: 1) -> (@done done: 1, @stable output: 1) {
  cells {
    counter = std_reg(97);
    out_reg = std_reg(1);
    shift = std_lsh(97);
    out_slice = std_bit_slice(97, 41, 42, 1);
  }
  wires {

    static<1> group updateReg {
      shift.right = 97'b1;
      shift.left = counter.out;
      counter.in = shift.out;
      counter.write_en = 1'b1;
    }

    out_slice.in = counter.out;
    output = out_slice.out;

  }
  control {
    updateReg;
  }
}
$ calyx -b verilog ap.futil 
thread 'main' panicked at 'Widths greater than 64 are not supported.', calyx-ir/src/builder.rs:170:21
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
EclecticGriffin commented 2 months ago

@rachitnigam Sorry if this is off base, but it seems like you could probably use 64 - val.leading_zeroes() to determine the minimum number of bits needed to represent the value and then compare that against the width, unless I'm overlooking something. This of course doesn't solve the issue when you want to write a constant which cannot be expressed in 64 bits but that would need parser changes anyway.

There are also the ilog methods but those unfortunately round down rather than up, but you could always add one to them if the value given is not a power of 2 minus 1, which I think would cover it. This has the advantage of not requiring changes if the internal size changes but might be more cumbersome?

Anyway, food for thought assuming I didn't make any mistakes in my reasoning

rachitnigam commented 2 months ago

@EclecticGriffin that sounds like a reasonable stopgap IMO. Do you want to open a PR doing this? I'm, as usual, not close enough to a computer to do any reasonable coding.

EclecticGriffin commented 2 months ago

Yeah I can handle that!

EclecticGriffin commented 2 months ago

stopgap patch available on the linked PR. Not sure if we should close this issue with it or leave it open until there is full support for arbitrary widths, but should at least unblock things