Bug 81077

Summary: Add support for Intel compiler on Linux QT port.
Product: WebKit Reporter: Kwonjin Jeong <gram>
Component: WebKit QtAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ariya.hidayat, hausmann, kenneth, ossy, skyul, vestbo, webkit.review.bot, yuqiang.xian, zecke
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 80971    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Link error
none
Patch none

Description Kwonjin Jeong 2012-03-13 21:51:52 PDT
Add support for Intel compiler on Linux QT port.

Build WebKit and test browser with

 $ WEBKITOUTPUTDIR=`pwd`/qtbuild ../Tools/Scripts/build-webkit --qt --qmakearg="CONFIG+=icc" --release

To succeed in building WebKit, Apply a patch in the following bug.
https://bugs.webkit.org/show_bug.cgi?id=80971
Comment 1 Kwonjin Jeong 2012-03-13 21:57:12 PDT
Created attachment 131784 [details]
Patch
Comment 2 Kwonjin Jeong 2012-03-13 22:28:15 PDT
Created attachment 131786 [details]
Patch
Comment 3 Kwonjin Jeong 2012-03-15 17:54:09 PDT
I'm looking forward to some comments about this patch.
Comment 4 Simon Hausmann 2012-03-16 02:52:04 PDT
Comment on attachment 131786 [details]
Patch

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

These may seem like simple changes, but unfortunately an explanation as of _why_ these changes are done is missing. A few comments below.

> Source/WebCore/Target.pri:3920
> +            unix:icc: QMAKE_CXX = icpc

Why is this needed? Doesn't the qmake icc makespace in qtbase set QMAKE_CXX to icpc?

> Tools/ChangeLog:11
> +        * qmake/mkspecs/features/unix/icc.prf: Added.

