mcycle value does not increment in sail

[danish-10xe]

I am writing test that checks mcycle value increments. For Spike, the value of mcycle increases, whereas for Sail, the value of mcycle remains constant. Here is the test result screenshot and the assembly code for the test.

LI(x1,0)
LI(a5,-1) // ALL 1's
LI(x5, 1)
csrw mcycle, a5
nop
csrr a4, mcycle
nop // nop in case of trap
RVTEST_SIGUPD(x13, a4) // Putting updated value in signature file

add t0, x5, x0
csrr a4, mcycle		
nop				// nop in case of trap
RVTEST_SIGUPD(x13, a4)		// Putting updated value in signature file	

addi t0, x5, 3
csrr a4, mcycle		
nop				// nop in case of trap
RVTEST_SIGUPD(x13, a4)		// Putting updated value in signature file	

csrr a4, mcycle		
nop				// nop in case of trap
RVTEST_SIGUPD(x13, a4)		// Putting updated value in signature file

[rmn30]

Hi. this looks like a problem with the integration of your tool (riscof?) with the Sail model. In the c emulator there is a call here to the Sail function tick_clock. You'll need a similar call in whatever is driving the Sail.

[neelgala]

@rmn30 within riscof, the C emulator is simply run as a command line similar to below (see this):

riscv_sim_RV32 --test-signature=ref.signature test.elf > test.log 2>& 1

where riscv_sim_RV32 is the c emulator binary built from this repo.

Do you suggest we build the emulator differently or use other flags in the cli for the mcycle issue ?

[rmn30]

Ah, on second thoughts the issue is more likely the rv_insns_per_tick shown in the C code. For some reason this defaults to 100 which means the mcycle only increments once every 100 instructions! There doesn't seem to be a way to change this other than changing it in the source code here (I assume you are not building the tandem verification mode with ENABLE_SPIKE -- I don't know if that still works).

So maybe we should add a new command line argument to the simulator to set rv_insns_per_tick and perhaps change the default value?

[rmn30]

Another potential solution would be to move the call to tick_clock outside the if statement, assuming the clock should increment once per instruction.

[neelgala]

Just curious - any reason why its 100 as of now ? would making the default to 1 break anything internally ?

A cli arg would definitely be best way forward.

[rmn30]

Just curious - any reason why its 100 as of now ? would making the default to 1 break anything internally ?

I'm not really sure. Looks like @pmundkur wrote it. Maybe something to do with what spike did or with booting OS more quickly?

[billmcspadden-riscv]

FYI: I intend to make this configurable at some point in the future (based on settings in the RISCV-Config file probably).

Note: There are other counters/timers that suffer from a similar problem. The Sail model has no concept of clock cycles or of time (which is the main source of the problem).

[allenjbaum]

I don't think there is any value with making it anything but 1, actually. DUTs will have nondeterministic values that could be both less than 1 (e.g. superscale, so multiple ops per cycle) or >1 (stalls), so there isn't much point in making it configurable that I can see. For tests that care about timing, we will need to use instret as a reference, not cycle (or time).

…

On Wed, Nov 23, 2022 at 7:23 AM Bill McSpadden ***@***.***> wrote: FYI: I intend to make this configurable at some point in the future (based on settings in the RISCV-Config file probably). Note: There are other counters/timers that suffer from a similar problem. The Sail model has no concept of clock cycles or of time (which is the main source of the problem). — Reply to this email directly, view it on GitHub <#193 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJQUQS2GEGB2EYG3GV3WJYZF7ANCNFSM6AAAAAASI3F3U4> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>