Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants