Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSR time and timeh registers #61

Closed
mvlamar opened this issue Nov 12, 2019 · 5 comments
Closed

CSR time and timeh registers #61

mvlamar opened this issue Nov 12, 2019 · 5 comments

Comments

@mvlamar
Copy link

mvlamar commented Nov 12, 2019

Hi,
I think it would be very nice if the CSR registers time (3073) and timeh (3201) could be implemented, as simple as
long csr_time = System.currentTimeMillis();

Suggestion:
Maybe, to reduce the computational complexity, instead of continuously showing their values in the register bank panel, these values can be updated only when these registers are read by csrr t0, 3073 or csrr t0, 3201 instructions.

We are implementing these registers on RISC-V hardware (FPGA), and this improvement in Rars would be nice to maintain the "compatibility".
Of course, there are many other issues involved, such as step-by-step execution and back step tracking, as you said before, but we can deal with this in sequence. :)

@TheThirdOne
Copy link
Owner

I am totally willing to implement time and timeh; because I was unsure of what behavior was best I was leaving off actually implementing it until someone needed it. So now that you need it I should implement it in some form.

Technically speaking, just using system time is a very simple solution that would take a total of ~10 lines. Another solution of similar complexity would just be mirroring instret or cycles.

Those two potential solutions would have different behavior when interactively debugging.

I would leave it to you to decide between those two or suggest a third option. I see pros and cons to both options for various people, but you have a concrete use case so it should be more clear.

@mvlamar
Copy link
Author

mvlamar commented Nov 13, 2019

Since Chapter 10 of the RISC-V Instruction Set Manual (version 20190608) defines the time and timeh registers as a real-time clock, I think using a millisecond counter and keeping them independent of the cycle and instret registers (in rather than mirroring them) is the best choice.
The decision to take cycle = instret is OK because it defines that Rars emulates a single-cycle processor. However, doing time = cycle forces the clock frequency of a theoretical processor to be 1kHz. It may be interesting to give Rars some degree of freedom. Thinking about educational purposes, it is very interesting to allow some performance analysis experiments. Therefore, using system time (a millisecond counter from January 1, 1970) is the best option in my opinion.
Perhaps step-by-step debugging should ignore this. The focus of debugging is to fix the code, not measure its performance.

@TheThirdOne
Copy link
Owner

I don't believe the specification defines anything about how long a single tick for the time register is.

While 1ms / tick seems reasonable, I think a wide range of periods would be compliant.

However, doing time = cycle forces the clock frequency of a theoretical processor to be 1kHz.

I believe making time = cycles would only restrict it to a hypothetical single stage, fixed frequency processor.

@TheThirdOne
Copy link
Owner

time and timeh are now implemented by just using system time. Rather than update only when read, they update their values at the end of each cycle.

You should be able to check it out in the latest continuous release. If you would like a stable release I can do that, but I would like to make the next release include support for doubles which will probably take at least a week.

@mvlamar
Copy link
Author

mvlamar commented Nov 19, 2019

Hi,
It is perfect!
Thank you!

marcelachim pushed a commit to marcelachim/rars that referenced this issue Dec 20, 2024
Some tools were not updated correctly because of forced colors on them (Issue TheThirdOne#61 ). This fixes half of them. The rest is still do be done.
marcelachim pushed a commit to marcelachim/rars that referenced this issue Dec 20, 2024
Last tool that needed modification for dark mode ( issue TheThirdOne#61 ) was the memory reference visualization that is updated here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants