This is an archive of the discontinued LLVM Phabricator instance.

Differential D12403

Fix GCC warning 'always_inline function might not be inlinable'
ClosedPublic

Authored by Hahnfeld on Aug 27 2015, 4:07 AM.

Details

Reviewers
jlpeyton
AndreyChurbanov
Summary

The function has to be marked inline which wasn't the case if STRICT_ANSI.

Compile tested with GCC 4.9 and 5.2, Intel 15.0 and clang 3.8.0 (from trunk). Now builds completely warning free in the default configuration.

Diff Detail

Repository
rL LLVM

Event Timeline

Hahnfeld updated this revision to Diff 33310.Aug 27 2015, 4:07 AM
Hahnfeld retitled this revision from to Fix GCC warning 'always_inline function might not be inlinable'.
Hahnfeld updated this object.
Hahnfeld added a reviewer: jlpeyton.
Hahnfeld set the repository for this revision to rL LLVM.
Hahnfeld added a subscriber: openmp-commits.
AndreyChurbanov added a subscriber: AndreyChurbanov.EditedAug 28 2015, 8:17 AM

Jonas,

I talked to the authors of this code (Intel Tools team) and they suggested to fix the problem in a slightly different way. Could you please check if the following diff solves it? And if yes, I would prefer we follow their change, so that we are in sync with other places (repositories) with ittnotify sources.

Thanks, Andrey

Index: runtime/src/thirdparty/ittnotify/ittnotify.h
===================================================================
--- runtime/src/thirdparty/ittnotify/ittnotify.h	(revision 246303)
+++ runtime/src/thirdparty/ittnotify/ittnotify.h	(working copy)
@@ -179,10 +179,11 @@
  */
 #ifdef __STRICT_ANSI__
 #define ITT_INLINE           static
+#define ITT_INLINE_ATTRIBUTE __attribute__((unused))
 #else  /* __STRICT_ANSI__ */
 #define ITT_INLINE           static inline
+#define ITT_INLINE_ATTRIBUTE __attribute__((always_inline, unused))
 #endif /* __STRICT_ANSI__ */
-#define ITT_INLINE_ATTRIBUTE __attribute__ ((always_inline, unused))
 #endif /* ITT_PLATFORM==ITT_PLATFORM_WIN */
 /** @endcond */
 
Index: runtime/src/thirdparty/ittnotify/ittnotify_config.h
===================================================================
--- runtime/src/thirdparty/ittnotify/ittnotify_config.h	(revision 246303)
+++ runtime/src/thirdparty/ittnotify/ittnotify_config.h	(working copy)
@@ -113,10 +113,11 @@
  */
 #ifdef __STRICT_ANSI__
 #define ITT_INLINE           static
+#define ITT_INLINE_ATTRIBUTE __attribute__((unused))
 #else  /* __STRICT_ANSI__ */
 #define ITT_INLINE           static inline
+#define ITT_INLINE_ATTRIBUTE __attribute__((always_inline, unused))
 #endif /* __STRICT_ANSI__ */
-#define ITT_INLINE_ATTRIBUTE __attribute__ ((always_inline, unused))
 #endif /* ITT_PLATFORM==ITT_PLATFORM_WIN */
 /** @endcond */
 
Index: runtime/src/thirdparty/ittnotify/legacy/ittnotify.h
===================================================================
--- runtime/src/thirdparty/ittnotify/legacy/ittnotify.h	(revision 246303)
+++ runtime/src/thirdparty/ittnotify/legacy/ittnotify.h	(working copy)
@@ -118,10 +118,11 @@
  */
 #ifdef __STRICT_ANSI__
 #define ITT_INLINE           static
+#define ITT_INLINE_ATTRIBUTE __attribute__((unused))
 #else  /* __STRICT_ANSI__ */
 #define ITT_INLINE           static inline
+#define ITT_INLINE_ATTRIBUTE __attribute__((always_inline, unused))
 #endif /* __STRICT_ANSI__ */
-#define ITT_INLINE_ATTRIBUTE __attribute__ ((always_inline, unused))
 #endif /* ITT_PLATFORM==ITT_PLATFORM_WIN */
 /** @endcond */
Hahnfeld added a comment.Aug 31 2015, 4:13 AM
In D12403#235158, @AndreyChurbanov wrote:

Jonas,

I talked to the authors of this code (Intel Tools team) and they suggested to fix the problem in a slightly different way. Could you please check if the following diff solves it? And if yes, I would prefer we follow their change, so that we are in sync with other places (repositories) with ittnotify sources.

Thanks, Andrey

It works and I'm fine with this as well although I don't really understand why there shouldn't be inlining with __STRICT_ANSI__ being defined...

Thanks,
Jonas

AndreyChurbanov added a comment.Aug 31 2015, 5:25 AM

Jonas,

although I don't really understand why there shouldn't be inlining with STRICT_ANSI being defined...

The GCC documentation for -ansi option says that "For the C compiler, it disables recognition of C++ style ‘//’ comments as well as the inline keyword."

So the way it is written now there may be problems with C compilation with -ansi option. The ittnotify team told me they had complaints from at least two customers about the inline keyword while -ansi used, so it is better to avoid using it.

Thanks,
Andrey

Hahnfeld added a comment.Aug 31 2015, 5:29 AM
In D12403#236292, @AndreyChurbanov wrote:

Jonas,

although I don't really understand why there shouldn't be inlining with STRICT_ANSI being defined...

The GCC documentation for -ansi option says that "For the C compiler, it disables recognition of C++ style ‘//’ comments as well as the inline keyword."

So the way it is written now there may be problems with C compilation with -ansi option. The ittnotify team told me they had complaints from at least two customers about the inline keyword while -ansi used, so it is better to avoid using it.

Thanks,
Andrey

Thanks for this explanation. Then your diff is without doubt preferable and LGTM.

Greetings,
Jonas

Diffusion mentioned this in rL246434: D12403: Fix GCC warning 'always_inline function might not be inlinable'.Aug 31 2015, 5:34 AM
AndreyChurbanov accepted this revision.Aug 31 2015, 5:46 AM
AndreyChurbanov added a reviewer: AndreyChurbanov.
This revision is now accepted and ready to land.Aug 31 2015, 5:46 AM
AndreyChurbanov closed this revision.Aug 31 2015, 5:46 AM

Revision Contents

PathSize
runtime/
src/
thirdparty/
ittnotify/
4 lines
4 lines
legacy/
4 lines

Diff 33310

runtime/src/thirdparty/ittnotify/ittnotify.h

Show First 20 Lines • Show All 171 Lines • ▼ Show 20 Lines
#define ITT_INLINE __forceinline #define ITT_INLINE __forceinline
#define ITT_INLINE_ATTRIBUTE /* nothing */ #define ITT_INLINE_ATTRIBUTE /* nothing */
#else /* ITT_PLATFORM==ITT_PLATFORM_WIN */ #else /* ITT_PLATFORM==ITT_PLATFORM_WIN */
/* /*
* Generally, functions are not inlined unless optimization is specified. * Generally, functions are not inlined unless optimization is specified.
* For functions declared inline, this attribute inlines the function even * For functions declared inline, this attribute inlines the function even
* if no optimization level was specified. * if no optimization level was specified.
*/ */
#ifdef __STRICT_ANSI__
#define ITT_INLINE static
#else /* __STRICT_ANSI__ */
#define ITT_INLINE static inline #define ITT_INLINE static inline
#endif /* __STRICT_ANSI__ */
#define ITT_INLINE_ATTRIBUTE __attribute__ ((always_inline, unused)) #define ITT_INLINE_ATTRIBUTE __attribute__ ((always_inline, unused))
#endif /* ITT_PLATFORM==ITT_PLATFORM_WIN */ #endif /* ITT_PLATFORM==ITT_PLATFORM_WIN */
/** @endcond */ /** @endcond */
#ifdef INTEL_ITTNOTIFY_ENABLE_LEGACY #ifdef INTEL_ITTNOTIFY_ENABLE_LEGACY
# if ITT_PLATFORM==ITT_PLATFORM_WIN # if ITT_PLATFORM==ITT_PLATFORM_WIN
# pragma message("WARNING!!! Deprecated API is used. Please undefine INTEL_ITTNOTIFY_ENABLE_LEGACY macro") # pragma message("WARNING!!! Deprecated API is used. Please undefine INTEL_ITTNOTIFY_ENABLE_LEGACY macro")
# else /* ITT_PLATFORM==ITT_PLATFORM_WIN */ # else /* ITT_PLATFORM==ITT_PLATFORM_WIN */
▲ Show 20 Lines • Show All 3,611 Lines • Show Last 20 Lines

