NJU-ProjectN / nemu

NJU EMUlator, a full system x86/mips32/riscv32/riscv64 emulator for teaching
Other
889 stars 192 forks source link

A quetion about difftest's implement in `src/cpu/difftest/dut.c` #51

Closed Ch111p closed 1 year ago

Ch111p commented 1 year ago

Hi, I'm not a student from NJU, but learning NJU's ICS accroding to ICS offcial website && open-source code right now. It's a very fantasitic experience to me! I have already learnt a lot from this course despite I'm just on the way to finish lab2.4! I want to say thank you for your guys hard works in this course and generosity at first!

So here is my question. https://github.com/NJU-ProjectN/nemu/blob/09bb925782871ed4eb98d17cfabab323d473df62/src/cpu/difftest/dut.c#L128 checkregs need a PC parameter, I think that means we should check pc's value in it's definition. But after ref_difftest_exec, the PC value in ref's CPU_STATE should be next_pc. So I think we should pass next_pc or npc when calling checkregs just like the code below https://github.com/NJU-ProjectN/nemu/blob/09bb925782871ed4eb98d17cfabab323d473df62/src/cpu/difftest/dut.c#L109

But ICS course gives student freedom to exploring by themselves. So I'm doubting whether it's a mistake, or it's on purpose for training student, or I'm just totally wrong like the code should be works fine(or works in some ISAs).

At last, very thanks for your guys again!

sashimi-yzh commented 1 year ago

Thank you for the question. I think the lecture note should be more elaborative.

The second argument of checkregs() is just for displaying messages. If difftest fails at pc = 0x1000, but after the execution of that instruction, cpu.pc will become 0x1004. However, we still want to display messages with pc = 0x1000, since it is the instruction which causes failure.

I am going to add the following text to the lecture note. Do you think it is clear enough?

特别地, isa_difftest_checkregs()对比结果不一致时, 第二个参数pc应指向导致对比结果不一致的指令, 可用于打印提示信息.
Ch111p commented 1 year ago

I think it's enough:wink:. Thanks for your reply!