Skip to content

Lean: add matchbv support #1222

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

Merged
merged 31 commits into from
May 6, 2025
Merged

Lean: add matchbv support #1222

merged 31 commits into from
May 6, 2025

Conversation

tobiasgrosser
Copy link
Collaborator

No description provided.

tobiasgrosser and others added 19 commits March 18, 2025 20:17
The only semantic change this PR proposes is to not assert on
argument pattern that we cannot yet translate, but instead
to record them as `TODO_ARG_PATTERN`. This is sufficient to
build the full RISC-V Sail model without assertions and with
about 3500 warnings and errors remaining.

This PR also introduces a GitHub action to validate that the
RISC-V model builds. The RISC-V model currently requires two
patches (one in Ryan's repository and one applied through this patch).
Over time, we expect to switch to the main RISC-V repo.

Co-authored-by: Léo Stefanesco <[email protected]>
@tobiasgrosser tobiasgrosser added the Lean Issues with Sail to Lean translation label Apr 9, 2025
@tobiasgrosser
Copy link
Collaborator Author

This superseeds #1182

@tobiasgrosser
Copy link
Collaborator Author

This superseeds #1179 and #1182

Copy link

github-actions bot commented Apr 10, 2025

Test Results

   13 files     28 suites   0s ⏱️
  868 tests   868 ✅ 0 💤 0 ❌
4 012 runs  4 012 ✅ 0 💤 0 ❌

Results for commit 4421904.

♻️ This comment has been updated with latest results.

@ineol
Copy link
Collaborator

ineol commented Apr 10, 2025

I get a super strange universe error in test test/c/lib_dec_bits

@ineol
Copy link
Collaborator

ineol commented Apr 10, 2025

@Alasdair @bacam How would you go about having a command line option to enable the matchbv? the issue is that we need to change the list of rewrites depending on the options, and currently this is given to the Target.register function at init time...

@bacam
Copy link
Contributor

bacam commented Apr 10, 2025

There's an If_flag option you can give in the rewrites list, see the mono_rewrites bit in the Lem backend for an example.

@ineol
Copy link
Collaborator

ineol commented May 1, 2025

It now has a flag -lean-matchbv that's disabled by default.

'encdec_subrange', # Deactivated to enable match_bv
'option_nest', # Deactivated to enable match_bv
'special_annot', # Deactivated to enable match_bv
'vector_subrange_pattern' # Deactivated to enable match_bv
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ineol, these do not need to be disabled, no? match_bv is disabled by default.

@tobiasgrosser
Copy link
Collaborator Author

tobiasgrosser commented May 2, 2025

Nice. Would it be possible to add a single match_bv test case where the corresponding option is set? This would help to make sure this functionality is exercised.

@ineol
Copy link
Collaborator

ineol commented May 2, 2025

Nice. Would it be possible to add a single match_bv test case where the corresponding option is set? This would help to make sure this functionality is exercised.

I changed it to run the tests in the test/lean directory with match_bv activated and the test/c tests without, so that both code paths are exercised.

@tobiasgrosser
Copy link
Collaborator Author

Nice. Then this is good to go.

Copy link
Collaborator Author

@tobiasgrosser tobiasgrosser left a comment

Choose a reason for hiding this comment

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

Lgtm

@tobiasgrosser
Copy link
Collaborator Author

Merging this will also help to reduce the code size of the arm model, which is important as we hit sie limits at github.

@bacam
Copy link
Contributor

bacam commented May 6, 2025

Thanks, will merge once the CI completes.

@bacam bacam merged commit 6c89592 into rems-project:sail2 May 6, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Lean Issues with Sail to Lean translation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants