Export functions without def file #303
Conversation
|
@isuruf The functions you removed from |
|
Ready for review.
After this PR is merged, |
Will do. Thanks Isuru.
I think you are suggesting that we can now adjust the Also, do you know if that compiler option is recognized by all of: gcc, clang, and icc? If we need to use a different option for clang or icc (or both), that's fine. I just need to (eventually) know what the equivalent option(s) is/are.
Got it. I'll add this to my near-term to-do list, though I have a lot on my plate in the next couple days, so it might not be until early next week that I can make significant progress on this. Also, if you don't mind, I'll wait on merging this PR until I can remove the macros from the internal APIs (I'll push to the PR branch). That way, you can also easily review/comment on it before I merge. |
A user can easily pass the
I know that gcc and clang on unix do. (clang doesn't on windows). Not sure about icc.
Sure. I'm not in a hurry. |
Understood. I'll probably make it a |
|
@isuruf I was taking a closer look at the use of Can you briefly explain which one, prototypes or definitions, the macro must be used in front of in order to have the desired effect vis-a-vis hiding/exposing symbols in shared libraries? |
|
From prototypes I assume you mean function declaration. The macro should be added to the declaration and not the definition. If you can point out the definitions I have used the macro on, I'll fix them |
|
@isuruf Yes, I mean declaration. Not sure which term is in vogue these days; I call them prototypes since that was what they were called when I learned C. :) You can find the affected source files by executing It may also be good to do the same thing but for |
|
@isuruf Your removal of |
|
@isuruf I am now going to go through all of the remaining occurrences of |
|
Thanks @fgvanzee |
|
Interesting observation: Because @isuruf wrote "Fixes #248" in the initial issue posting, merging this PR automatically closed #248. That seems like a generally smart and useful feature in GitHub, though not necessarily for those of us who start out oblivious to the effects of those magic words. :) It took me a little bit of digging to figure out what had happened. Did you know about this, Isuru? |
Details: - After merging PR #303, at Isuru's request, I removed the use of BLIS_EXPORT_BLIS from all function prototypes *except* those that we potentially wish to be exported in shared/dynamic libraries. In other words, I removed the use of BLIS_EXPORT_BLIS from all prototypes of functions that can be considered private or for internal use only. This is likely the last big modification along the path towards implementing the functionality spelled out in issue #248. Thanks again to Isuru Fernando for his initial efforts of sprinkling the export macros throughout BLIS, which made removing them where necessary relatively painless. Also, I'd like to thank Tony Kelman, Nathaniel Smith, Ian Henriksen, Marat Dukhan, and Matthew Brett for participating in the initial discussion in issue #37 that was later summarized and restated in issue #248. - CREDITS file update.
|
Thanks. 5a5f494 looks good to me. Maybe something in docs asking the user to add Yes, the fixes word was intentional. Closes and resolves also work. |
|
Thanks for those pro-tips. I'll work on something for the docs. Or, perhaps we should open a new issue to discuss whether |
* Revert "restore bli_extern_defs exporting for now" This reverts commit 09fb07c. * Remove symbols not intended to be public * No need of def file anymore * Fix whitespace * No need of configure option * Remove export macro from definitions * Remove blas export macro from definitions
Details: - After merging PR flame#303, at Isuru's request, I removed the use of BLIS_EXPORT_BLIS from all function prototypes *except* those that we potentially wish to be exported in shared/dynamic libraries. In other words, I removed the use of BLIS_EXPORT_BLIS from all prototypes of functions that can be considered private or for internal use only. This is likely the last big modification along the path towards implementing the functionality spelled out in issue flame#248. Thanks again to Isuru Fernando for his initial efforts of sprinkling the export macros throughout BLIS, which made removing them where necessary relatively painless. Also, I'd like to thank Tony Kelman, Nathaniel Smith, Ian Henriksen, Marat Dukhan, and Matthew Brett for participating in the initial discussion in issue flame#37 that was later summarized and restated in issue flame#248. - CREDITS file update.
* Revert "restore bli_extern_defs exporting for now" This reverts commit 09fb07c. * Remove symbols not intended to be public * No need of def file anymore * Fix whitespace * No need of configure option * Remove export macro from definitions * Remove blas export macro from definitions
Details: - After merging PR flame#303, at Isuru's request, I removed the use of BLIS_EXPORT_BLIS from all function prototypes *except* those that we potentially wish to be exported in shared/dynamic libraries. In other words, I removed the use of BLIS_EXPORT_BLIS from all prototypes of functions that can be considered private or for internal use only. This is likely the last big modification along the path towards implementing the functionality spelled out in issue flame#248. Thanks again to Isuru Fernando for his initial efforts of sprinkling the export macros throughout BLIS, which made removing them where necessary relatively painless. Also, I'd like to thank Tony Kelman, Nathaniel Smith, Ian Henriksen, Marat Dukhan, and Matthew Brett for participating in the initial discussion in issue flame#37 that was later summarized and restated in issue flame#248. - CREDITS file update.
* Revert "restore bli_extern_defs exporting for now" This reverts commit 09fb07c. * Remove symbols not intended to be public * No need of def file anymore * Fix whitespace * No need of configure option * Remove export macro from definitions * Remove blas export macro from definitions
Details: - After merging PR flame#303, at Isuru's request, I removed the use of BLIS_EXPORT_BLIS from all function prototypes *except* those that we potentially wish to be exported in shared/dynamic libraries. In other words, I removed the use of BLIS_EXPORT_BLIS from all prototypes of functions that can be considered private or for internal use only. This is likely the last big modification along the path towards implementing the functionality spelled out in issue flame#248. Thanks again to Isuru Fernando for his initial efforts of sprinkling the export macros throughout BLIS, which made removing them where necessary relatively painless. Also, I'd like to thank Tony Kelman, Nathaniel Smith, Ian Henriksen, Marat Dukhan, and Matthew Brett for participating in the initial discussion in issue flame#37 that was later summarized and restated in issue flame#248. - CREDITS file update.
@fgvanzee, can you confirm that the functions removed from libblis-symbols.def file are not intended to be public API?
Fixes #248