Bug 40671 – Patches for enabling WebKit to compile with the Intel 32-bit C++ compiler

• Status: RESOLVED
• Resolution: WONTFIX
• Component: JavaScriptCore

[Thiago Macieira]

I have managed to get WebKit to compile with the 11.1.072 release of the Intel compiler. 

The changes required consist of two compilation fixes, plus the addition of the WTF_COMPILER_INTEL macro and one more patch that just removes annoying warnings from wtf/Vector.h.

[Thiago Macieira]

Created attachment 58873 [details]
Add the WTF_COMPILER_INTEL define for the Intel C++ compiler

[WebKit Review Bot]

Attachment 58873 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
JavaScriptCore/ChangeLog:3:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 7 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Thiago Macieira]

Created attachment 58874 [details]
label pointers must be void*, not const void*

[WebKit Review Bot]

Attachment 58874 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
JavaScriptCore/ChangeLog:3:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 4 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Thiago Macieira]

Created attachment 58875 [details]
Intel C++ Compiler doesn't know about may_alias

[Thiago Macieira]

Created attachment 58876 [details]
compile JIT with Intel C++ Compiler

[WebKit Review Bot]

Attachment 58875 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
JavaScriptCore/ChangeLog:3:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:5:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
JavaScriptCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 4 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Thiago Macieira]

Created attachment 58881 [details]
Add the WTF_COMPILER_INTEL define for the Intel C++ compiler

[Thiago Macieira]

Created attachment 58882 [details]
label pointers must be void*, not const void*

[Thiago Macieira]

Created attachment 58883 [details]
Intel C++ Compiler doesn't know about may_alias

[Thiago Macieira]

Created attachment 58884 [details]
compile JIT with Intel C++ Compiler

[WebKit Commit Bot]

Comment on attachment 58881 [details]
Add the WTF_COMPILER_INTEL define for the Intel C++ compiler

Clearing flags on attachment: 58881

Committed r61473: <http://trac.webkit.org/changeset/61473>

[WebKit Commit Bot]

Comment on attachment 58882 [details]
label pointers must be void*, not const void*

Clearing flags on attachment: 58882

Committed r61474: <http://trac.webkit.org/changeset/61474>

[WebKit Commit Bot]

Comment on attachment 58883 [details]
Intel C++ Compiler doesn't know about may_alias

Clearing flags on attachment: 58883

Committed r61475: <http://trac.webkit.org/changeset/61475>

[Kenneth Rohde Christiansen]

Oliver, can you have a look at the JIT patch?

[Thiago Macieira]

Please note that the JIT patch should be looked at with diff -w (no whitespace). Most of the change is just reindenting the code.

There are two main changes there:
1) put the code inside a function marked __attribute__((used))

2) remove the ".text" asm keyword (that was added in an older patch of mine), since now the compiler will emit the proper section keywords because of the function body.

[Oliver Hunt]

(In reply to comment #16)
> Please note that the JIT patch should be looked at with diff -w (no whitespace). Most of the change is just reindenting the code.

Why are you reindenting the code?

[Eric Seidel (no email)]

Note that our code review docs (http://trac.webkit.org/wiki/CodeReview) specifically discourage mixing whitespace and non-whitespace changes. :/

[Thiago Macieira]

(In reply to comment #17)
> (In reply to comment #16)
> > Please note that the JIT patch should be looked at with diff -w (no whitespace). Most of the change is just reindenting the code.
> 
> Why are you reindenting the code?

Because the code was outside a function and now is inside. I understand the CodeReview policies, but the way I read it was "don't do unrelated whitespace changes".

I was changing:

asm_code_goes_here

to:
function()
{
    asm_code_goes_here
}

If that is not wanted, I will redo the diff.

[Thiago Macieira]

Created attachment 64204 [details]
compile JIT with Intel C++ Compiler

This is the same patch as the one before, except without the whitespace changes.

[Thiago Macieira]

Created attachment 64205 [details]
Whitespace-only change, reindenting the ASM code

This is the second part, after attachment 64204 [details]. This is a whitespace-only change that simply reindents the asm code into the function.

[Thiago Macieira]

Created attachment 64208 [details]
compile JIT with Intel C++ Compiler

[Thiago Macieira]

Created attachment 64209 [details]
Whitespace-only change, reindenting the ASM code

[Thiago Macieira]

Patches redone against trunk.

http://pastebin.com/strrKyMk contains the same two patches for the qtwebkit-2.0 branch.

[WebKit Review Bot]

Attachment 64208 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
JavaScriptCore/jit/JITStubs.cpp:116:  __attribute__ is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 64208 [details]
compile JIT with Intel C++ Compiler

Seems this will do no harm. Not very much in the WebKit style, though, given the name of the function you used was "asm_wrapper" and we never use that style.

[Thiago Macieira]

I took the liberty of using a non-WebKit name because it's a function never to be called.

[WebKit Commit Bot]

Comment on attachment 64209 [details]
Whitespace-only change, reindenting the ASM code

Rejecting patch 64209 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Darin Adler', u'--force']" exit_code: 1
Parsed 2 diffs from patch file(s).
patching file JavaScriptCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file JavaScriptCore/jit/JITStubs.cpp
Hunk #1 FAILED at 115.
1 out of 1 hunk FAILED -- saving rejects to file JavaScriptCore/jit/JITStubs.cpp.rej

Full output: http://queues.webkit.org/results/4042017

[Eric Seidel (no email)]

This needs an updated patch which applies to tip of tree.

[Thiago Macieira]

Thanks, will supply a patch next week.

[Andreas Kling]

(In reply to comment #30)
> Thanks, will supply a patch next week.

Ping ;)

[Thiago Macieira]

(In reply to comment #31)
> (In reply to comment #30)
> > Thanks, will supply a patch next week.
> 
> Ping ;)

I can't find time to rebase my patches and fix the conflicts in the ChangeLog.

And my Intel compiler license has expired.

[Eric Seidel (no email)]

Sounds like this bug is abandoned then?  Please re-open if I'm wrong.

[Thiago Macieira]

For the moment.

I'll reopen when I manage to get a new license and the installation working.