[Webkit-unassigned] [Bug 130389] Remove ENABLE(LLINT) and ENABLE(LLINT_C_LOOP) guards

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=130389


Mark Lam <mark.lam at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #232247|review?                     |review-
               Flag|                            |




--- Comment #3 from Mark Lam <mark.lam at apple.com>  2014-05-29 07:55:58 PST ---
(From update of attachment 232247)
View in context: https://bugs.webkit.org/attachment.cgi?id=232247&action=review

I've partially reviewed this patch from the bottom up.  There's a fundamental error in the patch, that is, it assumes that we'll always run with the LLINT enabled.  This is not true.  We always build with the LLINT because it provides some of the bootstrapping glue code implementation for the JIT.  However, we do not always run with the LLINT enabled.  It is still possible (and desirable ... especially for testing purposes) to be able to run with the LLINT disabled.  Hence, we should not be removing Options::useLLInt().

> Source/JavaScriptCore/runtime/Executable.cpp:307
> -    if (Options::useLLInt())
> -        setupLLInt(vm, codeBlock.get());
> -    else
> -        setupJIT(vm, codeBlock.get());
> -    
> +    setupLLInt(vm, codeBlock.get());

This change is wrong.  Please revert.

> Source/JavaScriptCore/runtime/Options.cpp:-214
> -    Options::useLLInt() = true;

This change is wrong.  Please revert.

> Source/JavaScriptCore/runtime/Options.cpp:-221
> -#if !ENABLE(LLINT)
> -    Options::useLLInt() = false;
> -#endif

This change is wrong.  Please revert.

> Source/JavaScriptCore/runtime/Options.h:-95
> -    v(bool, useLLInt,  true) \

This change is wrong.  Please revert.

> Source/JavaScriptCore/runtime/VM.h:318
> -#if ENABLE(JIT) && ENABLE(LLINT)
> -        bool canUseJIT() { return m_canUseJIT; }
> -#elif ENABLE(JIT)
> +#if ENABLE(JIT)
>          bool canUseJIT() { return true; } // jit only

This change is wrong.  Should return m_canUseJIT.

> Source/JavaScriptCore/tests/stress/deleteAllCompiledCode.js:-2
> -//@ runNoLLInt
> -

This change is wrong.  Please revert.

> Source/WTF/wtf/Platform.h:728
> +/* If the baseline jit is not available, then disable upper tiers aswell: */

s/aswell/as well/.

> Tools/Scripts/run-jsc-stress-tests:-590
> -def runNoLLInt
> -    run("no-llint", "--useLLInt=false")
> -end
> -

This change is wrong.  Please revert.

> Tools/Scripts/run-jsc-stress-tests:-661
> -    runNoLLInt

This change is wrong.  Please revert.

> Tools/Scripts/run-jsc-stress-tests:-696
> -    runNoLLInt

This change is wrong.  Please revert.

> Tools/Scripts/run-jsc-stress-tests:-740
> -def runLayoutTestNoLLInt
> -    runLayoutTest("no-llint", "--useLLInt=false")
> -end
> -

This change is wrong.  Please revert.

> Tools/Scripts/run-jsc-stress-tests:-763
> -    runLayoutTestNoLLInt

This change is wrong.  Please revert.

> Tools/Scripts/run-jsc-stress-tests:826
> -    runMozillaTest("baseline", mode, extraFiles, "--useLLInt=false", "--useDFGJIT=false")
> +    runMozillaTest("baseline", mode, extraFiles, "--useDFGJIT=false")

This is wrong.  The --useLLInt=false is still valid.  Just because we always build with it doesn't mean we have to run with it.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.