Bug 92682 – [CMake][EFL] Enable the LLInt

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Thiago Marcos P. Santos]

Use bug 88315 as model.

[Kenneth Rohde Christiansen]

Do you know if this is enabled for Qt?

[Thiago Marcos P. Santos]

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

[Laszlo Gombos]

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.

[Thiago Marcos P. Santos]

Created attachment 162763 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 162763 [details]
Patch

Attachment 162763 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13770794

[Csaba Osztrogonác]

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

[Thiago Marcos P. Santos]

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.

[Thiago Marcos P. Santos]

Created attachment 162876 [details]
Patch

[Csaba Osztrogonác]

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

[Csaba Osztrogonác]

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.

[Thiago Marcos P. Santos]

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

[Thiago Marcos P. Santos]

Created attachment 163134 [details]
Patch

[WebKit Review Bot]

Comment on attachment 163134 [details]
Patch

Clearing flags on attachment: 163134

Committed r128065: <http://trac.webkit.org/changeset/128065>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.