Bug 43688 - [Qt] Enable JIT on WinCE
Summary: [Qt] Enable JIT on WinCE
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P3 Normal
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
Depends on: 43303
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-08 09:03 PDT by Patrick R. Gansterer
Modified: 2013-10-31 01:29 PDT (History)
4 users (show)

See Also:


Attachments
Error while trying to compile GeneratedJITStubs_MSVC.asm (33.20 KB, text/plain)
2010-08-09 02:04 PDT, Ismail Donmez
no flags Details
Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE (1.35 KB, patch)
2010-08-09 02:32 PDT, Ismail Donmez
no flags Details | Formatted Diff | Diff
Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE (1.76 KB, patch)
2010-08-09 02:58 PDT, Ismail Donmez
no flags Details | Formatted Diff | Diff
Next try (1.49 KB, patch)
2010-08-09 03:33 PDT, Ismail Donmez
no flags Details | Formatted Diff | Diff
Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE (1.49 KB, patch)
2010-08-09 03:48 PDT, Ismail Donmez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick R. Gansterer 2010-08-08 09:03:19 PDT
Enable the JIT on Windows CE with ARM.
After bug 43303 we need to generate a object file and link it with qmake.
Comment 1 Ismail Donmez 2010-08-09 02:04:19 PDT
Created attachment 63871 [details]
Error while trying to compile GeneratedJITStubs_MSVC.asm
Comment 2 Ismail Donmez 2010-08-09 02:04:51 PDT
I am trying to add asm file to build system but armasm.exe fails to compile it, I attached the error log.
Comment 3 Patrick R. Gansterer 2010-08-09 02:14:56 PDT
(In reply to comment #2)
> I am trying to add asm file to build system but armasm.exe fails to compile it, I attached the error log.
#offset# should be replaced with 32.
This seams to be a problem with your build environment. Where did you generated the GeneratedJITStubs_MSVC.asm? I always generate it on Linux, because I was never successful on Windows (http://trac.webkit.org/wiki/QtWebKitPackaging).
Comment 4 Ismail Donmez 2010-08-09 02:15:51 PDT
I generate it on Windows, let me replace it by hand. We can tackle that problem later.
Comment 5 Ismail Donmez 2010-08-09 02:32:52 PDT
Created attachment 63877 [details]
Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE
Comment 6 Patrick R. Gansterer 2010-08-09 02:36:58 PDT
There is something missing in the patch:
a) ENABLE_JIT as preprocessordefine (or in Platform.h) or
b) a addional check for for ENABLE_JIT before linking the new objectfile
Comment 7 Patrick R. Gansterer 2010-08-09 02:37:47 PDT
(In reply to comment #5)
> Created an attachment (id=63877) [details]
> Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE
Please also add the bugnumber into the ChangeLog.
Comment 8 Ismail Donmez 2010-08-09 02:38:26 PDT
I forgot about a, for b) I will need to find a good way of checking it. Thanks!
Comment 9 Ismail Donmez 2010-08-09 02:58:58 PDT
Created attachment 63878 [details]
Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE

Try two.
Comment 10 Patrick R. Gansterer 2010-08-09 03:15:04 PDT
(In reply to comment #9)
> Created an attachment (id=63878) [details]
> Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE
> 
> Try two.
Clear r-:
1) Missing ChangeLog for Platform.h
2) Wrong place for ENABLE_JIT (don't use OS(WINDOWS); JIT is only supported for ARM now)
3) Set a #define only when when not already set (#ifndef). It must be possible to turn it off by a preprocessordefine.
4) There is already an #define ENABLE_JIT 1 in line 940 at Platform.h

IMHO setting ENABLE_JIT should be done in the pro-file (no changes to the Platform.h) and only form ARM platform.
Add the asm file only for ARM in the pro-file, because WinCE also supports X86, MIPS, SH1.

PS: Please use svn-create-patch. It adds addional info.
Comment 11 Ismail Donmez 2010-08-09 03:33:38 PDT
Created attachment 63880 [details]
Next try

