riscv / sail-riscv

Sail RISC-V model
https://lists.riscv.org/g/tech-golden-model
Other
456 stars 165 forks source link

Risc-V Sail code has no version string #109

Open allenjbaum opened 3 years ago

allenjbaum commented 3 years ago

We would like to implement a versioning string so that we can easily tell if a version with some non-backwards compatible change is being used by checking the version. I don't think it matters what the format of the version is - it just need to be monotonically increasing, and not a random hash value. The riscof team has implemented this as a simple CI script, I believe, and you might be able to follow their recipe.

neelgala commented 3 years ago

riscof uses semantic versioning. The CI script basically picks up the latest version present in the changelog.md and creates a release for the same version.

If every PR should create a release then every PR will need to include an entry in the CHANGELOG.md.

We also have a definition of how the versioning number must be assigned.

jrtc27 commented 3 years ago

Is semantic versioning really needed though? The architecture should always be backwards-compatible.

jrtc27 commented 3 years ago

If you just want a way to identify whether sail-riscv A is newer than sail-riscv B, git rev-list --first-parent --count HEAD gives you the number of commits (excluding the right side of any merges) made to the current branch; it's what FreeBSD uses in lieu of its old SVN revision numbers.

neelgala commented 3 years ago

not really.. any monotonically increasing versioning system will work. If you are using the CI from riscof then you simply need to change the format of the string inside [] each release entry of the changelog.md file. Calendar versioning will also work.

neelgala commented 3 years ago

its not just for comparison.. the compliance/compatibility reports (html-base) will require mentioning what version of sail was used to reference signature generations. Having something that we can quickly infer as old and new would be best.

allenjbaum commented 3 years ago

If counting the number of commits is easy, that's fine. Semantic versioning gives a bit more information, but not enough to require it if its any more work.

In the particular case that started all this, it wasn't the architecture that was changing, it was the reporting that was an issue. Specifically: issue 16, where CSRwrites to restricted view CSRs (e.g. xSTATUS) always report the full CSR (MSTATUS) in Spike rather than the actual CSR being written (SStatus or UStatus) that Sail reports, and the request was to make it match Spike. It's probably more useful to do it the Spike way, since SW that depends on it can do the filtering it needs, but couldn't do it the other way around.

That is an incompatible change that other SW (which use that reporting information) would notice - but isn't an architectural change. Having said that: there are architectural changes that are not backwards compatible, but they only get made if its highly unlikely anyone actually depends on that behavior. You can look at the priv spec preface that mentions them.

On Fri, Oct 1, 2021 at 4:05 AM Neel Gala @.***> wrote:

its not just for comparison.. the compliance/compatibility reports (html-base) will require mentioning what version of sail was used to reference signature generations. Having something that we can quickly infer as old and new would be best.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/riscv/sail-riscv/issues/109#issuecomment-932133644, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJUNTI6V7MJEMG5JGH3UEWIZBANCNFSM5FD7CLKA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

martinberger commented 3 years ago

We would like to implement a versioning string

Hello Allen, where do you want the version strings to be, in each .sail file, or once, somewhere in the top-level directory of the master branch? If the latter, would not git tags do the job for you?

allenjbaum commented 3 years ago

Top level, master branch is fine; that's the one that vendors will be using (and/or will be in the docker image)

On Sat, Oct 2, 2021 at 11:59 AM Martin Berger @.***> wrote:

We would like to implement a versioning string

Hello Allen, where do you want the version strings to be, in each .sail file, or once, somewhere in the top-level directory of the master branch?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/riscv/sail-riscv/issues/109#issuecomment-932804241, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJQVJB7ZDNK352BGF5LUE5JCNANCNFSM5FD7CLKA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

martinberger commented 3 years ago

How about we have a top-level file VERSION.md in master that contains as first line the git tag for the release?

jrtc27 commented 3 years ago

Markdown isn't helpful, it requires you to look at the source. Better to have something embedded in the binary itself. Perhaps the output of git describe (possibly with --first-parent, depending on how you want to think of merge commits)? If you have a tag checked out, it prints the name of that tag, otherwise it prints the name of the most recent tag on your branch, plus the number of commits made since then, plus the current hash. So, for example, master is currently 0.5-104-g9f71c75, meaning commit 9f71c75 (how many characters gets printed is configurable), which is 104 commits newer than 0.5 (including the "other" sides of merges), or 0.5-70-g9f71c75 if you pass --first-parent to consider merges as a single commit.

