Bug 125 - Eigen3, MinGW: compilation failure, intrin.h does not exist
Eigen3, MinGW: compilation failure, intrin.h does not exist
Status: RESOLVED FIXED
Product: Eigen
Classification: Unclassified
Component: Core - general
3.0
x86 - 32-bit Windows
: --- Unknown
Assigned To: Eamon Nerbonne
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-12-02 15:34 UTC by Matt
Modified: 2010-12-07 16:20 UTC (History)
4 users (show)



Attachments
Only do w64 intrin workaround in 64-bit mode (1.72 KB, patch)
2010-12-03 11:23 UTC, Eamon Nerbonne
no flags Details | Diff
Intrinsics Mingw32 fix via extern "C" rather than intrin.h (2.28 KB, patch)
2010-12-03 23:30 UTC, Eamon Nerbonne
no flags Details | Diff

Description Matt 2010-12-02 15:34:28 UTC
Currently trying out Eigen 3.0-beta2 (2010-10-15).

There seems to be a problem in file "eigen\Eigen\Core" -- right here:
// include files
#if (defined __GNUC__) && (defined __MINGW32__)
	#include <intrin.h>
	//including intrin.h works around a MINGW bug http://sourceforge.net/tracker/?func=detail&atid=102435&aid=2962480&group_id=2435
	//in essence, intrin.h is included by windows.h and also declares intrinsics (just as emmintrin.h etc. below do).  However,
	//intrin.h uses an extern "C" declaration, and g++ thus complains of duplicate declarations with conflicting linkage.  The linkage for intrinsics
	//doesn't matter, but at that stage the compiler doesn't know; so, to avoid compile errors when windows.h is included after Eigen/Core,
	//include intrin here.
#endif 

Despite the claim made in the above comment, there is no "intrin.h" file in MinGW.

Tested both under the "g++-4.3.0-20080502-mingw32-alpha" as well as "g++ (GCC) 4.5.0" (which is the current official MinGW.org release, installed by the automatic installer dated 2010-10-30, i.e. "mingw-get-inst-20101030.exe").

Incidentally, neither does "include/windows.h" in any of the above versions include any header file containing "intrin" in its name.

This prevents a successful compilation -- e.g. if a file "example.cpp" contains

#include "Eigen/Dense"

in line 4, then the compilation fails:

In file included from ../Eigen/Dense:1:0,
                     from example.cpp:4:
../Eigen/Core:97:26: fatal error: intrin.h: No such file or directory
compilation terminated.

I've tried two workarounds:
(0) comment out #include <intrin.h> (since it doesn't exist on MinGW)
(1) since my CPU's ISA is x86, replace it with #include <x86intrin.h> (which exists in MinGW since at least GCC 4.3.0)

Then, the code seemingly compiles, but it's unclear whether which one (if any) correctly solves the problem.

On top of that, irrespectively of whether (0) or (1) was tried, experiencing SEGFAULTs in SSE2/OpenMP environment -- please advise, should I file a separate report on that or wait for the resolution of this one first (or should I provide additional information on the second bug as well in this report, as they may be related)?

Forum thread, with additional another problem (the above, SSE2) report:
http://forum.kde.org/viewtopic.php?f=74&t=91709
Comment 1 Benoit Jacob 2010-12-02 15:38:08 UTC
According to hg annotate, this code was introduced in changeset 6e860548ac904c87bb42207f9dfd24ac32fbfe2a by Eamon. He doesn't have a bugzilla account yet, mailing him.
Comment 2 Benoit Jacob 2010-12-02 15:42:23 UTC
> On top of that, irrespectively of whether (0) or (1) was tried, experiencing
> SEGFAULTs in SSE2/OpenMP environment -- please advise, should I file a separate
> report on that or wait for the resolution of this one first (or should I
> provide additional information on the second bug as well in this report, as
> they may be related)?

This is almost certainly a separate bug, please file with steps to reproduce (with testcase if applicable).
Comment 3 Eamon Nerbonne 2010-12-02 17:41:02 UTC
I'll take a look.

MingW was rather slow at getting a 4.5 release, so I've been using other builds lately - but several (4.6 svn and 4.5 stable by equation.com, one by TDM, and one by mingw64); and all of them have an intrin.h.

Could this be x86/x64 related perhaps?
Comment 4 Matt 2010-12-03 00:19:35 UTC
(In reply to comment #3)
> I'll take a look.
> 
> MingW was rather slow at getting a 4.5 release, so I've been using other builds
> lately - but several (4.6 svn and 4.5 stable by equation.com, one by TDM, and
> one by mingw64); and all of them have an intrin.h.
> 
> Could this be x86/x64 related perhaps?

Could be -- I've also tried compiling Eigen 3.0-beta2 on another computer, with TDM MinGW, current 32-bit release (gcc-4.5.1-tdm-1) -- no "intrin.h" file either (there exists "c:\MinGW32\lib\gcc\mingw32\4.5.1\include\x86intrin.h", though; analogously to the official MinGW.org installation).

On a side note, I did try Equation.com's "gcc-4.5.1-32.exe" and "gcc-
4.6-20101127-32.exe" -- in contrast with MinGW, those third party packages have "intrin.h" present; however, the compilation failed for other reasons (with a long list of errors, not pretty) and I could not even find a workaround that helped to move it forward. I suppose this is possibly low-priority though (at least for me), as I do not plan to use third party packages (and I believe they are less popular than official MinGW and TDM's MinGW). If supporting these is important, I can file a separate bug report, too.

Still, getting even just the official MinGW to work would be great! :-)
Comment 5 Eamon Nerbonne 2010-12-03 02:05:01 UTC
Confirmed; and it indeed seems specific to the x86 version (which ships without intrin.h).

Perhaps the workaround is unnecessary in mingw64 by now too; I'll check, and if it's still necessary I'll just change the preprocessor guard to include a check for x64, otherwise it can just be deleted.
Comment 6 Eamon Nerbonne 2010-12-03 02:05:19 UTC
Hmm, on second thought - this might not be x86 vs. x64 but mingw32 vs. mingw64 (
http://mingw-w64.sourceforge.net/
) - which obviously *can* run on x64, but also runs on x86.

Some more tests required...
Comment 7 Eamon Nerbonne 2010-12-03 11:23:55 UTC
Created attachment 45 [details]
Only do w64 intrin workaround in 64-bit mode

Unfortunately, the mingw-w64 gcc builds still do require the intrin.h workaround. I tried mingw-w64's 32-bit builds; these do have an intrin.h but weirdly (fortunately!) don't require the intrin.h workaround.  In other words, simply adding a check for 64-bit seems safe; that'll work on plain mingw, and on mingw-w64, and on mingw-w64 for 32-bit systems.

The attached patch does the minor modification.
Comment 8 Benoit Jacob 2010-12-03 14:25:07 UTC
(In reply to comment #7)
> Created attachment 45 [details]
> Only do w64 intrin workaround in 64-bit mode
> 
> Unfortunately, the mingw-w64 gcc builds still do require the intrin.h
> workaround. I tried mingw-w64's 32-bit builds; these do have an intrin.h but
> weirdly (fortunately!) don't require the intrin.h workaround.  In other words,
> simply adding a check for 64-bit seems safe; that'll work on plain mingw, and
> on mingw-w64, and on mingw-w64 for 32-bit systems.
> 
> The attached patch does the minor modification.

I didn't know this __SIZEOF_POINTER__, but if this works on mingw why not...

Can the people who reported this bug please test this patch?
Comment 9 Matt 2010-12-03 15:41:40 UTC
(In reply to comment #7)
> Created attachment 45 [details]
> Only do w64 intrin workaround in 64-bit mode
> 
> Unfortunately, the mingw-w64 gcc builds still do require the intrin.h
> workaround. I tried mingw-w64's 32-bit builds; these do have an intrin.h but
> weirdly (fortunately!) don't require the intrin.h workaround.  In other words,
> simply adding a check for 64-bit seems safe; that'll work on plain mingw, and
> on mingw-w64, and on mingw-w64 for 32-bit systems.
> 
> The attached patch does the minor modification.

Eamon, thanks, will test.

By the way, have you considered using some of the flags mentioned here:
http://groups.google.com/group/mpir-devel/browse_thread/thread/e8c4aa0e4f4daafd

In particular: "MINGW64 defines [...] _WIN64 __MINGW64___".

If it works uniformly (across various distributions) well, then it might express the intent more clearly. Unfortunately, I do not have access to a x86-64 Windows OS here, so I won't be able to check the flags myself.
Comment 10 Benoit Jacob 2010-12-03 15:52:15 UTC
(In reply to comment #9)
> By the way, have you considered using some of the flags mentioned here:
> http://groups.google.com/group/mpir-devel/browse_thread/thread/e8c4aa0e4f4daafd
> 
> In particular: "MINGW64 defines [...] _WIN64 __MINGW64___".

I would prefer that. If that works.
Comment 11 Eamon Nerbonne 2010-12-03 23:25:10 UTC
(defined _WIN64) works too.

An alternative solution is perhaps cleaner:

The real problem here is that these intrinsics are defined as functions in plain C or C++ even though they're actually compiler intrinsics.  So even though it's nonsense, linkage matters; and in particular, intrin.h and emmintrin.h include some of the same intrinsics but intrin.h does so in an extern "C" section.

So, putting the SSE includes ???intrin.h in an extern "C" section in eigen also resolves the problem, without requiring another header - I tested this; it works.  In a sense, that's a cleaner solution all around.

So, we could just stick the ???intrin.h files eigen includes in an extern "C" section.  That's likely to work in all variants of gcc (after all, linkage doesn't matter), but to be conservative, we could do that for just mingw.  Then, if a future version of mingw *does* include intrin.h (or any other intrinsics header with extern "C" linkage, for that matter), Eigen doesn't suddenly break.
Comment 12 Eamon Nerbonne 2010-12-03 23:30:02 UTC
Created attachment 48 [details]
Intrinsics Mingw32 fix via extern "C" rather than intrin.h

This is a patch that sticks eigen's intrinsics header inclusions in extern "C" for any mingw version (and does not mess with intrin.h at all).

Tested in the latest mingw.org build, the latest mingw-w64 x64 build, and the latest mingw-w64 x86 build using the following:

#include <Eigen/Core>
#include <windows.h>
int main(int , char* []) {return 0;}
Comment 13 Matt 2010-12-04 00:31:12 UTC
(In reply to comment #7)
> Created attachment 45 [details]
> Only do w64 intrin workaround in 64-bit mode
> 
> Unfortunately, the mingw-w64 gcc builds still do require the intrin.h
> workaround. I tried mingw-w64's 32-bit builds; these do have an intrin.h but
> weirdly (fortunately!) don't require the intrin.h workaround.  In other words,
> simply adding a check for 64-bit seems safe; that'll work on plain mingw, and
> on mingw-w64, and on mingw-w64 for 32-bit systems.
> 
> The attached patch does the minor modification.

OK, tested patch with __SIZEOF_POINTER__.

Compilation without errors, tested under:
- MinGW.org GCC "g++-4.3.0-20080502-mingw32-alpha"
- MinGW.org GCC "g++ (GCC) 4.5.0" 
- TDM MinGW GCC, current 32-bit release (gcc-4.5.1-tdm-1)
- MinGW-w64 (target: w32, x86) sezero's build "mingw-w32-bin_i686-mingw_20101003_sezero.zip"

I guess this issue is done as far as I can test, thank you very much for the help! :-)

// Regarding another bug -- still experiencing (OpenMP & SSE2+) bug, interestingly it shows up under O0 and O2 optimizations while disappearing in O1 and O3, will have to prepare a minimal-reproducible-case and test it under various compilers, will report back (filing another report, as Benoit suggested) when done.
Comment 14 Eamon Nerbonne 2010-12-07 12:22:51 UTC
Can a core contributor verify that one of the two patches is OK?  My personal preference is for the second patch using extern "C"; that feels cleaner to me.
Comment 15 Benoit Jacob 2010-12-07 14:20:57 UTC
The extern "C" fix looks very much like the right fix, I love it. But why don't we always do it? Why only for MinGW? I think I'm going to take your patch and always do it, no #ifdef.
Comment 16 Eamon Nerbonne 2010-12-07 14:31:50 UTC
I didnt' always do it simply because I didn't test it on linux and whatnot - I don't *think* it'll blow anything up, but I can't verify that. Removing the #ifdef is definitely even cleaner :-).
Comment 17 Benoit Jacob 2010-12-07 16:15:10 UTC
Pushed. Thanks a lot. This extern "C" is something we theoretically needed anyways since C headers are not guaranteed to be valid as C++ (C is not strictly a subset of C++). Feel free to reopen if needed.
Comment 18 Benoit Jacob 2010-12-07 16:20:08 UTC
Sorry, I exaggerated a bit in my previous comment, extern "C" wont magically make C code compile as C++, but you get the point :)

Note You need to log in before you can comment on or make changes to this bug.