xls icon indicating copy to clipboard operation
xls copied to clipboard

DSLX: Support multidimensional `update` function

Open mczyz-antmicro opened this issue 1 year ago • 1 comments

Currently, in order to update 1 bit in a 2-dimensional array, one has to extract the row/column, update the 1-dimensional vector and then update the array. An example of updating the bit (13,5) to value 1 in the array is presented below:

let row = array2d[5];
let row = update(row, 13, 1);
let array2d = update(array2d, 5, row);

I propose expanding the definition of the update function to take in:

  • array reference
  • tuple of indices
  • new value for the array slice

The same example with new syntax would be:

let array2d = update(array2d, (13,5), 1);

It could then also allow updates of whole columns/rows, e.g.:

let array2d = update(array2d, (13,_), [0,1,2,3,...]);

Theoretically, this behavior can be generalized into higher dimensions, but if it is impractical to do so, I believe the 2d example would be the most used as this is the standard way to represent memory in digital circuits.

mczyz-antmicro avatar Jan 25 '24 14:01 mczyz-antmicro

There is one more caveat: I've made an assumption that row will be of type array, but it can as well be bits type

let row = array2d[5];

Then you have to use bit_slice_update to update the specific bits in the uN[bits] type.

mczyz-antmicro avatar Jan 29 '24 16:01 mczyz-antmicro

We do support this operation in the IR so it just needs to be added to the DSLX frontend.

meheff avatar May 01 '24 03:05 meheff

Looked into this a bit today: one of the "challenges" is that update is currently typed as (T[N], uN[M], T) -> T[N] https://github.com/google/xls/blob/4d63d892811b21ad4160173077730a77c2f175eb/xls/dslx/frontend/builtins_metadata.cc#L77 and there doesn't seem to be a way to define multiple overloads on a given builtins (or fn).

I can see a few solutions to this:

  • define a new update2d builtin.
  • turn GetParametricBuiltins into a absl::flat_hash_map<std::string, absl::span<BuiltinsData>> so that builtins can support more than one signature.
  • add new syntax elements to BuiltinsData::signature to support arbitrary array dimension and indices.

Curious what others think?

proppy avatar May 07 '24 22:05 proppy