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

Switch to CMake #551

Open
Timmmm opened this issue Sep 19, 2024 · 13 comments
Open

Switch to CMake #551

Timmmm opened this issue Sep 19, 2024 · 13 comments

Comments

@Timmmm
Copy link
Collaborator

Timmmm commented Sep 19, 2024

I think we should switch from Make to CMake. This would have a number of benefits:

  1. We can easily make the arch part of the targets instead of a Make parameter. I.e. instead of
ARCH=RV32 make c_emulator/riscv_sim_RV32
ARCH=RV64 make c_emulator/riscv_sim_RV64

you can do

make -j2 c_emulator/riscv_sim_RV32 c_emulator/riscv_sim_RV64

which is less redundant and confusing, and also easier to do in parallel (this would half CI time).

  1. We can use CTest. This allows running tests in parallel, uploading JUnit results to Github, etc.
  2. CMake - while pretty shit on an absolute scale - is much nicer to use than Make. E.g. generating a compile_commands.json for IDEs is one line, as is enabling LTO, PIC, etc. It supports splitting your build config into multiple files in a sane way.
  3. CMake is pretty much a de facto standard at this point and has native support from lots of IDEs.
@jrtc27
Copy link
Collaborator

jrtc27 commented Sep 19, 2024

You can do 1 already in Make if you write the file properly, we have that in sail-cheri-riscv.

@jrtc27
Copy link
Collaborator

jrtc27 commented Sep 19, 2024

3 and 4 don’t seem very relevant, there’s no IDE for Sail, and the C is extremely basic, it doesn’t warrant an IDE.

@jrtc27
Copy link
Collaborator

jrtc27 commented Sep 19, 2024

That leaves 2, for which using CMake seems a bit overkill?

@Timmmm
Copy link
Collaborator Author

Timmmm commented Sep 19, 2024

You can do 1 already in Make if you write the file properly, we have that in sail-cheri-riscv.

Ah you mean this? We should do that in this repo if we don't move to CMake at least. Still it is definitely easier in CMake (just do a loop over the architectures).

the C is extremely basic, it doesn’t warrant an IDE.

I would like to use an IDE with it please! I don't feel like I should not have nice tools just because it's only 2k lines of code (so far).

That leaves 2, for which using CMake seems a bit overkill?

I still think the other benefits are significant. We've been using CMake internally and it's a much nicer experience. Are there any downsides (apart from having to run cmake)?

@arichardson
Copy link
Collaborator

I also think this would be nice.

CTest is a bit awkward to use but having a real test driver is definitely worthwhile.

