Forcing inlining gives an error with MingW64 compiler

[GoogleCodeExporter]

I've got a new patch to loosen the inlining defined for MingW64 compilers.
It's just a workaround though, so it needs more investigation.

According to the C++ standard, methods that are defined inside the class 
declaration block are inline by nature, so I think it doesn't need to be forced 
to inline and better leave the decision to the compiler.

Inlining doesn't mean always "fast", there are better alternative optimization 
methods like LTO(link-time optimizations or code generation) and profile-based 
optimizations.

Cheers!
Jay

Index: Fog/Core/C++/CompilerGcc.h
===================================================================
--- Fog/Core/C++/CompilerGcc.h  (revision 772)
+++ Fog/Core/C++/CompilerGcc.h  (working copy)
@@ -21,7 +21,12 @@
 #endif

 // Standard attributes.
-#define FOG_INLINE inline __attribute__((always_inline))
+// FIXME: Forcing inlining for some methods gives a compiler error under 
MingW64
+#if defined(__MINGW32__)
+# define FOG_INLINE inline
+#else
+# define FOG_INLINE inline __attribute__((always_inline))
+#endif
 #define FOG_NO_INLINE __attribute__((noinline))
 #define FOG_NO_RETURN __attribute__((noreturn))
 #define FOG_DEPRECATED __attribute__((deprecated))

Original issue reported on code.google.com by jay3dli...@gmail.com on 11 Oct 2011 at 11:48

[GoogleCodeExporter]

r773 fixes several MinGW problems, but not all. I think that this MinGW version 
is probably broken, because the inlining error can be googled.

I'm going to investigate this more at home (don't have here MinGW 
installation). But I'd like to know the cause of this problem, not disabling 
"force_inline", because I experienced that this slow-downs blitters (they use 
many small inline functions).

So, leaving new, will be investigated.

Original comment by kobalicek.petr on 12 Oct 2011 at 6:08

[GoogleCodeExporter]

What about this:

#define FOG_INLINE inline __attribute__((gnu_inline))

It gives me a warning about "not inlinable" function, but not the error.

Original comment by kobalicek.petr on 12 Oct 2011 at 9:55

[GoogleCodeExporter]

inline will do the same thing without setting the attribute,

But as u mentioned before the main issue is that the compiler is not inlining 
those functions, I will check all the MingW64 flavors I have to see which one 
is stable enough to so it.

Original comment by jay3dli...@gmail.com on 12 Oct 2011 at 10:49

[GoogleCodeExporter]

I'm not sure. I replaced all "inline"s by FOG_INLINE, because code generated 
for blitters was wrong in the past (function calls inside blit pipeline).

I commited the gnu_inline hack, but I have now different problem with undefined 
_control87().

Original comment by kobalicek.petr on 12 Oct 2011 at 1:28

[GoogleCodeExporter]

Temporarily fixed by r777, but the code generated by MinGW is larger (About 
60%) and slow (about 10-15%).

Original comment by kobalicek.petr on 13 Oct 2011 at 3:05

• Changed state: Fixed

[GoogleCodeExporter]

I would suggest u test both cases, with inline keyword and without specifying 
-O3 option and test between both builds (-O3 will actually enable 
-finline-functions)

sometimes inlining makes things worse

Original comment by jay3dli...@gmail.com on 13 Oct 2011 at 3:21

[GoogleCodeExporter]

Fog philosophy is to mark all inlinable functions by FOG_INLINE, and to mark 
all functions that shouldn't be inlined by FOG_NO_INLINE. The problem here is 
that MinGW is unable to inline really trivial functions like getters/setters. I 
don't know what causes this error, but it's really strange, and more stranger 
is that hasn't been fixed for years:(

So my conclusion is simple - don't use MinGW compiler under Windows. I think 
that the strangest here is that GCC running under Linux hasn't these problems - 
generated code is larger, but performance is similar to MSCV compiled version.

BTW: I'm not using -O3 option. Loops unrolling is not necessary (all optimized 
code is already unrolled) and other optimizations had no effect when I compared 
the builds (only binary size was larger without effect).

So I don't know better fix, maybe to use old MinGW with GCC 4.2 which is okay?

Original comment by kobalicek.petr on 13 Oct 2011 at 3:41

[GoogleCodeExporter]

I think I got it working on MingW64 GCC 4.4.7 with the __forceinline macro 
defined in MingW headers for compatibility with MSVC (which is mapped to the 
always_inline attribute of GCC)

### Eclipse Workspace Patch 1.0
#P Fog_src
Index: Fog/Core/C++/CompilerGcc.h
===================================================================
--- Fog/Core/C++/CompilerGcc.h  (revision 777)
+++ Fog/Core/C++/CompilerGcc.h  (working copy)
@@ -25,7 +25,7 @@
 // MinGW has very strange problem that it can't inline some really trivial
 // functions. To workaround this bug we define FOG_INLINE to inline, but this
 // means that many blitters won't be inlined, degrading performance a lot.
-# define FOG_INLINE inline
+# define FOG_INLINE __forceinline
 #else
 # define FOG_INLINE inline __attribute__((always_inline))
 #endif

Inlining working in 4.4.7 but it apparently got broken in 4.5.x 4.6.x for some 
weird reason (along with broken exception handling on Windows 64bit), haven't 
checked 4.7.x though because it's still unstable enough.

I got only some warning on functions that's actually not flagged for inlining 
and defined as FOG_FASTCALL.

Please test it though to see if blitters are working as intended.

Cheers!
Jay

Original comment by jay3dli...@gmail.com on 14 Oct 2011 at 9:38

[GoogleCodeExporter]

I found a temporary fix, because of the bug in MinGW that prevents u from using 
__declspec(dllexport) together with __attribute__((always_inline))

Now defining it as __attribute__((visibility("default"))) which MinGW tends to 
ignore it.

Index: Fog/Core/C++/CompilerGcc.h
===================================================================
--- Fog/Core/C++/CompilerGcc.h  (revision 782)
+++ Fog/Core/C++/CompilerGcc.h  (working copy)
@@ -21,15 +21,7 @@
 #endif

 // Standard attributes.
-#if defined(__MINGW32__)
-// MinGW has very strange problem that it can't inline some really trivial
-// functions. To workaround this bug we define FOG_INLINE to inline, but this
-// means that many blitters won't be inlined, degrading performance a lot.
-# define FOG_INLINE inline
-#else
-# define FOG_INLINE inline __attribute__((always_inline))
-#endif
-
+#define FOG_INLINE inline __attribute__((always_inline))
 #define FOG_NO_INLINE __attribute__((noinline))
 #define FOG_NO_RETURN __attribute__((noreturn))
 #define FOG_DEPRECATED __attribute__((deprecated))
@@ -71,7 +63,13 @@
 // #endif
 #define FOG_NO_EXPORT

-#if defined(FOG_OS_WINDOWS)
+// XXX: MinGW starting from version 4.5.x has a bug
+// (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50779)
+// which prevents using __declspec(dllexport) attribute together with
+// __attribute__((always_inline)), otherwise compiling errors will occur.
+// This is a temporary solution at the cost of a slightly larger DLL,
+// but a higher performing blitters.
+#if defined(FOG_OS_WINDOWS) && !defined(__MINGW32__)
 # define FOG_DLL_IMPORT __declspec(dllimport)
 # define FOG_DLL_EXPORT __declspec(dllexport)
 #elif (defined(__GNUC__) && __GNUC__ >= 4) || \
Index: Fog/Core/OS/OSUtil.cpp
===================================================================
--- Fog/Core/OS/OSUtil.cpp  (revision 782)
+++ Fog/Core/OS/OSUtil.cpp  (working copy)
@@ -202,9 +202,15 @@
     case EILSEQ: return ERR_STRING_INVALID_INPUT;
 #endif // EILSEQ

-#if defined(EOVERFLOW)
+// XXX: EOVERFLOW does not exist on Windows,
+// but the recent MinGW defines it as EFBIG
+// which gives a compiler error here.
+// Ref: http://msdn.microsoft.com/en-us/library/t3ayayh1.aspx
+#if !defined(FOG_OS_WINDOWS)
+# if defined(EOVERFLOW)
     case EOVERFLOW: return ERR_RT_OVERFLOW;
-#endif // EOVERFLOW
+# endif // EOVERFLOW
+#endif // FOG_OS_WINDOWS

 #if defined(ENOMEDIUM)
     case ENOMEDIUM: return ERR_DEVICE_NOT_READY;

Original comment by jay3dli...@gmail.com on 20 Oct 2011 at 10:36

[GoogleCodeExporter]

I'm going to look;)

Do you think that it is possible to export only non-inlined functions? Because 
your patch generates behavior similar to MSVC, where all members in exported 
class (FOG_API) are exported.

I think that marking members which must be exported is good idea, but I don't 
know if compiler automatically exports also vtable and similar stuff, if not 
marked as FOG_API in class def.

Original comment by kobalicek.petr on 21 Oct 2011 at 9:35

[GoogleCodeExporter]

There's a GCC compiler option -fvisibility-inlines-hidden which will hide all 
inline functions, with no source alteration, which in turn means that the macro 
defining FOG_DLL_EXPORT should be empty so this switch can work properly. 
However I haven't tested if this switch will actually work with MinGW compiler.

MSVC tends to ignore silently those kind of issues and do do whatever it wants, 
while GCC will whine about and refuses to compile ;) That's what I understood 
when u mentioned that MSVC even exported all members and ignored the hiding.

Original comment by jay3dli...@gmail.com on 22 Oct 2011 at 8:04

[GoogleCodeExporter]

I'm not sure whether the -fvisibility-inlines-hidden works also with MinGW, 
because the visibility is not supported on Windows platform (on non-ELF 
platform to be correct). So I turned all -f settings regarding to visibility 
off.

I'm going to experiment with this a bit more, the same behavior for all 
platforms is something I'd like to accomplish.

Reopened until solved.

Original comment by kobalicek.petr on 22 Oct 2011 at 4:22

• Changed state: Accepted