martinberger commented 3 years ago

something embedded in the binary itself.

Which binaries have you got in mind?

jrtc27 commented 3 years ago

something embedded in the binary itself.

Which binaries have you got in mind?

riscv_sim_RV64 etc

martinberger commented 3 years ago

How do you suggest that version be extracted from binaries, by grepping or something like a --version command line switch?

jrtc27 commented 3 years ago

How do you suggest that version be extracted from binaries, by grepping or something like a --version command line switch?

The latter was what I assumed would be done, like any other versioned tool

martinberger commented 3 years ago

That's fine. Does Sail offer a reliable facility to drop this kind of information into all it's backends (OCaml, C, HOL ...) or would this have to be done on a per-backend basis?

jrtc27 commented 3 years ago

No, command-line parsing is done in each platform's wrapper (riscv_sim.c and riscv_ocaml_sim.ml), though in theory you could make it a Sail string and have each platform read that so there's at least only one copy of the version, but that might be a bit unnecessarily complicated (and might require flags to stop Sail optimising out the variables).

The prover snapshots don't support any notion of command-line arguments, nor do you "run" them, but their README.md's already automatically document what commit they were built from, so could be modified to match the C and OCaml versioning scheme.

martinberger commented 3 years ago

@jrtc27 Sounds all very reasonable, I'm happy to have such an addition. Now we 'only' need to agree on the exact naming scheme. I have no particular preferences. It sounds like @allenjbaum and @neelgala are OK with something like 0.5-104-g9f71c75, do I understand this correctly?

neelgala commented 3 years ago

@martinberger what we (arch-test folks) would like is something like riscv_sim_RV64 --version to return the version number (which I think @jrtc27 also agrees with). So I don't think the scheme you suggested (count of newer commits) might work in that case. Its possible that folks share just the c-emulator binaries without copying the source git-repo, in which case probing using git-describe would not be possible.

jrtc27 commented 3 years ago

Nobody ever suggested that be done at run time, that would be stupid. The version would be calculated at build time and baked into the binary.

neelgala commented 3 years ago

thanks for the correction, well.. then it works for us.

allenjbaum commented 3 years ago

Just to ensure I'm understand @jrtc27 https://github.com/jrtc27's comment: you're referring to the calculation of the version number not being a run time thing, because Neels suggestion of riscv_sim_RV64 --version is returning the version at runtime (as opposed to calculating it). Does using --version on the command line work for everyone? I don't think the exact format of the string that is returned is too critical, as long as we can do a simpler older/new ercomparison on it.

jrtc27 commented 3 years ago

Just to ensure I'm understand @jrtc27 https://github.com/jrtc27's comment: you're referring to the calculation of the version number not being a run time thing, because Neels suggestion of riscv_sim_RV64 --version is returning the version at runtime (as opposed to calculating it). Does using --version on the command line work for everyone? I don't think the exact format of the string that is returned is too critical, as long as we can do a simpler older/new ercomparison on it.

During the build process it would run git describe and bake that into the binary. You can then run riscv_sim_RV64 --version whenever and wherever you like and it will always tell you the same thing, corresponding to the git checkout it was built from at the time it was built.

allenjbaum commented 3 years ago

Ah, thank you - I hadn't connected the git -describe to this process. I'm not sure this is more or less complicated than the approach we are using on other repos, but if give me some monitonically increasing string, I'm happy.

On Tue, Oct 12, 2021 at 12:27 PM Jessica Clarke @.***> wrote:

Just to ensure I'm understand @jrtc27 https://github.com/jrtc27 https://github.com/jrtc27's comment: you're referring to the calculation of the version number not being a run time thing, because Neels suggestion of riscv_sim_RV64 --version is returning the version at runtime (as opposed to calculating it). Does using --version on the command line work for everyone? I don't think the exact format of the string that is returned is too critical, as long as we can do a simpler older/new ercomparison on it.

During the build process it would run git describe and bake that into the binary. You can then run riscv_sim_RV64 --version whenever and wherever you like and it will always tell you the same thing, corresponding to the git checkout it was built from at the time it was built.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/riscv/sail-riscv/issues/109#issuecomment-941347493, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJTNKIYQ2FQT2ZNUFYDUGSD3JANCNFSM5FD7CLKA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.