I also use CLion for the sail C code and it has basic syntax highlighting for sail if you import the sail syntax files (https://github.com/rems-project/sail?tab=readme-ov-file#editor-support).

I think point 2 is a strong enough reason and I am definitely supportive of this change.

@moste00
Copy link

moste00 commented Oct 6, 2024

Just chiming in to suggest that - if we're going to pay the full cost of transitioning anyway- we should probably transition to Bazel instead of CMake.

Advantages over CMake (and Make):

(1) Newer by about 1.5 decade or more than CMake, which means less Legacy cruft (e.g. the whole "legacy" CMake and "modern" CMake thing). Most importantly, it also means the build system doesn't contain whole classes of bugs just by the sheer virtue of being written in Java rather than C++.

(2) Developed for huge monorepos, Bazel has caching - even remote caching - as a central concern. This is good because sail-riscv is relatively huge as far as Sail projects go, it takes about 40 seconds or so for a call to Frontend.load_files to finish when called on the full list of files included by SAIL_SRCS on my somewhat ancient machine, although it takes only about 20 on a Github runner. Bazel could have better caching logic than either Make or CMake. (It's definitely better and more correct than Make, which naively relies on file timestamps as far as I remember.)

(3) Unlike CMake and Make, its build language does not aspire to be a stringly-typed shell language, it's just a (tiny) subset of python, with plenty of editor support out there.

(4) With the core written in Java and a launch wrapper like Bazelisk written in Go (downloadable as a single static binary), I believe Bazel is much easier to get up and running on a whole new machine than CMake, which - if I remember correctly - requires a C++ compiler and a compilation from source and some other inscrutable "toolchain" fiddling.

To elaborate on (4), I have worked on a project with CMake and another (massively larger, and multi-lingual) project using Bazel. CMake required the usual 1-2 days of dependency song-and-dance, while Bazel only required a JVM on the PATH and (optionally) the Bazelisk wrapper. It's accurate to say this is 2 days vs 2 hours.

There are real disadvantages to Bazel: it might be the case (I haven't measured) that the JVM-based tool uses more memory and maybe even CPU. Bazel sometimes also over-correct in the direction of "Hermetic builds", for example taking the hash of the toolchain itself (which would be the Sail compiler and maybe even the OCaml compiler, in our case) so it can gaurantee correct caching as much as possible. Some people see this as overkill and over-engineering.

I personally think the advantages of Bazel outweigh its few disadvantages.

Sorry if this is derailing to the conversation, I saw the issue's author say upfront that CMake is not ideal, so I just wanted to toss the idea that we're not hostages to CMake out in the open.

@rsnikhil
Copy link
Collaborator

rsnikhil commented Oct 6, 2024

Is Basel good for builds on multiple platforms (Various Unix, MacOS, Windows)?
(This is one of the advantages often cited for moving from Make to CMake).

@Timmmm
Copy link
Collaborator Author

Timmmm commented Oct 6, 2024

I also agree Bazel is probably the more correct option. However I have had very little success convincing colleagues to use it at two different companies even we inevitably ran into exactly the issues that I said we would if we didn't use Bazel - mainly (but not exclusively):

  • Accidentally not declaring dependencies leading to random build failures (one of these was so hard to debug we never fixed it)
  • CI taking forever because you have to run everything even if you only edited the readme.

Given that this project is very small and simple to compile, and it also has a few old school Make+C guys... maybe Bazel is a step too far for them.

But I would love to be wrong about that; if everyone else is happy with Bazel (or Buck2) I would be very keen.

Maybe we should prototype both and see what people think?

@Timmmm
Copy link
Collaborator Author

Timmmm commented Oct 6, 2024

Is Basel good for builds on multiple platforms (Various Unix, MacOS, Windows)?

Yeah it works on those platforms. It can also deal with dependencies a lot better than CMake, so we could get to a point where the instructions for building the Sail model on any platform are:

  1. Download Bazelisk binary for your platform and add it to PATH.
  2. bazelisk build //...

That's it; it will download zlib, libgmp, and the Sail compiler automatically.

@Alasdair
Copy link
Collaborator

Alasdair commented Oct 7, 2024

I think my PR #572 fixes a lot of issues with the current Makefile. Not that this should be taken as an argument in favour of make, I have no particular attachment to it as a tool. I think the only real argument for it is that it's basically already pre-installed everywhere.

(2) Developed for huge monorepos, Bazel has caching - even remote caching - as a central concern. This is good because sail-riscv is relatively huge as far as Sail projects go, it takes about 40 seconds or so for a call to Frontend.load_files to finish when called on the full list of files included by SAIL_SRCS on my somewhat ancient machine, although it takes only about 20 on a Github runner. Bazel could have better caching logic than either Make or CMake. (It's definitely better and more correct than Make, which naively relies on file timestamps as far as I remember.)

The current compilation strategy for Sail is quite unfriendly towards speeding up the build using caching unfortunately. We are someone unique in that we have a lot of different compilation targets, and to make this work the 'linking' step (where we turn all the files into a single model) happens very early in the frontend, rather than right at the end in a traditional compiler. The internal data structures we use also depend heavily on sharing, so they aren't very serialisation friendly. Right now the only thing we cache are the hashes of the SMT queries the type-checker makes (which are removed by make clean, so make clean && make is a bit of a foot-gun when you want make -B). I had the build time down to about 5s on my machine which I was quite happy with but if people are dealing with 40s I can probably look into some additional optimisations - there is still quite a bit of low hanging fruit performance wise.

@jrtc27
Copy link
Collaborator

jrtc27 commented Oct 7, 2024

Bazel is awful and way too overkill for a simple project like this. Please no.

@Timmmm
Copy link
Collaborator Author

Timmmm commented Oct 7, 2024

The current compilation strategy for Sail is quite unfriendly towards speeding up the build using caching unfortunately.

I was more thinking about the caching of the whole process. We do this internally (via hacky Python, rather than Bazel) and it works really well - you don't even need the Sail compiler if you don't change the Sail code, which is really the reason I did it.

I would say the Sail compilation process is fast enough that it isn't worth internal caching at the moment. Most of the time you're fixing type errors and those seem to happen early on, and anyway the vast amount of time is spent compiling the generated C which I think is probably difficult to improve without drastic rearchitecting.

Bazel is awful and way too overkill for a simple project like this. Please no.

It solves real problems... But I would be inclined to agree it's overkill here. What do you find awful about it out of interest?

Another advantage of CMake I forgot to mention is that it is the most popular C++ build system so it makes it easier to include this projects in other people's projects. They could just do

add_directory(sail-riscv)

and get access to all the build targets (not that useful now but in future we should expose the emulator as a library).

@Timmmm
Copy link
Collaborator Author

Timmmm commented Dec 18, 2024

Draft PR: #647

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

6 participants