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

Clone 40s-dv (Delete 40s directory) #2541

Open
wants to merge 2 commits into
base: cv32e40s/dev
Choose a base branch
from

Conversation

silabs-robin
Copy link
Contributor

@silabs-robin silabs-robin commented Oct 15, 2024

This PR deletes cv32e40s/*.


Overview:

After openhwgroup/cv32e40s-dv#3, the external "dv" repo can replace the directory cv32e40s/.
(Just like it was done for the x.)

The core-specific directories in core-v-verif (i.e. cv32e40s/ and cv32e40x/) will be empty, except for a README with instructions for getting the actual content.


Changes:

  • bin/clonetb gets new flags, to help clone cv32e40s-dv.
  • cv32e40s/README.md just explains how to use bin/clonetb.
  • cv32e40s/* is otherwise purged.

Rationale:

This was already done for the X, because 1) it significantly eases merging S->X, and 2) it reduces the size of core-v-verif itself (so it just containts common code, libraries, agents, etc).

The reason for doing it on the S as well is because 1) it makes the system symmetrical (both cores have a *-dv repo), and 2) merging will be even easier, 3) dealing with smaller repos are easier.

We already have the cv32e40s-dv repo established, and half-way up to date, and soon it is full-way up to date.


Reviewing:

Evaluate the "Changes:" above.

If you want to run hello world:
Clone this PR's branch.
See the instructions in cv32e40s/README.md but use clonetb like it was done in openhwgroup/cv32e40s-dv#3.
Then run hello world as normal.

@@ -41,6 +41,7 @@ usage() {
echo "usage: $0 [-x] [--clone] [--ignore] [--unignore]"
echo " -x clone cv32e40x subtree, known stable hash"
echo " --x-main clone cv32e40x subtree, latest main branch"
echo " --s-main clone cv32e40s subtree, latest main branch"
Copy link
Contributor Author

@silabs-robin silabs-robin Oct 15, 2024

Choose a reason for hiding this comment

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

There should be an -s flag too! ("known stable hash".)

But I want it to point to the merge-commit from openhwgroup/cv32e40s-dv#3 after that PR is merged.

@silabs-robin
Copy link
Contributor Author

Hi, @MikeOpenHWGroup, @silabs-hfegran, @silabs-krdosvik.
This PR is in draft, but only because it is pending on a different PR to be merged.
Hence, I am requesting an early review, to inspect the principal changes of this PR.

@silabs-krdosvik
Copy link

silabs-krdosvik commented Oct 15, 2024

Do you plan to change the merge.sh script: https://github.com/openhwgroup/core-v-verif/blob/cv32e40x/dev/bin/merge.sh, at least I think it would need a change, or be deleted to avoid confusion.

Otherwise, looks good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you plan to change the merge.sh script: https://github.com/openhwgroup/core-v-verif/blob/cv32e40x/dev/bin/merge.sh, at least I think it would need a change, or be deleted to avoid confusion.

(Moving this to a thread, to keep related comments bundled together, and to make it resolveable. @silabs-krdosvik )

Yes, that should be changed as well. Good point.

It should be easy?
Where git subtree is used, we can simply add a remote to the cv32e40s-dv repo instead.
Then everything else should work as before.

Do you think it should be done as part of this PR, or could it wait until later?
I also plan to merge cv32e40s-dv -> cv32e40x-dv, so at that time the script would be useful again and prompt a change.

@silabs-robin silabs-robin marked this pull request as ready for review November 11, 2024 12:26
@silabs-robin
Copy link
Contributor Author

silabs-robin commented Nov 11, 2024

The related changes in openhwgroup/cv32e40s-dv#3 have now been merged, so this PR is ready to be reviewed/merged.

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

Successfully merging this pull request may close these issues.

2 participants