Hi Liming,
On 09/27/19 09:46, Liming Gao wrote:
__inline__ attribute will make the functions not be exposed as the
library interface. It will cause CLANG9 compiler fail.
Signed-off-by: Liming Gao <liming.gao@intel.com>
---
MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c | 6 ------
1 file changed, 6 deletions(-)
Did you regression-test this change against GCC48 (for example)?
I can't tell why we have the __inline__'s in the first place. They date
back to historical commit e1f414b6a7d8 ("Import some basic libraries
instances for Mde Packages.", 2007-06-22). And that commit does not
explain __inline__.
If we remove __inline__ for the whole GCC toolchain *family*, then I
think we need a better justification than just "makes CLANG9 fail".
[Liming] I verify GCC5 tool chain. I will verify GCC48/GCC49 and XCODE5.
I don’t know the specific reason about __inline__. If there is no impact on
other GCC tool chain, I prefer to remove them.
Looks like __inline__ is the C89 version of inline.
I'm kind of surprised clang with LTO would just not ignore the inline, but then I came across....
"gnu89 semantics of inline and extern
inline are essentially the exact opposite of those in C99[4],
with the exception that gnu89 permits redefinition of an extern inline function
as an unqualified function, while C99 inline does
not[5].
Thus, gnu89 extern inline without
redefinition is like C99 inline,
and gnu89 inline is
like C99 extern inline;
in other words, in gnu89, a function defined inline will
always and a function defined extern inline will
never emit an externally visible function. The rationale for this is that it matches variables, for which storage will never be reserved if defined as extern and
always if defined without. The rationale for C99, in contrast, is that it would be astonishing if
using inline would
have a side-effect—to always emit a non-inlined version of the function—that is contrary to what its name suggests.
The remarks for C99 about the need to provide exactly one externally visible function instance for inlined functions and about the resulting problem
with unreachable code apply mutatis mutandis to gnu89 as well.
gcc up to and including version 4.2 used gnu89 inline semantics
even when -std=c99 was
explicitly specified.[6] With
version 5[5],
gcc switched from gnu89 to the gnu11 dialect, effectively enabling C99 inline semantics
by default. To use gnu89 semantics instead, they have to be enabled explicitly, either with -std=gnu89 or,
to only affect inlining, -fgnu89-inline,
or by adding the gnu_inline attribute
to all inline declarations.
To ensure C99 semantics, either -std=c99, -std=c11, -std=gnu99 or -std=gnu11 (without -fgnu89-inline)
can be used.[3]"
And the above makes you look at the C99 definition "In C99, a function defined inline will
never, and a function defined extern inline will
always, emit an externally visible function. ". So this make me wonder if clang is getting more pedantic about the C99 definition of inline (__inline__). So I wonder if we could use an` if ( __STDC_VERSION__ < 199901L)` to turn off
the __inline__ to fix the clang issue?
It also seems strange to me the __inline__ only exists next to the library function. Given it is not in the header (and the code is not in the header) I'm not really sure what the compiler can do? When the BaseIoLibIntrinsic
library gets compiled it is going to create the intrinsic functions. It seems like code only comes together a link time? So unless the GCC linker was doing inline code generation at link time I'm not sure how the __inline__ helps. Does the compiler tag the
object with some kind of hint? If you are doing Link Time Optimization (LTO) the __inline__ is kind of a moot point as the code gen will always inline simple stuff like this.
I'd point out when we ported to GCC we came from VC++ and always had LTO, so it is likely we did not have a good grasp of GCC inlining. Thus there is a non-zero chance this code is a no-op even on old GCC versions. But
it is worth checking out.