Bug 92682

Summary: [CMake][EFL] Enable the LLInt
Product: WebKit Reporter: Thiago Marcos P. Santos <tmpsantos>
Component: JavaScriptCoreAssignee: Thiago Marcos P. Santos <tmpsantos>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, donggwan.kim, fpizlo, gyuyoung.kim, hojong.han, kenneth, laszlo.gombos, mark.lam, mxie, ossy, paroga, rakuco, rwlbuis, ryuan.choi, tonikitoo, webkit.review.bot, yong.li.webkit
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 96299    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Thiago Marcos P. Santos 2012-07-30 14:29:38 PDT
Use bug 88315 as model.
Comment 1 Kenneth Rohde Christiansen 2012-08-02 09:56:59 PDT
Do you know if this is enabled for Qt?
Comment 2 Thiago Marcos P. Santos 2012-08-02 11:06:27 PDT
(In reply to comment #1)
> Do you know if this is enabled for Qt?

Last time I checked, only Apple and GTK had it enabled.
Comment 3 Laszlo Gombos 2012-08-02 16:24:45 PDT
Most of the expected changes are in the port specific build system so it make sense to separate the Qt port work from the EFL work.

Reference to the Qt-port specific work is here - bug 80839.
Comment 4 Thiago Marcos P. Santos 2012-09-07 07:15:18 PDT
Created attachment 162763 [details]
Patch
Comment 5 Gyuyoung Kim 2012-09-07 08:03:33 PDT
Comment on attachment 162763 [details]
Patch

Attachment 162763 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13770794
Comment 6 Csaba Osztrogonác 2012-09-07 08:10:56 PDT
Comment on attachment 162763 [details]
Patch

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

> Source/JavaScriptCore/CMakeLists.txt:299
> +IF (ENABLE_LLINT)
> +    # We cannot check for RUBY_FOUND because it is set only when the full package is installed and
> +    # the only thing we need is the interpretor. Unlike Python, cmake does not provide a macro
> +    # for finding the only Ruby interpretor.
> +    IF (NOT RUBY_EXECUTABLE)
> +        MESSAGE(FATAL_ERROR "The Ruby interpretor is needed to generate LLInt files.")

s/interpretor/interpreter/g

> Source/JavaScriptCore/CMakeLists.txt:348
> +        llint/LLIntOffsetsExtractor.cpp

I think it caused the build fail on the EFL EWS.

We don't need to add this file to the target. The binary of 
LLIntOffsetsExtractor is used for generating LLIntAssembly.h

> Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:37
> -#include "LLintCLoop.h"
> -#include "LLintSlowPaths.h"
> +#include "LLIntCLoop.h"
> +#include "LLIntSlowPaths.h"

If this patch will be the faster one, feel free to close https://bugs.webkit.org/show_bug.cgi?id=96099 as duplicated. ;-)
Comment 7 Thiago Marcos P. Santos 2012-09-07 14:34:41 PDT
Comment on attachment 162763 [details]
Patch

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

Thanks for reviewing.

>> Source/JavaScriptCore/CMakeLists.txt:348
>> +        llint/LLIntOffsetsExtractor.cpp
> 
> I think it caused the build fail on the EFL EWS.
> 
> We don't need to add this file to the target. The binary of 
> LLIntOffsetsExtractor is used for generating LLIntAssembly.h

Indeed. My mistake here.

>> Source/JavaScriptCore/llint/LowLevelInterpreter.cpp:37
>> +#include "LLIntSlowPaths.h"
> 
> If this patch will be the faster one, feel free to close https://bugs.webkit.org/show_bug.cgi?id=96099 as duplicated. ;-)

Let's land bug 96099. IMO it is better to fix this on a separated bug.
Comment 8 Thiago Marcos P. Santos 2012-09-07 14:50:39 PDT
Created attachment 162876 [details]
Patch
Comment 9 Csaba Osztrogonác 2012-09-10 06:43:15 PDT
Comment on attachment 162876 [details]
Patch

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

LGTM, r=me with a little nit.

> Source/JavaScriptCore/CMakeLists.txt:296
> +    # the only thing we need is the interpretor. Unlike Python, cmake does not provide a macro

one more s/interpretor/interpreter :)
Comment 10 Csaba Osztrogonác 2012-09-10 06:46:43 PDT
Just a question. Did you try LLInt C loop too on EFL?
(ENABLE_JIT=0, ENABLE_LLINT=1, ENABLE_CLASSIC_INTERPRETER=0)

I tried it on Qt, but there are many failing JSC tests and zillion 
crashing layout tests - https://bugs.webkit.org/show_bug.cgi?id=95749
And I don't know if the bug is in LLInt C loop or in QtWebKit somewhere.
Comment 11 Thiago Marcos P. Santos 2012-09-10 07:55:17 PDT
(In reply to comment #10)
> Just a question. Did you try LLInt C loop too on EFL?
> (ENABLE_JIT=0, ENABLE_LLINT=1, ENABLE_CLASSIC_INTERPRETER=0)
> 
> I tried it on Qt, but there are many failing JSC tests and zillion 
> crashing layout tests - https://bugs.webkit.org/show_bug.cgi?id=95749
> And I don't know if the bug is in LLInt C loop or in QtWebKit somewhere.

Not yet. Gonna try this week and I can give you some feedback.
Comment 12 Thiago Marcos P. Santos 2012-09-10 08:14:53 PDT
Created attachment 163134 [details]
Patch
Comment 13 WebKit Review Bot 2012-09-10 09:06:09 PDT
Comment on attachment 163134 [details]
Patch

Clearing flags on attachment: 163134

Committed r128065: <http://trac.webkit.org/changeset/128065>
Comment 14 WebKit Review Bot 2012-09-10 09:06:16 PDT
All reviewed patches have been landed.  Closing bug.