riscv bb executor + continuations #1921
Conversation
There was a problem hiding this comment.
Some small comments here and there, was mainly checking the new bootloader abstraction as requested by @pacheco
| @@ -25,16 +25,17 @@ pub enum FieldArgument { | |||
| Bb, | |||
| #[strum(serialize = "gl")] | |||
| Gl, | |||
| #[strum(serialize = "bn254")] | |||
| Bn254, | |||
| // TODO(leandro): do we need to support this in riscv now? | |||
There was a problem hiding this comment.
Maybe not, but why remove here?
| }) | ||
| .collect::<Vec<_>>(); | ||
|
|
||
| // TODO: PC needs to be split for small fields! |
There was a problem hiding this comment.
Not sure, because in the RISCV machine we cap it at 24 bits.
| todo!() | ||
| // TODO: small field MemoryWithBootloaderWrite machine | ||
| r#" | ||
| std::machines::memory_bb::Memory memory(byte2); |
There was a problem hiding this comment.
| std::machines::memory_bb::Memory memory(byte2); | |
| std::machines::small_field::memory::Memory memory(bit12, byte2); |
| // Stores val(W) at address (V = val(X) - val(Z) + Y) % 2**32. | ||
| // V can be between 0 and 2**33. |
There was a problem hiding this comment.
| // Stores val(W) at address (V = val(X) - val(Z) + Y) % 2**32. | |
| // V can be between 0 and 2**33. | |
| // Stores val(WL) at address (V = val(XL) - val(ZL) + (YH, YL)) % 2**32. |
| link ~> (tmp3_h, tmp3_l) = regs.mload(WL, STEP + 2) | ||
| link ~> (tmp4_h, tmp4_l) = arith_bb.sub(tmp1_h, tmp1_l, tmp2_h, tmp2_l) | ||
| link ~> (tmp5_h, tmp5_l) = arith_bb.add(tmp4_h, tmp4_l, YH, YL) | ||
| link ~> memory.mstore_bootloader(tmp6_l, STEP + 3, tmp3_h, tmp3_l) |
There was a problem hiding this comment.
ah we need to add this bootloader stuff to the normal memory machine, right?
| /// For now, this trait is implemented directly by each `FieldElement` type, because the hash functions (i.e., poseidon) are field-specific. | ||
| pub trait BootloaderImpl { | ||
| type Fe: FieldElement; | ||
| const FE_PER_WORD: usize; |
There was a problem hiding this comment.
| const FE_PER_WORD: usize; | |
| const FE_PER_RISCV32_WORD: usize; |
just to make it more obvious?
There was a problem hiding this comment.
i think just WORD is fine, we already use "WORD" with this same meaning in the existing bootloader constants (e.g., WORDS_PER_PAGE). I'll add a small comment
| // bootloader inputs, we store it as a "word", the same representation | ||
| // as the other registers in memory, which may be more than one fe | ||
| // (e.g., 2 for small fields such as BabyBear). | ||
| // Here we return the composed fe value. |
There was a problem hiding this comment.
I think we could keep the pc as a single FE because in asm we do that right now.
There was a problem hiding this comment.
I think its better to keep everything as 2 fe in the bootloader itself, because it all becomes "bootloader inputs", which are two columns in the small field machine.
|
|
||
| fn update_page(page: &mut Self::Page, idx: usize, word: u32) { | ||
| let (hi, lo) = bb_split_word(word); | ||
| // TODO: check proper endianess here! |
There was a problem hiding this comment.
The actual memory is little endian
Just for running tests. PR will be broken down