- Enables JIT only on ARM
- Defines ENABLE_JIT in WebCore.pro
Comment 12 Patrick R. Gansterer 2010-08-09 03:43:11 PDT
(In reply to comment #11)
> Created an attachment (id=63880) [details]
> Next try
> 
> - Enables JIT only on ARM
> - Defines ENABLE_JIT in WebCore.pro
1) Is "contains(DEFINES, ARM)" the prefered way to check for ARM? Is there no special plaform variable to check for? I think this is normaly done via checking for makespec.
2) There are two spaces in "asm_compiler.commands +=  -o ...".
Otherwise LGTM!
Comment 13 Ismail Donmez 2010-08-09 03:46:48 PDT
I am not really sure about the proper way for checking ARM but Qt defined the following variable for ARM mkspecs: _ARMV4I_ armv4i _ARM_ ARM _M_ARM __arm__
Comment 14 Ismail Donmez 2010-08-09 03:48:57 PDT
Created attachment 63881 [details]
Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE

Fixup double space.
Comment 15 Patrick R. Gansterer 2010-08-09 04:06:24 PDT
(In reply to comment #13)
> I am not really sure about the proper way for checking ARM but Qt defined the following variable for ARM mkspecs: _ARMV4I_ armv4i _ARM_ ARM _M_ARM __arm__
Let's see what the Qt folks say. ;-)

Did you run the LayoutTests for this change? This change is a good candidate for problems. ;-)

