feat(avm)!: byte indexed PC #9582
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @fcarreiro and the rest of your teammates on |
fcarreiro
force-pushed
the
fc/avm-byte-pc
branch
from
c1771d4 to
c98ed60
Compare
fcarreiro
requested review from
dbanks12,
jeanmon,
IlyasRidhuan and
Maddiaa0
as code owners
| FF { value: FieldElement }, | ||
| // Unresolved brillig pc that needs translation to a 16 bit byte-indexed PC. | ||
| BRILLIG_LOCATION { brillig_pc: u32 }, | ||
| } |
| const auto last_mul_pc = | ||
| 2 * Deserialization::get_pc_increment(OpCode::SET_8) + 11 * Deserialization::get_pc_increment(OpCode::MUL_8); | ||
|
|
There was a problem hiding this comment.
I added a constant for the number of iterations and replaced the related magic constants. This will be shipped as part of my next PR.
| const auto opcode = static_cast<OpCode>(opcode_byte); | ||
| const auto iter = OPCODE_WIRE_FORMAT.find(opcode); | ||
| if (iter == OPCODE_WIRE_FORMAT.end()) { | ||
| throw_or_abort("Opcode not found in OPCODE_WIRE_FORMAT: " + to_hex(opcode) + " name " + to_string(opcode)); | ||
| } |
There was a problem hiding this comment.
What would it mean for the wire format not to include the opcode? Is this basically a redundant check that "opcode is valid"?
There was a problem hiding this comment.
I see now that this isn't new code and that the only diff for it is that it was pulled into a helper function and indentation changed
| expect(results.reverted).toBe(false); | ||
| const expectedResults = Buffer.concat('0010101011'.split('').map(c => new Fr(Number(c)).toBuffer())); | ||
| const resultBuffer = Buffer.concat(results.output.map(f => f.toBuffer())); | ||
|
|
||
| expect(resultBuffer.equals(expectedResults)).toBe(true); | ||
| expect(results.output.map(f => f.toNumber().toString()).join('')).toEqual('0010101011'); | ||
| }); | ||
|
|
| // but otherwise will run out of gas | ||
| new Jump(/*jumpOffset*/ 2), | ||
| // Note: 15 is the byte index, calculated as 3*size(Set.wireFormat8) | ||
| new Jump(/*jumpOffset*/ 15), | ||
| ]); |
There was a problem hiding this comment.
we should probably just use size of set here instead of having a magic 15 (even with the comment)?
This PR moves the AVM to use byte-indexed PCs
Why are we doing this?
A note on how PCs are mapped in the transpiler: we do 2 passes. First we translate all instructions and leave brillig location operands as
BRILLIG_LOCATION. On a second pass, since now we know the structure of the program and the brillig=>AVM pcs, we replace those.There are a few big caveats
Since the JUMP(I) and INTERNALCALL operands are U16, we cannot jump or call a location bigger than 2^16. This effectively constrains the contract size to 65kB.We use 32 bit jumps now.Solutions?
Part of #9059.