Bug 40671

Summary: Patches for enabling WebKit to compile with the Intel 32-bit C++ compiler
Product: WebKit Reporter: Thiago Macieira <thiago.macieira>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED WONTFIX    
Severity: Normal CC: commit-queue, eric, hausmann, kenneth, kling, oliver, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Add the WTF_COMPILER_INTEL define for the Intel C++ compiler
none
label pointers must be void*, not const void*
none
Intel C++ Compiler doesn't know about may_alias
none
compile JIT with Intel C++ Compiler
none
Add the WTF_COMPILER_INTEL define for the Intel C++ compiler
none
label pointers must be void*, not const void*
none
Intel C++ Compiler doesn't know about may_alias
none
compile JIT with Intel C++ Compiler
none
compile JIT with Intel C++ Compiler
none
Whitespace-only change, reindenting the ASM code
none
compile JIT with Intel C++ Compiler
darin: review+
Whitespace-only change, reindenting the ASM code darin: review+, commit-queue: commit-queue-

Description Thiago Macieira 2010-06-16 04:25:32 PDT
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.
Comment 1 Thiago Macieira 2010-06-16 04:26:46 PDT
Created attachment 58873 [details]
Add the WTF_COMPILER_INTEL define for the Intel C++ compiler
Comment 2 WebKit Review Bot 2010-06-16 04:27:42 PDT
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.
Comment 3 Thiago Macieira 2010-06-16 04:27:42 PDT
Created attachment 58874 [details]
label pointers must be void*, not const void*
Comment 4 WebKit Review Bot 2010-06-16 04:28:11 PDT
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.
Comment 5 Thiago Macieira 2010-06-16 04:28:24 PDT
Created attachment 58875 [details]
Intel C++ Compiler doesn't know about may_alias
Comment 6 Thiago Macieira 2010-06-16 04:28:59 PDT
Created attachment 58876 [details]
compile JIT with Intel C++ Compiler
Comment 7 WebKit Review Bot 2010-06-16 04:33:03 PDT
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.
Comment 8 Thiago Macieira 2010-06-16 04:55:33 PDT
Created attachment 58881 [details]
Add the WTF_COMPILER_INTEL define for the Intel C++ compiler
Comment 9 Thiago Macieira 2010-06-16 04:56:11 PDT
Created attachment 58882 [details]
label pointers must be void*, not const void*
Comment 10 Thiago Macieira 2010-06-16 04:56:43 PDT
Created attachment 58883 [details]
Intel C++ Compiler doesn't know about may_alias
Comment 11 Thiago Macieira 2010-06-16 04:57:15 PDT
Created attachment 58884 [details]
compile JIT with Intel C++ Compiler
Comment 12 WebKit Commit Bot 2010-06-19 02:09:34 PDT
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>
Comment 13 WebKit Commit Bot 2010-06-19 02:23:25 PDT
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>
Comment 14 WebKit Commit Bot 2010-06-19 02:45:22 PDT
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>
Comment 15 Kenneth Rohde Christiansen 2010-06-30 00:18:41 PDT
Oliver, can you have a look at the JIT patch?
Comment 16 Thiago Macieira 2010-06-30 00:45:44 PDT
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.
Comment 17 Oliver Hunt 2010-07-02 12:38:44 PDT
(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?
Comment 18 Eric Seidel (no email) 2010-07-02 12:48:31 PDT
Note that our code review docs (http://trac.webkit.org/wiki/CodeReview) specifically discourage mixing whitespace and non-whitespace changes. :/
Comment 19 Thiago Macieira 2010-07-03 00:01:43 PDT
(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.
Comment 20 Thiago Macieira 2010-08-12 03:21:07 PDT
Created attachment 64204 [details]
compile JIT with Intel C++ Compiler

This is the same patch as the one before, except without the whitespace changes.
Comment 21 Thiago Macieira 2010-08-12 03:22:44 PDT
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.
Comment 22 Thiago Macieira 2010-08-12 04:40:46 PDT
Created attachment 64208 [details]
compile JIT with Intel C++ Compiler
Comment 23 Thiago Macieira 2010-08-12 04:41:14 PDT
Created attachment 64209 [details]
Whitespace-only change, reindenting the ASM code
Comment 24 Thiago Macieira 2010-08-12 04:45:00 PDT
Patches redone against trunk.

http://pastebin.com/strrKyMk contains the same two patches for the qtwebkit-2.0 branch.
Comment 25 WebKit Review Bot 2010-08-12 04:46:44 PDT
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.
Comment 26 Darin Adler 2010-08-29 12:48:39 PDT
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.
Comment 27 Thiago Macieira 2010-08-30 02:52:01 PDT
I took the liberty of using a non-WebKit name because it's a function never to be called.
Comment 28 WebKit Commit Bot 2010-09-15 00:37:36 PDT
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
Comment 29 Eric Seidel (no email) 2010-10-14 11:35:47 PDT
This needs an updated patch which applies to tip of tree.
Comment 30 Thiago Macieira 2010-10-15 01:34:27 PDT
Thanks, will supply a patch next week.
Comment 31 Andreas Kling 2010-11-12 16:49:38 PST
(In reply to comment #30)
> Thanks, will supply a patch next week.

Ping ;)
Comment 32 Thiago Macieira 2010-11-13 03:25:25 PST
(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.
Comment 33 Eric Seidel (no email) 2010-12-14 15:13:31 PST
Sounds like this bug is abandoned then?  Please re-open if I'm wrong.
Comment 34 Thiago Macieira 2010-12-15 03:09:44 PST
For the moment.

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