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

MCS CRefine sorried for RISCV64 #683

Merged
merged 4 commits into from
Oct 19, 2023
Merged

MCS CRefine sorried for RISCV64 #683

merged 4 commits into from
Oct 19, 2023

Conversation

michaelmcinerney
Copy link
Contributor

This sorries the last files in the CRefine session, thereby completing the setup of the proof infrastructure for MCS CRefine.

In addition, this enables testing of the CRefine session via our continuous integration setup.

@michaelmcinerney michaelmcinerney added the MCS related to `rt` branch and mixed-criticality systems label Oct 17, 2023
@michaelmcinerney michaelmcinerney self-assigned this Oct 17, 2023
@Xaphiosis
Copy link
Member

Looking at the commits for now:

The "update copyright" commit (no tags, no explanantion), at least the CRefine parts look like they should be folded into the sorrying run. Not sure about the lib part.

For the "rt crefine: finish sorrying run", I know I lost the battle about indicating in the tags that this is only for RISCV64, but maybe I could get a comment in the body of the commit, as it's quite a large commit?

For the run_tests and Makefile changes, why are they tagged "rt:", these are not rt specific. The first should say "run_tests: enable CRefine for RISCV64". The second is misleading, as it's RISCV64 only, and also not rt-specific; I'd suggest Gerwin-styled "riscv64 proofs: turn on quick_and_dirty for CRefine".

@lsf37 lsf37 changed the base branch from rt to copyright-update October 18, 2023 00:33
@lsf37 lsf37 changed the title MCS CRefine sorried MCS CRefine sorried for RISCV64 Oct 18, 2023
Base automatically changed from copyright-update to rt October 18, 2023 00:56
@Xaphiosis
Copy link
Member

Xaphiosis commented Oct 19, 2023

rt: update the sorrying-run to seL4 revision 4214775 commit is also crefine-only and not tagged as such. Also for consistency, it's "sorrying run" in one commit and "the sorrying-run" in the next.

The run_tests change is still not tagged with run_tests.

I'm trying to push you a bit towards more consistency with the rest of the repo. In future, if you're doing a change to run_tests or something, can you look at a few past commits with git log run_tests or whatever to see what they look like and try fit in with that rather than invent something new on the spot? It'll really cut down on the amount of these discussions, which I know you probably don't like, and I don't enjoy pointing these out either, believe it or not.

Similarly for the manifest commit, if you do git log mcs.xml you'll see what the previous ones look like, usually something like mcs: bump seL4 revision, whereas yours says rt: update seL4 for MCS testing to current master ... what MCS testing, what does that mean? There's no rt: branch on the manifest repo, and previous commits are tagged mcs:...

Finish the CRefine proof setup for RISCV64 on seL4 revision
756a37b7d3c0.

Signed-off-by: Michael McInerney <[email protected]>
Update CRefine proof setup for RISCV64 to seL4 revision 4214775.

Signed-off-by: Michael McInerney <[email protected]>
Allow sorries during development.

Signed-off-by: Michael McInerney <[email protected]>
Copy link
Member

@lsf37 lsf37 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, the commit messages are cleaned up and the content has been good the entire time already, so that is also fine.

Thanks for diligently adding new lemmas for all of the new MCS functions, this makes the sorry-count much more useful.

@lsf37 lsf37 merged commit b2507df into rt Oct 19, 2023
11 checks passed
@lsf37 lsf37 deleted the michaelm-crefine_sorried branch October 19, 2023 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MCS related to `rt` branch and mixed-criticality systems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants