Bug 65088 - Intel Compiler on windows fails to build
Summary: Intel Compiler on windows fails to build
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-24 16:35 PDT by Aron Rosenberg
Modified: 2011-08-08 07:59 PDT (History)
5 users (show)

See Also:


Attachments
Patch to fix issues with Intel compiler on Windows. (3.37 KB, patch)
2011-07-24 16:37 PDT, Aron Rosenberg
morrita: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch to fix issues with Qt and Intel compiler on Windows. (4.96 KB, patch)
2011-07-29 20:39 PDT, Aron Rosenberg
no flags Details | Formatted Diff | Diff
Fix building with Intel Compiler on Windows with Qt (5.16 KB, patch)
2011-07-31 15:47 PDT, Aron Rosenberg
benjamin: review-
benjamin: commit-queue-
Details | Formatted Diff | Diff
Fix building with Intel Compiler on Windows with Qt (5.47 KB, patch)
2011-08-05 14:07 PDT, Aron Rosenberg
benjamin: review+
benjamin: commit-queue-
Details | Formatted Diff | Diff
fix using Intel compiler with Qt on Windows (6.75 KB, patch)
2011-08-06 08:26 PDT, Aron Rosenberg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aron Rosenberg 2011-07-24 16:35:51 PDT
The current Qt webkit 2.2 branch fails to build using the Intel compiler on Windows. The attached patch fixes the various qmake and code issues.
Comment 1 Aron Rosenberg 2011-07-24 16:37:11 PDT
Created attachment 101839 [details]
Patch to fix issues with Intel compiler on Windows.
Comment 2 WebKit Review Bot 2011-07-24 16:38:56 PDT
Attachment 101839 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1

Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Hajime Morrita 2011-07-29 01:26:01 PDT
It looks Qt folks are good for taking look at this change.
Comment 4 WebKit Review Bot 2011-07-29 02:25:38 PDT
Comment on attachment 101839 [details]
Patch to fix issues with Intel compiler on Windows.

Rejecting attachment 101839 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-02', '--port..." exit_code: 2

