Skip to content

Use compiler-specific WINRT_IMPL_NOINLINE macro for noinline functions #893

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.

Sign up for GitHub

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

Merged
kennykerr merged 1 commit into from
Mar 18, 2021
Merged

Use compiler-specific WINRT_IMPL_NOINLINE macro for noinline functions #893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions strings/base_activation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace winrt::impl


template <bool isSameInterfaceAsIActivationFactory>
__declspec(noinline) hresult get_runtime_activation_factory_impl(param::hstring const& name, winrt::guid const& guid, void** result) noexcept
WINRT_IMPL_NOINLINE hresult get_runtime_activation_factory_impl(param::hstring const& name, winrt::guid const& guid, void** result) noexcept
{
if (winrt_activation_handler)
{
Expand Down Expand Up @@ -339,7 +339,7 @@ namespace winrt::impl
struct factory_cache_entry : factory_cache_entry_base
{
template <typename F>
__declspec(noinline) auto call(F&& callback)
WINRT_IMPL_NOINLINE auto call(F&& callback)
{
#ifdef WINRT_DIAGNOSTICS
get_diagnostics_info().add_factory<Class>();
Expand Down
2 changes: 1 addition & 1 deletion strings/base_com_ptr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ WINRT_EXPORT namespace winrt
}
}

__declspec(noinline) void unconditional_release_ref() noexcept
WINRT_IMPL_NOINLINE void unconditional_release_ref() noexcept
{
std::exchange(m_ptr, {})->Release();
}
Expand Down
6 changes: 3 additions & 3 deletions strings/base_error.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ WINRT_EXPORT namespace winrt
hresult_canceled(take_ownership_from_abi_t) noexcept : hresult_error(impl::error_canceled, take_ownership_from_abi) {}
};

[[noreturn]] inline __declspec(noinline) void throw_hresult(hresult const result)
[[noreturn]] inline WINRT_IMPL_NOINLINE void throw_hresult(hresult const result)
{
if (winrt_throw_hresult_handler)
{
Expand Down Expand Up @@ -513,7 +513,7 @@ WINRT_EXPORT namespace winrt
throw hresult_error(result, take_ownership_from_abi);
}

inline __declspec(noinline) hresult to_hresult() noexcept
inline WINRT_IMPL_NOINLINE hresult to_hresult() noexcept
{
if (winrt_to_hresult_handler)
{
Expand Down Expand Up @@ -546,7 +546,7 @@ WINRT_EXPORT namespace winrt
}
}

inline __declspec(noinline) hstring to_message()
inline WINRT_IMPL_NOINLINE hstring to_message()
{
if (winrt_to_message_handler)
{
Expand Down
8 changes: 8 additions & 0 deletions strings/base_macros.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@
#undef _WINDOWS_NUMERICS_END_NAMESPACE_
#endif

#if defined(_MSC_VER)
#define WINRT_IMPL_NOINLINE __declspec(noinline)
#elif defined(__GNUC__)
#define WINRT_IMPL_NOINLINE __attribute__((noinline))
#else
#define WINRT_IMPL_NOINLINE
#endif
Comment on lines +48 to +54
Copy link
Collaborator

@kennykerr kennykerr Mar 17, 2021

Choose a reason for hiding this comment

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

How does this cover Clang? I believe clang already know how to interpret __declspec(noinline). I wouldn't want to degrade Clang inadvertently.

Copy link
Member Author

@oldnewthing oldnewthing Mar 17, 2021

Choose a reason for hiding this comment

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

Clang defines __GNUC__ and supports GNU-style __attribute__((noinline)). Clang has a (non-default) option to support MSVC-style __declspec(noinline) as well. This version removes the need to add the -fdeclspec flag when compiling with clang.

Copy link
Member Author

@oldnewthing oldnewthing Mar 17, 2021

Choose a reason for hiding this comment

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

The breakdown is

Toolchain Special macros __declspec(noinline) __attribute__((noinline))
MSVC _MSC_VER ☑
Clang (default) __GNUC__
__clang__		 ☑
Clang (-fdeclspec) __GNUC__
__clang__		 ☑ ☑
gcc __GNUC__ ☑
icc __GNUC__
__INTEL_COMPILER		 ☑
icx __GNUC__
__INTEL_LLVM_COMPILER		 ☑ ☑

djgpp, ellcc and zapcc are based on one of the above toolchains.

So sniffing for _MSC_VER and __GNUC__ covers all the major toolchains.

Copy link
Collaborator

@kennykerr kennykerr Mar 17, 2021

Choose a reason for hiding this comment

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

Great!


#ifdef __IUnknown_INTERFACE_DEFINED__
#define WINRT_IMPL_IUNKNOWN_DEFINED
#endif
2 changes: 1 addition & 1 deletion strings/base_windows.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ WINRT_EXPORT namespace winrt::Windows::Foundation
}
}

__declspec(noinline) void unconditional_release_ref() noexcept
WINRT_IMPL_NOINLINE void unconditional_release_ref() noexcept
{
std::exchange(m_ptr, {})->Release();
}
Expand Down