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

Phasar DynLib and LTO #621

Merged
merged 10 commits into from
Jun 5, 2023
Merged

Phasar DynLib and LTO #621

merged 10 commits into from
Jun 5, 2023

Conversation

fabianbs96
Copy link
Member

PhASAR 's codebase is growing continuously which makes it harder for externals to keep an overview, e.g. which libraries should be linked against, which headers to include (and where to find them).

This PR adds convenience headers that include every header from a particular subdirectory (plus one that includes whole phasar).
Additionally, phasar now builds a large library libphasar.a / libphasar.so by default that can be linked against.
The smaller "component-wise" libraries are still there for a more optimized build.

@fabianbs96 fabianbs96 self-assigned this May 21, 2023
@fabianbs96 fabianbs96 added enhancement New feature or request usability labels May 21, 2023
@fabianbs96 fabianbs96 marked this pull request as ready for review May 29, 2023 12:11
@fabianbs96 fabianbs96 requested a review from MMory as a code owner May 29, 2023 12:11
@fabianbs96 fabianbs96 requested review from janniclas and MMory and removed request for MMory May 29, 2023 12:11
Copy link
Member

@MMory MMory left a comment

Choose a reason for hiding this comment

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

Would be good to have a hint somewhere that makes people include their newly added header(s) to the directory include headers. Coding guidelines?

CMakeLists.txt Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
include/phasar.h Outdated Show resolved Hide resolved
@fabianbs96 fabianbs96 changed the title Phasar DynLib Phasar DynLib and LTO May 31, 2023
Copy link
Member

@MMory MMory left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@MMory MMory requested a review from vulder June 1, 2023 16:30
@MMory
Copy link
Member

MMory commented Jun 1, 2023

@vulder does this break vara? just wanna make sure it doesn't before we merge, adding a block for this.

@MMory MMory added the blocked A prerequisite for this ticket prevents it from being worked on label Jun 1, 2023
Copy link
Collaborator

@vulder vulder left a comment

Choose a reason for hiding this comment

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

From a comparability standpoint, this works for us.
PS: I did not fully check the rest of the PR, just if it works in our in-tree setup. 👍

@fabianbs96 fabianbs96 removed the blocked A prerequisite for this ticket prevents it from being worked on label Jun 5, 2023
@fabianbs96 fabianbs96 merged commit 4f568a7 into development Jun 5, 2023
@fabianbs96 fabianbs96 deleted the f-PhasarDynLib branch June 5, 2023 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request usability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants