refactor/ remove MOCK_PROGRAM in MockProver #395
Conversation
ceno_emul/src/tracer.rs
Outdated
| @@ -167,6 +192,39 @@ impl StepRecord { | |||
| } | |||
| } | |||
|
|
|||
| fn new_insn2( | |||
There was a problem hiding this comment.
Tried not to touch existing code so having this function and have a quick PoC. Will modify new_insn if this approach works. The same as above new_r_instruction2
ceno_emul/src/tracer.rs
Outdated
| rd: Option<Change<Word>>, | ||
| previous_cycle: Cycle, | ||
| ) -> StepRecord { | ||
| let insn = DecodedInstruction::from_raw(insn_kind, MOCK_RS1, MOCK_RS2, MOCK_RD); |
There was a problem hiding this comment.
This from_raw method does not look right. It cannot be that simple to support all the instruction formats.
There was a problem hiding this comment.
Instead, I would write "encode" functions for each type that return insn_code: u32, and then pass this to (like StepRecord::new_r_instruction). This way the test data go through the same decoder as production code.
ceno_zkvm/src/scheme/mock_prover.rs
Outdated
| Self::run_maybe_challenge(cb, wits_in, &[], &[], None) | ||
| } | ||
|
|
||
| pub fn run_with_program( |
ceno_emul/src/tracer.rs
Outdated
| @@ -52,6 +52,10 @@ impl<T> MemOp<T> { | |||
| pub type ReadOp = MemOp<Word>; | |||
| pub type WriteOp = MemOp<Change<Word>>; | |||
|
|
|||
| const MOCK_RS1: u32 = 2; | |||
| const MOCK_RS2: u32 = 3; | |||
| const MOCK_RD: u32 = 4; | |||
There was a problem hiding this comment.
That kind of test data belongs more closely to the MockProver module or test utils of ceno_zkvm.
| @@ -85,13 +87,14 @@ fn test_opcode_or() { | |||
| .unwrap() | |||
| .unwrap(); | |||
|
|
|||
| let insn_code = EncodedInstruction::encode(InsnKind::OR, 2, 3, 4, 0); | |||
ceno_emul/src/rv32im.rs
Outdated
| @@ -207,6 +207,81 @@ pub struct InsnCodes { | |||
| pub func7: u32, | |||
| } | |||
|
|
|||
| pub struct EncodedInstruction; | |||
There was a problem hiding this comment.
This could be a module rather than a type.
ceno_emul/src/rv32im.rs
Outdated
| @@ -207,6 +207,81 @@ pub struct InsnCodes { | |||
| pub func7: u32, | |||
| } | |||
|
|
|||
| pub struct EncodedInstruction; | |||
| impl EncodedInstruction { | |||
| pub fn encode(kind: InsnKind, rs1: u32, rs2: u32, rd: u32, imm: u32) -> u32 { | |||
KimiWu123
force-pushed
the
refactor/mock_program
branch
from
948b163 to
eff9abf
Compare
KimiWu123
force-pushed
the
refactor/mock_program
branch
from
eff9abf to
ffaece6
Compare
KimiWu123
changed the title
|
@naure, do you mind reviewing this PR again? It's the final version now. |
This PR is trying to remove
MOCK_PROGRAMandMOCK_PC_XXXinMockProver, related to part of #331.The idea is to
load_program_tableout fromload_tableso that we can pass any opcodes we would like to test, andDecodedInstruction::from_raw()to reassemble the opcode format forMockProver.