Last 500 characters of output:
decf9a2f5f30b99e6b5b0e5a1e354f584377a460
r91979 = 62f09c276b0408816db7d77c914a1471e08e6a19
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.
Updating chromium port dependencies using gclient...

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/9269362
Comment 5 Yael 2011-07-29 14:34:46 PDT
Ademar, is this something you should cherrypick to QtWebKit 2.2 ?
Comment 6 Aron Rosenberg 2011-07-29 20:39:31 PDT
Created attachment 102422 [details]
Patch to fix issues with Qt and Intel compiler on Windows.
Comment 7 WebKit Review Bot 2011-07-29 20:41:27 PDT
Attachment 102422 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1

Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Aron Rosenberg 2011-07-29 21:00:34 PDT
Updated patch to include Changelog entries. Any idea why the base Style check is still failing?
Comment 9 Yael 2011-07-30 16:46:46 PDT
(In reply to comment #8)
> Updated patch to include Changelog entries. Any idea why the base Style check is still failing?

Sorry, I am not sure why the style bot is still unhappy, but you need to add the bug id in your changelog.
Comment 10 Aron Rosenberg 2011-07-31 15:47:34 PDT
Created attachment 102469 [details]
Fix building with Intel Compiler on Windows with Qt

Update changelog to include Bug Id.
Comment 11 WebKit Review Bot 2011-07-31 15:50:47 PDT
Attachment 102469 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1

Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Ademar Reis 2011-08-01 07:01:28 PDT
(In reply to comment #5)
> Ademar, is this something you should cherrypick to QtWebKit 2.2 ?

Yep, looks like a simple non-intrusive buildfix. Adding as a nice-to-have.

Thanks.
Comment 13 Benjamin Poulain 2011-08-05 13:25:14 PDT
Comment on attachment 102469 [details]
Fix building with Intel Compiler on Windows with Qt

View in context: https://bugs.webkit.org/attachment.cgi?id=102469&action=review

> ChangeLog:4
> +
> +        Reviewed by Yael Aharon.
> +

Uh???

> Source/JavaScriptCore/wtf/ByteArray.h:94
> +#if COMPILER(MSVC) && !COMPILER(INTEL)

You mean COMPILER(MSVC) is true for the Intel compiler??? That is really weird.

> Source/JavaScriptCore/wtf/NullPtr.h:38
> +#if __has_feature(cxx_nullptr) || (GCC_VERSION_AT_LEAST(4, 6, 0) && defined(__GXX_EXPERIMENTAL_CXX0X__)) || (defined(_MSC_VER) && _MSC_VER >= 1600 && !COMPILER(INTEL))

The AND make the whole line harder to read and look suspicious.
Why not || !COMPILER(INTEL)?

> Source/WebKit.pri:160
> +win32-icc: QMAKE_CXXFLAGS += -wd873

You must add an explanation for this in the changelog.
Comment 14 Aron Rosenberg 2011-08-05 13:36:44 PDT
(In reply to comment #13)
> 
> > Source/JavaScriptCore/wtf/ByteArray.h:94
> > +#if COMPILER(MSVC) && !COMPILER(INTEL)
> 
> You mean COMPILER(MSVC) is true for the Intel compiler??? That is really weird.
The Intel compiler identifies itself as MSVC as well as Intel on Windows based upon which version of MSVC it's paired with.
> 
> > Source/JavaScriptCore/wtf/NullPtr.h:38
> > +#if __has_feature(cxx_nullptr) || (GCC_VERSION_AT_LEAST(4, 6, 0) && defined(__GXX_EXPERIMENTAL_CXX0X__)) || (defined(_MSC_VER) && _MSC_VER >= 1600 && !COMPILER(INTEL))
> 
> The AND make the whole line harder to read and look suspicious.
> Why not || !COMPILER(INTEL)?
> 
See above about the paired identity

> > Source/WebKit.pri:160
> > +win32-icc: QMAKE_CXXFLAGS += -wd873
> 
> You must add an explanation for this in the changelog.
Will do
Comment 15 Aron Rosenberg 2011-08-05 14:07:12 PDT
Created attachment 103108 [details]
Fix building with Intel Compiler on Windows with Qt

Updated based on review comments. Added more changelog information
Comment 16 WebKit Review Bot 2011-08-05 14:08:22 PDT
Attachment 103108 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files']" exit_code: 1

Total errors found: 0 in 0 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 Benjamin Poulain 2011-08-05 14:35:24 PDT
View in context: https://bugs.webkit.org/attachment.cgi?id=103108&action=review

> > You mean COMPILER(MSVC) is true for the Intel compiler??? That is really weird.
> The Intel compiler identifies itself as MSVC as well as Intel on Windows based upon which version of MSVC it's paired with.

Arg, that is annoying.
How much changes would be needed if WTF_COMPILER_MSVC is #undef if the compiler is Intel?
That would make such changes much more explicit.

> ChangeLog:9
> +
> +        Disable warning that uses different number in Intel compiler than Visual Studio

I mean tell which warning you disable. That is useful for future reference for those who will modify the file :)
Comment 18 Aron Rosenberg 2011-08-05 15:03:29 PDT
(In reply to comment #17)
> View in context: https://bugs.webkit.org/attachment.cgi?id=103108&action=review
> 
> > > You mean COMPILER(MSVC) is true for the Intel compiler??? That is really weird.
> > The Intel compiler identifies itself as MSVC as well as Intel on Windows based upon which version of MSVC it's paired with.
> 
> Arg, that is annoying.
> How much changes would be needed if WTF_COMPILER_MSVC is #undef if the compiler is Intel?
A huge amount since on Linux or Mac it looks like GCC in addition to INTEL. I wouldn't want to take the risk of doing that

> That would make such changes much more explicit.
> 
> > ChangeLog:9
> > +
> > +        Disable warning that uses different number in Intel compiler than Visual Studio
> 
> I mean tell which warning you disable. That is useful for future reference for those who will modify the file :)
Will do
Comment 19 Benjamin Poulain 2011-08-06 04:36:39 PDT
Comment on attachment 103108 [details]
Fix building with Intel Compiler on Windows with Qt

View in context: https://bugs.webkit.org/attachment.cgi?id=103108&action=review

> ChangeLog:9
> +        Disable warning that uses different number in Intel compiler than Visual Studio
> +

Before landing, improve the changelog mentioning which warning is disabled.

> Source/JavaScriptCore/wtf/NullPtr.h:38
> -#if __has_feature(cxx_nullptr) || (GCC_VERSION_AT_LEAST(4, 6, 0) && defined(__GXX_EXPERIMENTAL_CXX0X__)) || (defined(_MSC_VER) && _MSC_VER >= 1600)
> +#if __has_feature(cxx_nullptr) || (GCC_VERSION_AT_LEAST(4, 6, 0) && defined(__GXX_EXPERIMENTAL_CXX0X__)) || (defined(_MSC_VER) && _MSC_VER >= 1600 && !COMPILER(INTEL))

My bad, first time I did not notice the new test was inside the parenthesis. This is ok.
Comment 20 Aron Rosenberg 2011-08-06 08:26:49 PDT
Created attachment 103148 [details]
fix using Intel compiler with Qt on Windows

Added more changelog details on warning being disabled.
Comment 21 Benjamin Poulain 2011-08-06 10:09:23 PDT
Comment on attachment 103148 [details]
fix using Intel compiler with Qt on Windows

Let's land this :)
Comment 22 WebKit Review Bot 2011-08-06 12:52:53 PDT
Comment on attachment 103148 [details]
fix using Intel compiler with Qt on Windows

Clearing flags on attachment: 103148

Committed r92555: <http://trac.webkit.org/changeset/92555>
Comment 23 WebKit Review Bot 2011-08-06 12:52:59 PDT
All reviewed patches have been landed.  Closing bug.
Comment 24 Ademar Reis 2011-08-08 07:59:37 PDT
Revision r92555 cherry-picked into qtwebkit-2.2 with commit 0c20586 <http://gitorious.org/webkit/qtwebkit/commit/0c20586>