As far as I know the run-webkit-tests don't work for WinCE (at least for me).
I never tried all tests, because I had no problems. But to enable it in the default configuration this is required IMHO.
Comment 16 Ismail Donmez 2010-08-09 04:08:43 PDT
(In reply to comment #15)
> (In reply to comment #13)
> > I am not really sure about the proper way for checking ARM but Qt defined the following variable for ARM mkspecs: _ARMV4I_ armv4i _ARM_ ARM _M_ARM __arm__
> Let's see what the Qt folks say. ;-)
> 
> Did you run the LayoutTests for this change? This change is a good candidate for problems. ;-)
> 
> As far as I know the run-webkit-tests don't work for WinCE (at least for me).
> I never tried all tests, because I had no problems. But to enable it in the default configuration this is required IMHO.

I don't even know how can LayoutTests can run on WinCE! :) I don't think Qt team runs WebKit tests on WinCE.

Also, I think I already found a problem, loading http://sabah.com.tr is stuck at 83% when JIT is enabled.
Comment 17 Patrick R. Gansterer 2010-08-09 04:15:17 PDT
(In reply to comment #16)
> I don't even know how can LayoutTests can run on WinCE! :) I don't think Qt team runs WebKit tests on WinCE.
Compile DumpRenderTree and run it with the files in LayoutTests. I never tried this for WinCE, only on Linux.

> Also, I think I already found a problem, loading http://sabah.com.tr is stuck at 83% when JIT is enabled.
So it won't be a good idea to enable it by default at the moment.
Comment 18 Ismail Donmez 2010-08-09 07:08:27 PDT
(In reply to comment #17)
> (In reply to comment #16)
> > I don't even know how can LayoutTests can run on WinCE! :) I don't think Qt team runs WebKit tests on WinCE.
> Compile DumpRenderTree and run it with the files in LayoutTests. I never tried this for WinCE, only on Linux.

I will try this but no promises, device is slow as 400mhz.

> > Also, I think I already found a problem, loading http://sabah.com.tr is stuck at 83% when JIT is enabled.
> So it won't be a good idea to enable it by default at the moment.

True, we should disable this by default.
Comment 19 Ismail Donmez 2010-08-09 08:16:17 PDT
DumpRenderTree will not compile out of the box, it needs wcecompat to compile it seems. I'll see what I can do tomorrow.
Comment 20 Ismail Donmez 2010-08-10 00:50:01 PDT
I compiled DumpRenderTree put it on the device, passing LayoutTests path as the argument but I have zero output but the process seems to be running.
Comment 21 Ismail Donmez 2010-08-10 05:15:39 PDT
Got DRT running but the results will take a long time, I will give an update in time.
Comment 22 Ismail Donmez 2010-08-13 06:49:44 PDT
I also hit an assertion while loading http://sabah.com.tr ;

ASSERTION FAILED: !m_closed
(..\..\..\WebCore\platform\text\SegmentedString.cpp:144 WebCore::SegmentedString::append)
Comment 23 Ismail Donmez 2010-08-16 04:05:19 PDT
Ok I tested multiple configurations for http://sabah.com.tr problem;

JIT = 1,
YARR = 1,
YARR_JIT = 1 fails.

JIT= 1,
YARR = 1,
YARR_JIT= 0, fails.

JIT = 0,
YARR = 0,
YARR_JIT = 0, works

JIT = 0,
YARR  = 1,
YARR_JIT = 0, works
Comment 24 Andreas Kling 2010-10-07 01:13:56 PDT
Comment on attachment 63881 [details]
Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE

Removing from review queue as agreed on IRC. Switching on the JIT breaks many websites right now, and is not ready to go in.
Comment 25 Benjamin Poulain 2011-01-14 07:21:02 PST
Any news on this?

I lower the priority since this does not seem active :(
Comment 26 Ismail Donmez 2011-01-14 07:22:29 PST
Sadly I don't have time to test this atm.
Comment 27 Patrick R. Gansterer 2011-01-14 08:53:58 PST
(In reply to comment #25)
> Any news on this?
IMHO it should work now, but I'm not able to run any test suite.
Many of the run-javascriptcore-tests already fail already without JIT, so I can't say if it works with JIT enabled.

Are there any _working_ test for WinCE JavaScriptCore at Nokia? Can someone try to enable JIT and tell me what's not wokring correctly?
Maybe you need the patch from bug 52450 at the moment, because r64818 <http://trac.webkit.org/changeset/64818> broke JIT WinCE Qt.
Comment 28 Benjamin Poulain 2011-01-14 10:22:10 PST
(In reply to comment #27)
> Are there any _working_ test for WinCE JavaScriptCore at Nokia? Can someone try to enable JIT and tell me what's not wokring correctly?

Nope, nobody of the WebKit team works on WinCE. This platform is supported by another team who make sure the abstractions used by WebKit are working.

Ismail did some nice contribution as well to make QtWebKit rocks on WinCE.
Comment 29 Ismail Donmez 2011-01-14 11:16:11 PST
OK Let me test this next week, I still have WinCE6 tablets around with Qt.
Comment 30 Patrick R. Gansterer 2011-01-26 07:30:27 PST
(In reply to comment #29)
> OK Let me test this next week, I still have WinCE6 tablets around with Qt.
"Next week" is over :-)
I tried http://sabah.com.tr a few minutes ago and it seams to be working correctly now.
Comment 31 Ismail Donmez 2011-01-26 07:32:56 PST
(In reply to comment #30)
> (In reply to comment #29)
> > OK Let me test this next week, I still have WinCE6 tablets around with Qt.
> "Next week" is over :-)
> I tried http://sabah.com.tr a few minutes ago and it seams to be working correctly now.

I am busy with Android work still :( If sabah.com.tr works and haberturk.com works then it must be OK :)
Comment 32 Patrick R. Gansterer 2011-01-26 07:36:21 PST
(In reply to comment #31)
> If sabah.com.tr works and haberturk.com works then it must be OK :)
haberturk.com works too. :-)
Comment 33 Ismail Donmez 2011-01-26 07:39:03 PST
(In reply to comment #32)
> (In reply to comment #31)
> > If sabah.com.tr works and haberturk.com works then it must be OK :)
> haberturk.com works too. :-)

Thats good to go then! Thanks.
Comment 34 Patrick R. Gansterer 2013-10-31 01:29:05 PDT
Qt has been removed from tree.