Bug 65088 – Intel Compiler on windows fails to build

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Aron Rosenberg]

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.

[Aron Rosenberg]

Created attachment 101839 [details]
Patch to fix issues with Intel compiler on Windows.

[WebKit Review Bot]

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.

[Hajime Morrita]

It looks Qt folks are good for taking look at this change.

[WebKit Review Bot]

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

[Yael]

Ademar, is this something you should cherrypick to QtWebKit 2.2 ?

[Aron Rosenberg]

Created attachment 102422 [details]
Patch to fix issues with Qt and Intel compiler on Windows.

[WebKit Review Bot]

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.

[Aron Rosenberg]

Updated patch to include Changelog entries. Any idea why the base Style check is still failing?

[Yael]

(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.

[Aron Rosenberg]

Created attachment 102469 [details]
Fix building with Intel Compiler on Windows with Qt

Update changelog to include Bug Id.

[WebKit Review Bot]

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.

[Ademar Reis]

(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.

[Benjamin Poulain]

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.

[Aron Rosenberg]

(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

[Aron Rosenberg]

Created attachment 103108 [details]
Fix building with Intel Compiler on Windows with Qt

Updated based on review comments. Added more changelog information

[WebKit Review Bot]

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.

[Benjamin Poulain]

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 :)

[Aron Rosenberg]

(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

[Benjamin Poulain]

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.

[Aron Rosenberg]

Created attachment 103148 [details]
fix using Intel compiler with Qt on Windows

Added more changelog details on warning being disabled.

[Benjamin Poulain]

Comment on attachment 103148 [details]
fix using Intel compiler with Qt on Windows

Let's land this :)

[WebKit Review Bot]

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>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ademar Reis]

Revision r92555 cherry-picked into qtwebkit-2.2 with commit 0c20586 <http://gitorious.org/webkit/qtwebkit/commit/0c20586>