Where is this used? I don't see a CONFIG += icc anywhere.
Comment 5 Kwonjin Jeong 2012-03-16 03:38:53 PDT
(In reply to comment #4)
Thank you for your comments.

> (From update of attachment 131786 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=131786&action=review
> 
> These may seem like simple changes, but unfortunately an explanation as of _why_ these changes are done is missing. A few comments below.
> 
> > Source/WebCore/Target.pri:3920
> > +            unix:icc: QMAKE_CXX = icpc
> 
> Why is this needed? Doesn't the qmake icc makespace in qtbase set QMAKE_CXX to icpc?

No, it doesn't. Therefore, ANGLE is still compiled with g++ without the code.

I don't know the exact reason but I roughly guessed that icc.prf file is applied after Target.pri file is evaluated.


> 
> > Tools/ChangeLog:11
> > +        * qmake/mkspecs/features/unix/icc.prf: Added.
> 
> Where is this used? I don't see a CONFIG += icc anywhere.

It used with command-line arguments for build-webkit script.
I already mentioned in the bug description. (https://bugs.webkit.org/show_bug.cgi?id=81077#c0)
Comment 6 Simon Hausmann 2012-03-16 04:22:08 PDT
Comment on attachment 131786 [details]
Patch

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

>>> Source/WebCore/Target.pri:3920
>>> +            unix:icc: QMAKE_CXX = icpc
>> 
>> Why is this needed? Doesn't the qmake icc makespace in qtbase set QMAKE_CXX to icpc?
> 
> No, it doesn't. Therefore, ANGLE is still compiled with g++ without the code.
> 
> I don't know the exact reason but I roughly guessed that icc.prf file is applied after Target.pri file is evaluated.

I think we should get to the bottom of it though instead of applying this change. Next we need the same hack for msvc or cross-compilers (arm-foo-g++ for example). QMAKE_CXX should come from qmake.conf, and I do see qtbase/mkspecs/linux-icc/qmake.conf initializing QMAKE_CXX correctly to icpc.

Can you try taking the commandline of "make qmake" from the Makefile, execute it manually and add a "-d -d -d" to the parameters passed to qmake? Then you'll get a ton of debug output that should show you which variables are assigned where, and from there you should be able to see why QMAKE_CXX is not set to icpc for you.
Comment 7 Kwonjin Jeong 2012-03-18 18:35:12 PDT
Created attachment 132517 [details]
Link error

(In reply to comment #6)
I'm sorry for late reply.

I tried making a Makefile with the following command.
 $ QMAKEPATH=/home/gram/project/parallel/WebKit/Tools/qmake /home/gram/project/parallel/QtSDK/Desktop/Qt/4.8.0/gcc/bin/qmake -d -d -d /home/gram/project/parallel/WebKit/Source/WebCore/Target.pri CONFIG+=icc CONFIG+=release CONFIG-=debug -o Makefile.WebCore.Target &> log

and I found the following line of the log.
   168 DEBUG 1: Project Parser: /home/gram/project/parallel/QtSDK/Desktop/Qt/4.8.0/gcc/mkspecs/common/g++-base.conf:18 :QMAKE_CXX: :=: (g++)


Therefore, I build WebKit with --qmakearg="-spec linux-icc" option after discarding changes on Source/WebCore/Target.pri, and removing all options excepts QMAKE_CXXFLAGS. But the build is failed with link errors.
Comment 8 Kwonjin Jeong 2012-03-19 03:39:52 PDT
I find the reason why the build is failed with '-spec linux-icc'.

There are many g++-specific configurations something like that.
 ./Source/WebCore/WebCore.pri:unix:!mac:*-g++*:QMAKE_LFLAGS += -Wl,--gc-sections

But those options are not applied with '-spec linux-icc' option.

So, I just copy linux-icc mkspecs to linux-g++-icc, then build with '-spec linux-g++-icc' options. In result, I succeed in building WebKit.


I think that the best solution is modifying g++-specific configurations for Intel compiler.
Comment 9 Simon Hausmann 2012-03-19 07:41:02 PDT
(In reply to comment #7)
> Created an attachment (id=132517) [details]
> Link error
> 
> (In reply to comment #6)
> I'm sorry for late reply.
> 
> I tried making a Makefile with the following command.
>  $ QMAKEPATH=/home/gram/project/parallel/WebKit/Tools/qmake /home/gram/project/parallel/QtSDK/Desktop/Qt/4.8.0/gcc/bin/qmake -d -d -d /home/gram/project/parallel/WebKit/Source/WebCore/Target.pri CONFIG+=icc CONFIG+=release CONFIG-=debug -o Makefile.WebCore.Target &> log
> 
> and I found the following line of the log.
>    168 DEBUG 1: Project Parser: /home/gram/project/parallel/QtSDK/Desktop/Qt/4.8.0/gcc/mkspecs/common/g++-base.conf:18 :QMAKE_CXX: :=: (g++)

This shows that you're using Qt from the Qt SDK in its gcc configuration. In order to build WebKit with icc your Qt also needs to be built with icc, so that qmake's default mkspec is linux-icc.
Comment 10 Kwonjin Jeong 2012-03-19 19:32:47 PDT
(In reply to comment #9)

I don't think that Qt needs to be built with icc.
We can choose a mkspec with --qmakearg="-spec [mkspec_name]" instead of default one, and I already succeed in building with the option and some hack. (refer to comment #7)

The remain issue is that some g++-specific configurations on project files(.pro, .pri, .prf files) are also applied when using Intel compiler.
Comment 11 Kwonjin Jeong 2012-03-19 22:29:13 PDT
Created attachment 132758 [details]
Patch

This patch resolve the link errors that I uploaded before.

You can build with Intel compiler by running the following command.

  WEBKITOUTPUTDIR=`pwd`/qtbuild ../Tools/Scripts/build-webkit --qt --qmakearg="CONFIG+=icc -spec linux-icc" --makeargs="-j20" --release
Comment 12 Kwonjin Jeong 2012-03-21 19:50:45 PDT
(In reply to comment #9)

Hi, Simon.

If you don't mind, would you review my patch about Bug 81077(Add support for Intel compiler on Linux QT port)?

I think you are the right man to review the patch because I found and fix the problem by the help of you, and the patch modify a line of code what you modified before.

I'm looking forward to your review.
Comment 13 Kwonjin Jeong 2012-03-27 18:33:06 PDT
Ping.

I still waiting for review.
Comment 14 Tor Arne Vestbø 2012-03-28 02:58:45 PDT
Comment on attachment 132758 [details]
Patch

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

> Tools/qmake/mkspecs/features/default_post.prf:138
> +        !linux-g++*:!linux-icc*:contains(QT_CONFIG, reduce_exports):CONFIG += hide_symbols

Please put this one in the icc.prf file as well:

contains(TEMPLATE, lib):!plugin:contains(QT_CONFIG, reduce_exports):CONFIG += hide_symbols
Comment 15 Tor Arne Vestbø 2012-03-28 03:01:44 PDT
Comment on attachment 132758 [details]
Patch

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

>> Tools/qmake/mkspecs/features/default_post.prf:138
>> +        !linux-g++*:!linux-icc*:contains(QT_CONFIG, reduce_exports):CONFIG += hide_symbols
> 
> Please put this one in the icc.prf file as well:
> 
> contains(TEMPLATE, lib):!plugin:contains(QT_CONFIG, reduce_exports):CONFIG += hide_symbols

Sorry, my bad, didn't see the ! Keep it like this, but I suspect there's a better way to write this down the road.
Comment 16 WebKit Review Bot 2012-03-28 03:11:31 PDT
Comment on attachment 132758 [details]
Patch

Clearing flags on attachment: 132758

Committed r112380: <http://trac.webkit.org/changeset/112380>
Comment 17 WebKit Review Bot 2012-03-28 03:11:36 PDT
All reviewed patches have been landed.  Closing bug.