Mike Bond mbond@cox.rr.com writes:
A couple days ago a number of inline functions were put into winebase.h. Since then I have been unable to build, using gcc 2.96-81 (RedHat). Apparently this version of gcc thinks that because there is an extern prefixing the inline, even though the function does in fact have a body, that the function really is external. Removing the extern, just having inline as one would do in C++, does not appear to work correctly either, and causes the function to appear as a exported symbol in all the object files that header gets included with, thus resulting in multiple definitions.
This patch should fix the build failure, though it doesn't explain why gcc doesn't inline such a simple function.
Index: dlls/kernel/kernel32.spec =================================================================== RCS file: /opt/cvs-commit/wine/dlls/kernel/kernel32.spec,v retrieving revision 1.39 diff -u -r1.39 kernel32.spec --- dlls/kernel/kernel32.spec 2001/08/06 17:50:42 1.39 +++ dlls/kernel/kernel32.spec 2001/08/16 16:30:03 @@ -922,7 +922,9 @@ @ stub GetProcessPriorityBoost @ stdcall GetThreadPriorityBoost(long ptr) GetThreadPriorityBoost @ stdcall InterlockedCompareExchange (ptr long long) InterlockedCompareExchange +@ stdcall InterlockedCompareExchangePointer(ptr ptr ptr) InterlockedCompareExchangePointer @ stdcall InterlockedExchangeAdd (ptr long ) InterlockedExchangeAdd +@ stdcall InterlockedExchangePointer(ptr ptr) InterlockedExchangePointer @ stdcall IsProcessorFeaturePresent(long) IsProcessorFeaturePresent @ stdcall OpenWaitableTimerA(long long str) OpenWaitableTimerA @ stdcall OpenWaitableTimerW(long long wstr) OpenWaitableTimerW Index: scheduler/critsection.c =================================================================== RCS file: /opt/cvs-commit/wine/scheduler/critsection.c,v retrieving revision 1.30 diff -u -r1.30 critsection.c --- scheduler/critsection.c 2001/08/09 21:21:13 1.30 +++ scheduler/critsection.c 2001/08/16 16:30:11 @@ -86,7 +86,7 @@ /*********************************************************************** * InterlockedCompareExchange (KERNEL32.@) */ -/* PVOID WINAPI InterlockedCompareExchange( PVOID *dest, PVOID xchg, PVOID compare ); */ +/* LONG WINAPI InterlockedCompareExchange( PLONG dest, LONG xchg, LONG compare ); */ __ASM_GLOBAL_FUNC(InterlockedCompareExchange, "movl 12(%esp),%eax\n\t" "movl 8(%esp),%ecx\n\t" @@ -95,10 +95,31 @@ "ret $12");
/*********************************************************************** + * InterlockedCompareExchangePointer (KERNEL32.@) + */ +/* PVOID WINAPI InterlockedCompareExchangePointer( PVOID *dest, PVOID xchg, PVOID compare ); */ +__ASM_GLOBAL_FUNC(InterlockedCompareExchangePointer, + "movl 12(%esp),%eax\n\t" + "movl 8(%esp),%ecx\n\t" + "movl 4(%esp),%edx\n\t" + "lock; cmpxchgl %ecx,(%edx)\n\t" + "ret $12"); + +/*********************************************************************** * InterlockedExchange (KERNEL32.@) */ /* LONG WINAPI InterlockedExchange( PLONG dest, LONG val ); */ __ASM_GLOBAL_FUNC(InterlockedExchange, + "movl 8(%esp),%eax\n\t" + "movl 4(%esp),%edx\n\t" + "lock; xchgl %eax,(%edx)\n\t" + "ret $8"); + +/*********************************************************************** + * InterlockedExchangePointer (KERNEL32.@) + */ +/* PVOID WINAPI InterlockedExchangePointer( PVOID *dest, PVOID val ); */ +__ASM_GLOBAL_FUNC(InterlockedExchangePointer, "movl 8(%esp),%eax\n\t" "movl 4(%esp),%edx\n\t" "lock; xchgl %eax,(%edx)\n\t"
On Thu, Aug 16, 2001 at 10:37:39AM -0700, Alexandre Julliard wrote:
This patch should fix the build failure, though it doesn't explain why gcc doesn't inline such a simple function.
This indeed allow me to build. My guess is that gcc-2.96 won't inline anything in a debug build. The suggestion about 'static inline' instead of 'extern inline' worked right up until it got to the file that actually implemented the various functions in question, at which point the compiler complained about the two definitions not matching, one being static the other not. My guess as to why the 'static inline' would work is that while it would still not actually inline it in a debug build, it is providing a static implementation to each file that includes winbase.h. This is horrible bloat for debug builds as many files that do not need it will get those functions anyway. It would still be nice if there could be a way to inline those various functions in a more generic way so anything built against winelib outside of the scheduler could take advantage of the inlining, but I really don't see, the way inline is being interpreted in 2.96, that this could be done.
On Fri, 17 Aug 2001, Mike Bond wrote:
On Thu, Aug 16, 2001 at 10:37:39AM -0700, Alexandre Julliard wrote:
This patch should fix the build failure, though it doesn't explain why gcc doesn't inline such a simple function.
This indeed allow me to build. My guess is that gcc-2.96 won't inline anything in a debug build. The suggestion about 'static inline' instead of 'extern inline' worked right up until it got to the file that actually implemented the various functions in question, at which point the compiler complained about the two definitions not matching, one being static the other not. My guess as to why the 'static inline' would work is that while it would still not actually inline it in a debug build, it is providing a static implementation to each file that includes winbase.h. This is horrible bloat for debug builds as many files that do not need it will get those functions anyway. It would still be nice if there could be a way to inline those various functions in a more generic way so anything built against winelib outside of the scheduler could take advantage of the inlining, but I really don't see, the way inline is being interpreted in 2.96, that this could be done.
Linux itself just went through a transformation of extern inline into static inline. It was triggered by a change in gcc, presumably for the better. There was some on-list grumbling, but the change was necessary.
john
I would have thought any inlining would enlarge a program compared with single-copy-and-call functions.
Extern inline would imply that there had to be an un-inlined copy somewhere to advertise to the linker, sort of like a weak symbol.
At 07:56 17/08/01 -0700, John Alvord wrote:
On Fri, 17 Aug 2001, Mike Bond wrote:
On Thu, Aug 16, 2001 at 10:37:39AM -0700, Alexandre Julliard wrote:
This patch should fix the build failure, though it doesn't explain why gcc doesn't inline such a simple function.
<snip: static inline causes more bloat than extern inline >
Linux itself just went through a transformation of extern inline into static inline. It was triggered by a change in gcc, presumably for the better. There was some on-list grumbling, but the change was necessary.
john
In general, that is correct. The problem in the case of gcc 2.96 is that it appears to actually be not inlining the function at all in a non-optimized build and further is putting the actual function in all objects generated from source that included the headers that defined the static inline function, even those that did not use it. I may be wrong on this, but given my limited digging through gcc this appears to be what is happening.
As for extern inline, yes, that may well be why they got away from that syntax, as it does indeed suggest an external symbol somewhere.
On Fri, Aug 17, 2001 at 04:34:02PM +0100, J. Cone wrote:
I would have thought any inlining would enlarge a program compared with single-copy-and-call functions.
Extern inline would imply that there had to be an un-inlined copy somewhere to advertise to the linker, sort of like a weak symbol.
True... But at least in Linux source, inlines are used deliberately and with every expectation that the compiler will inline those routines. Linux is very focussed on superb performance and doesn't want any stupid compiler tricks to mess around with the code.
I have no idea how this applies to the WINE philosphy. I generally use inlines only for very small functions.
john
On Fri, 17 Aug 2001, J. Cone wrote:
I would have thought any inlining would enlarge a program compared with single-copy-and-call functions.
Extern inline would imply that there had to be an un-inlined copy somewhere to advertise to the linker, sort of like a weak symbol.
At 07:56 17/08/01 -0700, John Alvord wrote:
On Fri, 17 Aug 2001, Mike Bond wrote:
On Thu, Aug 16, 2001 at 10:37:39AM -0700, Alexandre Julliard wrote:
This patch should fix the build failure, though it doesn't explain why gcc doesn't inline such a simple function.
<snip: static inline causes more bloat than extern inline >
Linux itself just went through a transformation of extern inline into static inline. It was triggered by a change in gcc, presumably for the better. There was some on-list grumbling, but the change was necessary.
john
-
Alexandre Julliard -
J. Cone -
John Alvord -
Mike Bond