runtime/src/thirdparty/ittnotify/ittnotify_config.h

Show First 20 Lines • Show All 105 Lines • ▼ Show 20 Lines
#define ITT_INLINE __forceinline #define ITT_INLINE __forceinline
#define ITT_INLINE_ATTRIBUTE /* nothing */ #define ITT_INLINE_ATTRIBUTE /* nothing */
#else /* ITT_PLATFORM==ITT_PLATFORM_WIN */ #else /* ITT_PLATFORM==ITT_PLATFORM_WIN */
/* /*
* Generally, functions are not inlined unless optimization is specified. * Generally, functions are not inlined unless optimization is specified.
* For functions declared inline, this attribute inlines the function even * For functions declared inline, this attribute inlines the function even
* if no optimization level was specified. * if no optimization level was specified.
*/ */
#ifdef __STRICT_ANSI__
#define ITT_INLINE static
#else /* __STRICT_ANSI__ */
#define ITT_INLINE static inline #define ITT_INLINE static inline
#endif /* __STRICT_ANSI__ */
#define ITT_INLINE_ATTRIBUTE __attribute__ ((always_inline, unused)) #define ITT_INLINE_ATTRIBUTE __attribute__ ((always_inline, unused))
#endif /* ITT_PLATFORM==ITT_PLATFORM_WIN */ #endif /* ITT_PLATFORM==ITT_PLATFORM_WIN */
/** @endcond */ /** @endcond */
#ifndef ITT_ARCH_IA32 #ifndef ITT_ARCH_IA32
# define ITT_ARCH_IA32 1 # define ITT_ARCH_IA32 1
#endif /* ITT_ARCH_IA32 */ #endif /* ITT_ARCH_IA32 */
▲ Show 20 Lines • Show All 351 Lines • Show Last 20 Lines

runtime/src/thirdparty/ittnotify/legacy/ittnotify.h

Show First 20 Lines • Show All 110 Lines • ▼ Show 20 Lines
#define ITT_INLINE __forceinline #define ITT_INLINE __forceinline
#define ITT_INLINE_ATTRIBUTE /* nothing */ #define ITT_INLINE_ATTRIBUTE /* nothing */
#else /* ITT_PLATFORM==ITT_PLATFORM_WIN */ #else /* ITT_PLATFORM==ITT_PLATFORM_WIN */
/* /*
* Generally, functions are not inlined unless optimization is specified. * Generally, functions are not inlined unless optimization is specified.
* For functions declared inline, this attribute inlines the function even * For functions declared inline, this attribute inlines the function even
* if no optimization level was specified. * if no optimization level was specified.
*/ */
#ifdef __STRICT_ANSI__
#define ITT_INLINE static
#else /* __STRICT_ANSI__ */
#define ITT_INLINE static inline #define ITT_INLINE static inline
#endif /* __STRICT_ANSI__ */
#define ITT_INLINE_ATTRIBUTE __attribute__ ((always_inline, unused)) #define ITT_INLINE_ATTRIBUTE __attribute__ ((always_inline, unused))
#endif /* ITT_PLATFORM==ITT_PLATFORM_WIN */ #endif /* ITT_PLATFORM==ITT_PLATFORM_WIN */
/** @endcond */ /** @endcond */
/** @cond exclude_from_documentation */ /** @cond exclude_from_documentation */
/* Helper macro for joining tokens */ /* Helper macro for joining tokens */
#define ITT_JOIN_AUX(p,n) p##n #define ITT_JOIN_AUX(p,n) p##n
#define ITT_JOIN(p,n) ITT_JOIN_AUX(p,n) #define ITT_JOIN(p,n) ITT_JOIN_AUX(p,n)
▲ Show 20 Lines • Show All 840 Lines • Show Last 20 Lines