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

SymCC Runtime Integration #55

Merged
merged 19 commits into from
Apr 9, 2024
Merged

Conversation

rmalmain
Copy link
Collaborator

@rmalmain rmalmain commented Apr 4, 2024

Now that SymCC and Symcc Runtime are being decorrelated, we can get rid of the SymCC dependency and replace it with SymCC Runtime.
It comes with nice side features, like the automatic building of symcc-rt with meson / cmake and the full integration with the dependency system of meson, removing some ugly building code.

There is also:

  • An option to change symcc-rt's backend.
  • An option to compile symcc-rt statically or dynamically.

What remains to fix:

  • The Dockerfile has not been tested. It will most likely not work before some minor modifications
  • Problem of linking when choosing the simple backend for symcc-rt. It must come from symcc-rt not declaring correctly some dependencies in the corresponding CMakeLists.txt, the solution should not be too hard to fix.

Compagnon PRs: SymCC PR, SymCC Runtime PR

* Removed options to located symcc.
* Automatic building of symcc-rt.
* Clean integration with meson using dependencies.
* Option to change symcc rt backend.
* Option to compile symcc rt statically and dynamically.
@@ -1,5 +1,5 @@
project('qemu', ['c'], meson_version: '>=0.63.0',
default_options: ['warning_level=1', 'c_std=gnu11', 'cpp_std=gnu++11', 'b_colorout=auto',
default_options: ['warning_level=1', 'c_std=gnu11', 'cpp_std=gnu++17', 'b_colorout=auto',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is it ok to keep this? I couldn't get rid of this modification in the end

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it required because the SymCC runtime uses C++17? Fine for me to keep it 🤷‍♂️

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should not be necessary in theory.
Subprojects in meson should not rely on the options of the root project by default. I suspect the parsing of cmake by meson is doing something wrong, I will check

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

After investigation, I don't think we can do much about this.
Apparently, a few things are being hardcoded in the default options of the meson file generated from the cmake project that cannot be avoided (https://github.com/mesonbuild/meson/blob/master/mesonbuild/cmake/interpreter.py).
Maybe I missed something. If someone finds a way, we can change. Otherwise, I'll keep things that way.

.gitmodules Outdated
[submodule "subprojects/symcc-rt"]
path = subprojects/symcc-rt
url = https://github.com/eurecom-s3/symcc-rt.git
branch = export_symcc_runtime
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be main?

Copy link
Member

Choose a reason for hiding this comment

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

Once merged I guess yes.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@@ -1,5 +1,5 @@
project('qemu', ['c'], meson_version: '>=0.63.0',
default_options: ['warning_level=1', 'c_std=gnu11', 'cpp_std=gnu++11', 'b_colorout=auto',
default_options: ['warning_level=1', 'c_std=gnu11', 'cpp_std=gnu++17', 'b_colorout=auto',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it required because the SymCC runtime uses C++17? Fine for me to keep it 🤷‍♂️

meson.build Outdated Show resolved Hide resolved
@aurelf aurelf merged commit 45b4700 into eurecom-s3:master Apr 9, 2024
1 check passed
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.

3 participants