-
Notifications
You must be signed in to change notification settings - Fork 399
[WIP] Try to support header only mode for core library. #2832
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
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
369cf59 to
a907356
Compare
1. Added the `inline` keyword to all functions in the `.cc` files. 2. Changed all anonymous namespaces to a named namespace (`detail`) in the `.cc` files. 3. Changed static class field to inline global variable. 4. Included the `.cc` file from its corresponding header file. An `INTERFACE` library has been added.
a907356 to
6e8b293
Compare
jan-wassenberg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! I could imagine landing a change like this. Some comments:
| } // namespace detail | ||
|
|
||
| HWY_HEADER_ONLY_FUN | ||
| HWY_DLLEXPORT int Unpredictable1() { return timer::Start() != ~0ULL; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worthwhile to merge these into a HWY_HEADER_ONLY_EXPORT?
This is a super WIP (Work in Progress) attempt, shared for discussion purposes. Hope it's not too disruptive.
Code Changes
To enable header-only mode, the following changes have been made:
inlinekeyword to all functions in the.ccfiles.detail) in the.ccfiles..ccfile from its corresponding header file.CMake Changes
An
INTERFACElibrary has been added.TODO
pre_target.ccfile is not yet completed, as it relies onfor_each.hwith some magical macros 😸.HWY_HEADER_ONLY_FUNmacro seems a bit clumsy; perhaps just using theinlinekeyword would be better.References