feat: remove generic parameter from the BigNum trait
#44
Conversation
TomAFrench
changed the title
BigNum trait
* main: feat: remove generic parameter from the `BigNum` trait (#44) fix: fix broken tests in `runtime_bignum_test.nr` (#39) feat: remove a bunch of unnecessary bytecode from unconstrained ops (#50) fix: Fix barrett reduction bug (#51) feat: optimize brillig execution of `split_X_bits` functions (#47)
| @@ -272,11 +270,17 @@ where | |||
| let params = Params::get_params(); | |||
| evaluate_quadratic_expression::<_, MOD_BITS, _, _, _, _>( | |||
| params, | |||
| map(lhs_terms, |bns| map(bns, |bn| Self::get_limbs(bn))), | |||
| map( | |||
There was a problem hiding this comment.
@TomAFrench I've honed in on this PR being the one that caused a regression in the constraint counts of BigNum. In particular, I was measuring the constraints of calls to this evaluate_quadratic_expression function.
Any thoughts on why the changes in this PR are increasing constraint counts? Is it simply that converting between slices and arrays is not free?
There was a problem hiding this comment.
The extra ACIR opcodes (for a main function which calls this evaluate_quadratic_expression) function appear to be:
INIT (id: 0, len: 4)
EXPR [ (-1, _5) 0 ]
MEM (id: 0, read at: x5, value: x6)
EXPR [ (-1, _7) 1 ]
MEM (id: 0, read at: x7, value: x8)
EXPR [ (-1, _9) 2 ]
MEM (id: 0, read at: x9, value: x10)
EXPR [ (-1, _11) 3 ]
MEM (id: 0, read at: x11, value: x12)
INIT (id: 1, len: 4)
MEM (id: 1, read at: x5, value: x13)
MEM (id: 1, read at: x7, value: x14)
MEM (id: 1, read at: x9, value: x15)
MEM (id: 1, read at: x11, value: x16)
There was a problem hiding this comment.
Yeah, this is bad actor codegen as we're just reading all the values out of the array to write into another one.
There was a problem hiding this comment.
Neater code here, to fix it: #53
Although not explained in the original code, there's a reason why arrays of size
Nare never passed to/from setters and getters ofBigNum; it's so that external libs can write use a genericBigNumwhere BigNum: BigNumTrait, without having to specify anN. Removal offrom_arrayandget_limbsresolves this.Also resolves a modulus_bits typo in one of the off-the-shelf params files.