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

Update README for new CMake build system #756

Open
Alasdair opened this issue Feb 26, 2025 · 6 comments
Open

Update README for new CMake build system #756

Alasdair opened this issue Feb 26, 2025 · 6 comments

Comments

@Alasdair
Copy link
Collaborator

I think the README needs to be updated for the new CMake build system as says:

make -C build riscv_sim_rv64f_rvfi

but this doesn't work until you've run CMake at least to get the build directory. I think the intent behind the current wording is to run the build_simulators.sh script first, but I don't think this is obvious enough.

@Timmmm
Copy link
Collaborator

Timmmm commented Feb 26, 2025

Hmm I couldn't think of a good way to word this so what if we just change the code block to this?

$ ./build_simulators.sh
$ make -C build riscv_sim_rv64f_rvfi

@jordancarlin
Copy link
Collaborator

jordancarlin commented Feb 26, 2025

What about having the first line directly invoke cmake to avoid unnecessarily building the standard simulators first?

@Timmmm
Copy link
Collaborator

Timmmm commented Feb 26, 2025

That gets a bit complex for a first command the user ever runs maybe...

mkdir -p build
cmake -S . -B build -DCMAKE_BUILD_TYPE=RelWithDebInfo -DDOWNLOAD_GMP=TRUE
make -C build riscv_sim_rv64f_rvfi

@jordancarlin
Copy link
Collaborator

The mkdircommand shouldn’t be needed (cmake creates that directory automatically).

That gets a bit complex for a first command the user ever runs maybe...

I assume most users won’t be using the rvfi version, so not sure that this is a huge concern. Also, once we have the pre-compiled releases I think we should change this section to direct users to those and have a separate developers (or something along those lines) section that explains how to actually build the model. Most people should not need to build from source themselves.

@Timmmm
Copy link
Collaborator

Timmmm commented Feb 26, 2025

So this?

cmake -S . -B build -DCMAKE_BUILD_TYPE=RelWithDebInfo -DDOWNLOAD_GMP=TRUE
make -C build riscv_sim_rv64f_rvfi

I'm ok with that if you guys think it is better.

@jordancarlin
Copy link
Collaborator

jordancarlin commented Feb 26, 2025

So this?

cmake -S . -B build -DCMAKE_BUILD_TYPE=RelWithDebInfo -DDOWNLOAD_GMP=TRUE
make -C build riscv_sim_rv64f_rvfi

That’s what I was thinking. I think we should definitely go with one of the options discussed in this thread, and this one would be my preference to avoid redundant/unnecessary compilation, but I’d be fine with either.

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

No branches or pull requests

3 participants