| Summary: | [Qt] Enable JIT on WinCE | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Patrick R. Gansterer <paroga> |
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED WONTFIX | ||
| Severity: | Normal | CC: | benjamin, ismail, thomas, zy.zorro |
| Priority: | P3 | Keywords: | Qt, QtTriaged |
| Version: | 528+ (Nightly build) | ||
| Hardware: | Other | ||
| OS: | Other | ||
| Bug Depends on: | 43303 | ||
| Bug Blocks: | |||
| Attachments: | |||
|
Description
Patrick R. Gansterer
2010-08-08 09:03:19 PDT
Created attachment 63871 [details]
Error while trying to compile GeneratedJITStubs_MSVC.asm
I am trying to add asm file to build system but armasm.exe fails to compile it, I attached the error log. (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). I generate it on Windows, let me replace it by hand. We can tackle that problem later. Created attachment 63877 [details]
Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE
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 (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. I forgot about a, for b) I will need to find a good way of checking it. Thanks! Created attachment 63878 [details]
Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE
Try two.
(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. Created attachment 63880 [details]
Next try
- Enables JIT only on ARM
- Defines ENABLE_JIT in WebCore.pro
(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! 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__ Created attachment 63881 [details]
Add GeneratedJITStubs_MSVC.asm to the build system for Windows CE
Fixup double space.
(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. (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. (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. (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. DumpRenderTree will not compile out of the box, it needs wcecompat to compile it seems. I'll see what I can do tomorrow. 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. Got DRT running but the results will take a long time, I will give an update in time. I also hit an assertion while loading http://sabah.com.tr ; ASSERTION FAILED: !m_closed (..\..\..\WebCore\platform\text\SegmentedString.cpp:144 WebCore::SegmentedString::append) 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 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.
Any news on this? I lower the priority since this does not seem active :( Sadly I don't have time to test this atm. (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. (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. OK Let me test this next week, I still have WinCE6 tablets around with Qt. (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. (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 :) (In reply to comment #31) > If sabah.com.tr works and haberturk.com works then it must be OK :) haberturk.com works too. :-) (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. Qt has been removed from tree. |