-
Notifications
You must be signed in to change notification settings - Fork 660
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
Explicit assign the symbol visibility #1760
Conversation
This modification is based on https://gcc.gnu.org/wiki/Visibility. In this change, export entries are explicitly marked as default visibility. If put -fvisibility=hidden to CXXFLAGS, dxcompiler binary size reduces for ~25%.
@@ -13,8 +13,18 @@ | |||
#ifndef __DXC_API__ | |||
#define __DXC_API__ | |||
|
|||
#ifndef DXC_API_IMPORT | |||
#define DXC_API_IMPORT __declspec(dllimport) | |||
#ifdef _WIN32 |
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.
Don't you need compiler-specific switches here? Do all windows compilers support __declspec(dll*)
and all non-windows compilers support __attribute__((visibility))
?
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.
MinGW and clang on Windows also supports __declspec(dll*). So it become a platform thing.
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.
The stated gains are only when using GCC, right?
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.
Build by both gcc and clang can benefit from this. For gcc, the libdxcompiler.so drops from 28M to 22M. For clang, it's from 27M to 20M. They still larger than build by msvc on Windows, which is 17M.
@@ -13,8 +13,18 @@ | |||
#ifndef __DXC_API__ | |||
#define __DXC_API__ | |||
|
|||
#ifndef DXC_API_IMPORT | |||
#define DXC_API_IMPORT __declspec(dllimport) | |||
#ifdef _WIN32 |
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.
MinGW and clang on Windows also supports __declspec(dll*). So it become a platform thing.
The full change, including changing the default compiler flags to hidden, was merged as part of #2213. So we can close this. |
This modification is based on https://gcc.gnu.org/wiki/Visibility. In this change, export entries are explicitly marked as default visibility. If put -fvisibility=hidden to CXXFLAGS, dxcompiler binary size reduces for ~25%.