Bug 43688 – [Qt] Enable JIT on WinCE

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

[Patrick R. Gansterer]

Enable the JIT on Windows CE with ARM.
After bug 43303 we need to generate a object file and link it with qmake.

[Ismail Donmez]

Created attachment 63871 [details]
Error while trying to compile GeneratedJITStubs_MSVC.asm

[Ismail Donmez]

I am trying to add asm file to build system but armasm.exe fails to compile it, I attached the error log.

[Patrick R. Gansterer]

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

[Ismail Donmez]

I generate it on Windows, let me replace it by hand. We can tackle that problem later.

[Ismail Donmez]

Created attachment 63877 [details]
Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE

[Patrick R. Gansterer]

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

[Patrick R. Gansterer]

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

[Ismail Donmez]

I forgot about a, for b) I will need to find a good way of checking it. Thanks!

[Ismail Donmez]

Created attachment 63878 [details]
Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE

Try two.

[Patrick R. Gansterer]

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

[Ismail Donmez]

Created attachment 63880 [details]
Next try

- Enables JIT only on ARM
- Defines ENABLE_JIT in WebCore.pro

[Patrick R. Gansterer]

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

[Ismail Donmez]

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__

[Ismail Donmez]

Created attachment 63881 [details]
Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE

Fixup double space.

[Patrick R. Gansterer]

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

[Ismail Donmez]

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

[Patrick R. Gansterer]

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

[Ismail Donmez]

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

[Ismail Donmez]

DumpRenderTree will not compile out of the box, it needs wcecompat to compile it seems. I'll see what I can do tomorrow.

[Ismail Donmez]

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.

[Ismail Donmez]

Got DRT running but the results will take a long time, I will give an update in time.

[Ismail Donmez]

I also hit an assertion while loading http://sabah.com.tr ;

ASSERTION FAILED: !m_closed
(..\..\..\WebCore\platform\text\SegmentedString.cpp:144 WebCore::SegmentedString::append)

[Ismail Donmez]

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

[Andreas Kling]

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.

[Benjamin Poulain]

Any news on this?

I lower the priority since this does not seem active :(

[Ismail Donmez]

Sadly I don't have time to test this atm.

[Patrick R. Gansterer]

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

[Benjamin Poulain]

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

[Ismail Donmez]

OK Let me test this next week, I still have WinCE6 tablets around with Qt.

[Patrick R. Gansterer]

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

[Ismail Donmez]

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

[Patrick R. Gansterer]

(In reply to comment #31)
> If sabah.com.tr works and haberturk.com works then it must be OK :)
haberturk.com works too. :-)

[Ismail Donmez]

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

[Patrick R. Gansterer]

Qt has been removed from tree.