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

Use Zc* extensions instead of just the C extension #517

Merged
merged 2 commits into from
Aug 12, 2024

Conversation

jordancarlin
Copy link
Contributor

@jordancarlin jordancarlin commented Jul 28, 2024

The C extension was subdivided into several smaller extensions (Zca, Zcf, Zcd). The sail model already had the instructions for these separate extensions split into separate files, but they were all enabled by the C extension. This PR switches them to be gated by their respective Zc* extensions.

For now, Zca (which Zcf and Zcd both require) is enabled when C is enabled to keep consistency with the current configuration. Once model configuration is possible we should change this to support any legal configuration (ie RV32FD_Zca without Zcf or Zcd).

Copy link

github-actions bot commented Jul 29, 2024

Test Results

712 tests  ±0   712 ✅ ±0   0s ⏱️ ±0s
  6 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit b4a6db0. ± Comparison against base commit 7de3d70.

♻️ This comment has been updated with latest results.

model/riscv_insts_zcb.sail Outdated Show resolved Hide resolved
@billmcspadden-riscv billmcspadden-riscv added the tgmm-agenda Tagged for the next Golden Model meeting agenda. label Aug 12, 2024
@billmcspadden-riscv
Copy link
Collaborator

@jrtc27 , are you satisfied with the changes made per your request?

Copy link
Collaborator

@jrtc27 jrtc27 left a comment

Choose a reason for hiding this comment

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

I think this is ok now

@billmcspadden-riscv
Copy link
Collaborator

@jrtc27 : Thanks.

@billmcspadden-riscv billmcspadden-riscv merged commit 05b845c into riscv:master Aug 12, 2024
2 checks passed
@jordancarlin jordancarlin deleted the zc_extensions branch August 13, 2024 02:24
@Timmmm
Copy link
Collaborator

Timmmm commented Aug 29, 2024

@billmcspadden-riscv for PRs like this please can you squash them and set the commit message to something informative (i.e. in this case it should be the text from the PR "The C extension was subdivided...").

Otherwise the actual Git history - which is the thing that is preserved for all time, and what you see in editors etc. - just says "Use Zc* extentions" which is very unhelpful.

Next to the merge button there is a drop-down where you can select "Squash and Merge":

image

Then when you click it you get to edit the message and you can make sure it contains the relevant information.

For some PRs (e.g. the vector one) we probably want to preserve history, but I think for the vast majority it is not relevant and people don't do a good job of writing commit messages anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tgmm-agenda Tagged for the next Golden Model meeting agenda.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants