Summary: | [4.5/4.6 Regression] ia32intrin.h defines of _rotl, _rotr conflict with target stdlib.h decls | ||
---|---|---|---|
Product: | gcc | Reporter: | Danny Smith <dannysmith> |
Component: | target | Assignee: | Not yet assigned to anyone <unassigned> |
Status: | RESOLVED INVALID | ||
Severity: | normal | CC: | esigra, gcc-bugs, hjl.tools, jakub, ktietz, nightstrike |
Priority: | P2 | ||
Version: | 4.5.0 | ||
Target Milestone: | 4.5.3 | ||
Host: | i686-pc-mingw32 | Target: | i686-pc-mingw32 |
Build: | i686-pc-mingw32 | Known to work: | |
Known to fail: | Last reconfirmed: | ||
Attachments: |
Broken stdlib.h
log from buildbot gcc46-pr40722.patch |
Description
Danny Smith
2009-07-11 22:07:18 UTC
These were added by HJ. Either we need to fixinclude stdlib.h or not define these based on a configure test (I guess the former is more robust if ia32intrin.h defines these only if they are not already defined). (In reply to comment #1) > These were added by HJ. Either we need to fixinclude stdlib.h or not define > these based on a configure test (I guess the former is more robust if > ia32intrin.h defines these only if they are not already defined). > fixinclude sounds a good idea, but I don't know how to do it. Or I can change ia32inintrin.h by not defining them for mingw. (In reply to comment #2) > (In reply to comment #1) > > These were added by HJ. Either we need to fixinclude stdlib.h or not define > > these based on a configure test (I guess the former is more robust if > > ia32intrin.h defines these only if they are not already defined). > > > > fixinclude sounds a good idea, but I don't know how to do it. Or > I can change ia32inintrin.h by not defining them for mingw. > I would vote for guarding the lrotl, lrotr by checks, if they aren't defined already. There is an other issue about ia32intrin.h and mingw targets, but I assume, that it is true for other i386 and x86_64 targets, too. There are the functions __crc32b, __crc32w, and __crt32d, which are in general just available for a target that has SSE4.1 enabled (-mcrc). But the inlines aren't guarded and so the builtins __builtin_ia32_crc32qi (etc) don't have prototypes and so g++ will throw errors. The following sample shows the issue pretty well. t.cc: #include <x86intrin.h> int main() { return 0; } Can we fix this before 4.5 is released? I saw _CRTIMP unsigned int __cdecl __MINGW_NOTHROW _rotl(unsigned int, int) __MINGW_ATTRIB_CONST; _CRTIMP unsigned int __cdecl __MINGW_NOTHROW _rotr(unsigned int, int) __MINGW_ATTRIB_CONST; _CRTIMP unsigned long __cdecl __MINGW_NOTHROW _lrotl(unsigned long, int) __MINGW_ATTRIB_CONST; _CRTIMP unsigned long __cdecl __MINGW_NOTHROW _lrotr(unsigned long, int) __MINGW_ATTRIB_CONST; There is no easy way to check it. I think they should be fixed by fixinclude. A patch is posted at http://gcc.gnu.org/ml/gcc-patches/2010-03/msg00936.html An updated patch is at http://gcc.gnu.org/ml/gcc-patches/2010-03/msg00937.html (In reply to comment #7) > An updated patch is at > > http://gcc.gnu.org/ml/gcc-patches/2010-03/msg00937.html > Patch is fine. It is absolutely necessary to support gcc's intrinsic headers for mingw. The fixinclude approach is one way to solve it and I am fine by it. For mingw-w64 we used the #pragma push/pop_macro feature to make sure we declare function without getting problems by this define in ia32intrin.h. Kai Subject: Bug 40722 Author: hjl Date: Tue Mar 23 13:24:37 2010 New Revision: 157665 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157665 Log: Fix stdlib.h for mingw. 2010-03-23 H.J. Lu <hongjiu.lu@intel.com> PR target/40722 * mkfixinc.sh: Fix stdlib.h for mingw. Modified: trunk/fixincludes/ChangeLog trunk/fixincludes/mkfixinc.sh Created attachment 20228 [details]
Broken stdlib.h
Hi,
This patched caused problems for mingw-w64 stdlib.h, see attachment.
Created attachment 20229 [details]
log from buildbot
Here is the error output from one of the buildbot slaves.
(In reply to comment #10) > Created an attachment (id=20228) [edit] > Broken stdlib.h > > Hi, > > This patched caused problems for mingw-w64 stdlib.h, see attachment. > Someone from mingw64 should adjust fixincludes/mkfixinc.sh for mingw64. (In reply to comment #12) > (In reply to comment #10) > > Created an attachment (id=20228) [edit] > > Broken stdlib.h > > > > Hi, > > > > This patched caused problems for mingw-w64 stdlib.h, see attachment. > > > > Someone from mingw64 should adjust fixincludes/mkfixinc.sh for mingw64. HJ, please consider reverting your patch. Subject: Bug 40722 Author: hjl Date: Sun Mar 28 16:40:50 2010 New Revision: 157784 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157784 Log: 2010-03-28 H.J. Lu <hongjiu.lu@intel.com> PR target/40722 * mkfixinc.sh: Revert the last change for mingw. Modified: trunk/fixincludes/ChangeLog trunk/fixincludes/mkfixinc.sh GCC 4.5.0 is being released. Deferring to 4.5.1. GCC 4.5.1 is being released, adjusting target milestone. GCC 4.5.2 is being released, adjusting target milestone. (In reply to comment #8) > (In reply to comment #7) > > An updated patch is at > > > > http://gcc.gnu.org/ml/gcc-patches/2010-03/msg00937.html > > > > Patch is fine. It is absolutely necessary to support gcc's intrinsic headers > for mingw. The fixinclude approach is one way to solve it and I am fine by it. > For mingw-w64 we used the #pragma push/pop_macro feature to make sure we > declare function without getting problems by this define in ia32intrin.h. > > Kai Why can't you just _CRTIMP unsigned int __cdecl __MINGW_NOTHROW (_rotl)(unsigned int, int) __MINGW_ATTRIB_CONST; ? _rotl etc. from ia32intrin.h is a function-like macro, so the above prevents the expansion. Is this still a problem on *mingw and *mingw64? I mean, if x86intrin.h can't be included, it sounds like a serious problem for those that use this arch, and it is open for more than a year. I guess H.J.'s patch is just too dangerous, it assumes that _rotl is the first declaration and _lrotr is the last one, I guess much better would be to ifdef out each declaration separately, then there wouldn't be issues with unmatched #if/#endif. Created attachment 23233 [details] gcc46-pr40722.patch Modified H.J.'s patch which just adds ()s around _rotl/_rotr/_lrotl/_lrotr names in their prototypes, all I've tested is the sed command on mingw.org and mingw64 stdlib.h and eyeballed the differences. Can anyone please test this both with mingw64 and mingw.org stdlib.h? Thanks. So, as more as I think about it, come to the point that this fix-include for stdlib.h isn't something good. The correct solution here is that mingw.org fixes their stdlib.h header by () (as suggested by this fix-include sed command, which by the way works as expected without clobbering ifdef/else blocks). The issue I see here is, that the local include directory (LOCAL_INCLUDE_DIR) comes before fix-include in include order. So for a native configuration, this won't solve anything as it won't be used at all. So IMHO, this problem should be closed as invalid. |