Bug 144039 – Enable FTL JIT by default on AArch64

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

[Csaba Osztrogonác]

There are 2 bugs cause failures (crashes!) with FTL JIT
on Linux (X86_64 and AArch64 too): bug143087 and bug143089

( We might want to bump to LLVM 3.6 - bug143821 
after somebody fixed the infinite loop bug in it. )

If the FTL JIT is fast and stable enough on 
AArch64, maybe we should enable it by default.

[Csaba Osztrogonác]

Created attachment 251308 [details]
AArch64 Linux performance results

The deviation is extremely big, so I have no idea how can 
we get trusty performance results on an AArch64 board.

[Csaba Osztrogonác]

(In reply to comment #1)
> Created attachment 251308 [details]
> AArch64 Linux performance results
> 
> The deviation is extremely big, so I have no idea how can 
> we get trusty performance results on an AArch64 board.

I found the reason, we have different cores (big.LITTLE) 
and Linux dynamically scales the frequency. I'll try to
set fixed frequency and lock the run on same cores.

[Csaba Osztrogonác]

Now there is no LLVM, but B3 based FTL JIT.

I tested the conformance on AArch64 Linux with it.

With disabled FTL JIT
----------------------
** The following JSC stress test failures have been introduced:
	executableAllocationFuzz.yaml/executableAllocationFuzz/v8-raytrace.js.executable-allocation-fuzz-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-no-llint
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-no-llint
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-llint
	jsc-layout-tests.yaml/js/script-tests/number-toLocaleString.js.layout
	jsc-layout-tests.yaml/js/script-tests/number-toLocaleString.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/number-toLocaleString.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/number-toLocaleString.js.layout-no-llint
	jsc-layout-tests.yaml/js/slow-stress/script-tests/variadic-closure-call.js.default
	mozilla-tests.yaml/js1_2/Array/slice.js.mozilla-baseline
	regress/script-tests/get-by-id-bimorphic-check-structure-elimination.js.no-llint
	regress/script-tests/hoist-poly-check-structure.js.dfg-eager-no-cjit-validate
	regress/script-tests/v8-regexp-search.js.default
	stress/ftl-put-by-id-setter-exception.js.dfg-maximal-flush-validate-no-cjit
	stress/proxy-revoke.js.no-llint
	stress/super-property-access.js.no-cjit-validate-phases
	stress/v8-splay-strict.js.no-cjit-validate-phases
	v8-v6/v8-crypto.js.no-llint

Results for JSC stress tests:
    27 failures found.

With enabled FTL JIT
----------------------
** The following JSC stress test failures have been introduced:
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-ftl
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-ftl-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-ftl-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-no-llint
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-ftl
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-ftl-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-ftl-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-no-llint
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-ftl
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-ftl-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-ftl-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-llint
	jsc-layout-tests.yaml/js/script-tests/number-toLocaleString.js.layout
	jsc-layout-tests.yaml/js/script-tests/number-toLocaleString.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/number-toLocaleString.js.layout-ftl
	jsc-layout-tests.yaml/js/script-tests/number-toLocaleString.js.layout-ftl-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/number-toLocaleString.js.layout-ftl-no-cjit
	jsc-layout-tests.yaml/js/script-tests/number-toLocaleString.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/number-toLocaleString.js.layout-no-llint
	mozilla-tests.yaml/ecma/String/15.5.4.4-1.js.mozilla-baseline
	regress/script-tests/call-spread-call.js.always-trigger-copy-phase
	regress/script-tests/call-spread-call.js.default
	regress/script-tests/call-spread-call.js.dfg-maximal-flush-validate-no-cjit
	regress/script-tests/deltablue-for-of.js.ftl-eager-no-cjit
	regress/script-tests/deltablue-for-of.js.ftl-no-cjit-no-inline-validate
	regress/script-tests/deltablue-for-of.js.no-llint
	regress/script-tests/generator-fib.js.ftl-no-cjit-validate-sampling-profiler
	regress/script-tests/generator-sunspider-access-nsieve.js.ftl-no-cjit-validate-sampling-profiler
	regress/script-tests/generator-with-several-types.js.ftl-no-cjit-validate-sampling-profiler
	regress/script-tests/richards-empty-try-catch.js.dfg-eager-no-cjit-validate
	regress/script-tests/undefined-property-access.js.default
	regress/script-tests/varargs-call.js.ftl-no-cjit-validate-sampling-profiler
	stress/array-concat-spread-object.js.ftl-no-cjit-validate-sampling-profiler
	stress/array-concat-with-slow-indexingtypes.js.dfg-eager-no-cjit-validate
	stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-no-cjit-validate-sampling-profiler
	stress/arrowfunction-lexical-bind-this-8.js.ftl-no-cjit-validate-sampling-profiler
	stress/double-rep-with-undefined.js.ftl-no-cjit-validate-sampling-profiler
	stress/es6-default-parameters.js.ftl-no-cjit-validate-sampling-profiler
	stress/floating-point-div-to-mul.js.dfg-maximal-flush-validate-no-cjit
	stress/ftl-library-inlining-loops.js.no-llint
	stress/ftl-to-ftl-arity-fixup.js.ftl-no-cjit-validate-sampling-profiler
	stress/ftl-try-catch-varargs-call-throws.js.ftl-no-cjit-validate-sampling-profiler
	stress/generator-function-declaration-sinking-put.js.ftl-no-cjit-validate-sampling-profiler
	stress/math-floor-basics.js.dfg-maximal-flush-validate-no-cjit
	stress/proxy-prevent-extensions.js.ftl-no-cjit-no-inline-validate
	stress/proxy-revoke.js.no-llint
	stress/spread-calling.js.ftl-no-cjit-validate-sampling-profiler
	stress/super-property-access.js.ftl-eager-no-cjit
	stress/super-property-access.js.no-llint
	stress/tail-call-no-stack-overflow.js.ftl-no-cjit-validate-sampling-profiler
	stress/typedarray-functions-with-neutered.js.ftl-no-cjit-validate-sampling-profiler
	v8-v6/v8-crypto.js.dfg-eager
	v8-v6/v8-deltablue.js.ftl-eager-no-cjit
	v8-v6/v8-splay.js.no-llint

Results for JSC stress tests:
    63 failures found.

[Csaba Osztrogonác]

cc-ing GTK guys, maybe you are interested in enabling FTL JIT on AArch64
and fixing random crashes on it.

[Csaba Osztrogonác]

AArch64 Linux backend seems to be quite stable nowadays, because
the normal 10-20 random crashes went away near r202199 - r202216.
There were only 2 JSC changes that time, I'll check which fixed
the random crashes:
- https://trac.webkit.org/changeset/202214
- https://trac.webkit.org/changeset/202205

updated status:
================

without FTL JIT
----------------
** The following JSC stress test failures have been introduced:
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-no-llint
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-no-llint
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-llint
	jsc-layout-tests.yaml/js/script-tests/stringimpl-to-jsstring-on-large-strings-1.js.layout-no-cjit
	mozilla-tests.yaml/ecma_3/Date/15.9.5.6.js.mozilla
	mozilla-tests.yaml/ecma_3/Date/15.9.5.6.js.mozilla-baseline
	mozilla-tests.yaml/ecma_3/Date/15.9.5.6.js.mozilla-dfg-eager-no-cjit-validate-phases
	mozilla-tests.yaml/ecma_3/Date/15.9.5.6.js.mozilla-llint

Results for JSC stress tests:
    17 failures found.



with FTL JIT
--------------
** The following JSC stress test failures have been introduced:
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-ftl
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-ftl-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-ftl-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-collator.js.layout-no-llint
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-ftl
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-ftl-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-ftl-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-datetimeformat.js.layout-no-llint
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-ftl
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-ftl-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-ftl-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/intl-numberformat.js.layout-no-llint
	jsc-layout-tests.yaml/js/script-tests/stringimpl-to-jsstring-on-large-strings-1.js.layout
	jsc-layout-tests.yaml/js/script-tests/stringimpl-to-jsstring-on-large-strings-3.js.layout-ftl
	jsc-layout-tests.yaml/js/script-tests/stringimpl-to-jsstring-on-large-strings-3.js.layout-no-llint
	mozilla-tests.yaml/ecma_3/Date/15.9.5.6.js.mozilla
	mozilla-tests.yaml/ecma_3/Date/15.9.5.6.js.mozilla-baseline
	mozilla-tests.yaml/ecma_3/Date/15.9.5.6.js.mozilla-dfg-eager-no-cjit-validate-phases
	mozilla-tests.yaml/ecma_3/Date/15.9.5.6.js.mozilla-ftl
	mozilla-tests.yaml/ecma_3/Date/15.9.5.6.js.mozilla-ftl-eager-no-cjit-validate-phases
	mozilla-tests.yaml/ecma_3/Date/15.9.5.6.js.mozilla-llint
	regress/script-tests/generator-fib.js.ftl-no-cjit-validate-sampling-profiler
	regress/script-tests/generator-sunspider-access-nsieve.js.ftl-no-cjit-validate-sampling-profiler
	regress/script-tests/generator-with-several-types.js.ftl-no-cjit-validate-sampling-profiler
	regress/script-tests/varargs-call.js.ftl-no-cjit-validate-sampling-profiler
	stress/arrow-functions-as-default-parameter-values.js.ftl-no-cjit-validate-sampling-profiler
	stress/arrowfunction-lexical-bind-arguments-strict.js.ftl-no-cjit-validate-sampling-profiler
	stress/arrowfunction-lexical-bind-superproperty.js.ftl-no-cjit-validate-sampling-profiler
	stress/arrowfunction-lexical-bind-this-8.js.ftl-no-cjit-validate-sampling-profiler
	stress/double-rep-with-undefined.js.ftl-no-cjit-validate-sampling-profiler
	stress/ftl-to-ftl-arity-fixup.js.ftl-no-cjit-validate-sampling-profiler
	stress/ftl-try-catch-varargs-call-throws.js.ftl-no-cjit-validate-sampling-profiler
	stress/generator-function-declaration-sinking-osrexit.js.ftl-no-cjit-validate-sampling-profiler
	stress/generator-function-declaration-sinking-put.js.ftl-no-cjit-validate-sampling-profiler
	stress/spread-calling.js.ftl-no-cjit-validate-sampling-profiler
	stress/tail-call-no-stack-overflow.js.ftl-no-cjit-validate-sampling-profiler

Results for JSC stress tests:
    45 failures found.


Extra crashes with FTL JIT
---------------------------
	regress/script-tests/generator-fib.js.ftl-no-cjit-validate-sampling-profiler
	regress/script-tests/generator-sunspider-access-nsieve.js.ftl-no-cjit-validate-sampling-profiler
	regress/script-tests/generator-with-several-types.js.ftl-no-cjit-validate-sampling-profiler
	regress/script-tests/varargs-call.js.ftl-no-cjit-validate-sampling-profiler
	stress/arrow-functions-as-default-parameter-values.js.ftl-no-cjit-validate-sampling-profiler
	stress/arrowfunction-lexical-bind-arguments-strict.js.ftl-no-cjit-validate-sampling-profiler
	stress/arrowfunction-lexical-bind-superproperty.js.ftl-no-cjit-validate-sampling-profiler
	stress/arrowfunction-lexical-bind-this-8.js.ftl-no-cjit-validate-sampling-profiler
	stress/double-rep-with-undefined.js.ftl-no-cjit-validate-sampling-profiler
	stress/ftl-to-ftl-arity-fixup.js.ftl-no-cjit-validate-sampling-profiler
	stress/ftl-try-catch-varargs-call-throws.js.ftl-no-cjit-validate-sampling-profiler
	stress/generator-function-declaration-sinking-osrexit.js.ftl-no-cjit-validate-sampling-profiler
	stress/generator-function-declaration-sinking-put.js.ftl-no-cjit-validate-sampling-profiler
	stress/spread-calling.js.ftl-no-cjit-validate-sampling-profiler
	stress/tail-call-no-stack-overflow.js.ftl-no-cjit-validate-sampling-profiler

[Csaba Osztrogonác]

Update: There are a few crashes/failures with enabled FTL JIT on AArch64 Linux: (tested on r213461)

stress/array-concat-spread-object.js.ftl-no-cjit-validate-sampling-profiler: Exception: failed normally with an object
stress/array-concat-spread-object.js.ftl-no-cjit-validate-sampling-profiler: ERROR: Unexpected exit code: 3

stress/class-syntax-super-in-eval.js.ftl-eager-no-cjit: Segmentation fault
stress/class-syntax-super-in-eval.js.ftl-eager-no-cjit: ERROR: Unexpected exit code: 139

microbenchmarks/generator-with-several-types.js.ftl-eager-no-cjit: Timed out after 207.000000 sec seconds!
microbenchmarks/generator-with-several-types.js.ftl-eager-no-cjit: Segmentation fault
microbenchmarks/generator-with-several-types.js.ftl-eager-no-cjit: ERROR: Unexpected exit code: 139

microbenchmarks/varargs-call.js.ftl-no-cjit-validate-sampling-profiler: Segmentation fault
microbenchmarks/varargs-call.js.ftl-no-cjit-validate-sampling-profiler: ERROR: Unexpected exit code: 139


If there is any volunteer to check and fix these crashes, we could enable 
it in the near future. Unfortunately I don't and won't have time to do
it myself.

[Yusuke Suzuki]

(In reply to comment #6)
> Update: There are a few crashes/failures with enabled FTL JIT on AArch64
> Linux: (tested on r213461)

Maybe some of them can be fixed since they are not AArch64 specific thing I think.
But others are a bit difficult...

> 
> stress/array-concat-spread-object.js.ftl-no-cjit-validate-sampling-profiler:
> Exception: failed normally with an object
> stress/array-concat-spread-object.js.ftl-no-cjit-validate-sampling-profiler:
> ERROR: Unexpected exit code: 3

This should be related to https://bugs.webkit.org/show_bug.cgi?id=153704.
So it should be debugged even in x86_64 environment. I'll check this this weekend.

> stress/class-syntax-super-in-eval.js.ftl-eager-no-cjit: Segmentation fault
> stress/class-syntax-super-in-eval.js.ftl-eager-no-cjit: ERROR: Unexpected
> exit code: 139

I think it's difficult to debug without AArch64 machine.
Can anyone investigate this issue?

> microbenchmarks/generator-with-several-types.js.ftl-eager-no-cjit: Timed out
> after 207.000000 sec seconds!
> microbenchmarks/generator-with-several-types.js.ftl-eager-no-cjit:
> Segmentation fault
> microbenchmarks/generator-with-several-types.js.ftl-eager-no-cjit: ERROR:
> Unexpected exit code: 139

I guess this is pure timeout. Can anyone investigate it?
If so, just moving it to slowMicrobenchmarks directory solves the problem.


> microbenchmarks/varargs-call.js.ftl-no-cjit-validate-sampling-profiler:
> Segmentation fault
> microbenchmarks/varargs-call.js.ftl-no-cjit-validate-sampling-profiler:
> ERROR: Unexpected exit code: 139

And I think this is also related to https://bugs.webkit.org/show_bug.cgi?id=153704.
I believe varargs forwarding in FTL have some bugs related to SP update & signal stack.

> 
> 
> If there is any volunteer to check and fix these crashes, we could enable 
> it in the near future. Unfortunately I don't and won't have time to do
> it myself.

[Yusuke Suzuki]

Hi Ossy!

Today I've landed the patch to fix call-varargs failures when enabling sampling profilers.
https://trac.webkit.org/changeset/213631

I think this fix reduces the listed failures.
Could you run the tests with AArch64 FTL and update the remaining failures?

[Csaba Osztrogonác]

(In reply to comment #8)
> Hi Ossy!
> 
> Today I've landed the patch to fix call-varargs failures when enabling
> sampling profilers.
> https://trac.webkit.org/changeset/213631
> 
> I think this fix reduces the listed failures.
> Could you run the tests with AArch64 FTL and update the remaining failures?

Sure, I can run tests later today and try to check if increasing the timeout helps or not.

[Csaba Osztrogonác]

Unfortunately JSC is completely broken on AArch64 Linux because of bug169510,
there is no reason to talk about testing/enabling FTL until bug169510 is fixed.

[Yusuke Suzuki]

(In reply to comment #10)
> Unfortunately JSC is completely broken on AArch64 Linux because of bug169510,
> there is no reason to talk about testing/enabling FTL until bug169510 is
> fixed.

Maybe, that ifdef thing reverted the above change.
So, how is the current FTL AArch64 situation?

[Zan Dobersek]

(In reply to Yusuke Suzuki from comment #11)
> (In reply to comment #10)
> > Unfortunately JSC is completely broken on AArch64 Linux because of bug169510,
> > there is no reason to talk about testing/enabling FTL until bug169510 is
> > fixed.
> 
> Maybe, that ifdef thing reverted the above change.
> So, how is the current FTL AArch64 situation?

I fixed various problems over the past few weeks:
- bug #169510
- bug #170672
- bug #170891
- bug #171563

On the hardware I use, this leaves no consistent failures, but there are some flakes.

Currently, with FTL still disabled, ChakraCore.yaml/ChakraCore/test/GlobalFunctions/ParseInt1.js.default seems to fail due to a compiler bug. There's also failures in datetime and number formatting tests, that's fixable by using ICU 57.1.

I'd propose enabling FTL and WebAssembly for Linux ports at least for a shorter period of time to see how the test suite responds.

[Yusuke Suzuki]

(In reply to Zan Dobersek from comment #12)
> 
> I fixed various problems over the past few weeks:
> - bug #169510
> - bug #170672
> - bug #170891
> - bug #171563
> 
> On the hardware I use, this leaves no consistent failures, but there are
> some flakes.
> 
> Currently, with FTL still disabled,
> ChakraCore.yaml/ChakraCore/test/GlobalFunctions/ParseInt1.js.default seems
> to fail due to a compiler bug. There's also failures in datetime and number
> formatting tests, that's fixable by using ICU 57.1.
> 
> I'd propose enabling FTL and WebAssembly for Linux ports at least for a
> shorter period of time to see how the test suite responds.

I strongly support for this proposal!

[Zan Dobersek]

Created attachment 309499 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 309499 [details]
Patch

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

I would like to ask GTK folks about enabling FTL on ARM64 too.

> Source/cmake/WebKitFeatures.cmake:69
> +    if (WTF_CPU_X86_64 OR WTF_CPU_ARM64)

Is it necessary to modify cmake/OptionsWPE.cmake?

[Saam Barati]

(In reply to Zan Dobersek from comment #12)
> (In reply to Yusuke Suzuki from comment #11)
> > (In reply to comment #10)
> > > Unfortunately JSC is completely broken on AArch64 Linux because of bug169510,
> > > there is no reason to talk about testing/enabling FTL until bug169510 is
> > > fixed.
> > 
> > Maybe, that ifdef thing reverted the above change.
> > So, how is the current FTL AArch64 situation?
> 
> I fixed various problems over the past few weeks:
> - bug #169510
> - bug #170672
> - bug #170891
> - bug #171563
> 
> On the hardware I use, this leaves no consistent failures, but there are
> some flakes.
> 
> Currently, with FTL still disabled,
> ChakraCore.yaml/ChakraCore/test/GlobalFunctions/ParseInt1.js.default seems
> to fail due to a compiler bug. 
This could be related to some of my work on parseInt. Can you reproduce this easily locally? If so, can you see if it passes with DFG disabled. 

There's also failures in datetime and number
> formatting tests, that's fixable by using ICU 57.1.
> 
> I'd propose enabling FTL and WebAssembly for Linux ports at least for a
> shorter period of time to see how the test suite responds.

[Michael Catanzaro]

Comment on attachment 309499 [details]
Patch

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

>> Source/cmake/WebKitFeatures.cmake:69
>> +    if (WTF_CPU_X86_64 OR WTF_CPU_ARM64)
> 
> Is it necessary to modify cmake/OptionsWPE.cmake?

Good call. Yes and no. ENABLE_FTL_DEFAULT should not be set at all in OptionsWPE.cmake, because it's just being immediately overridden here, since the next thing OptionsWPE.cmake does is call WEBKIT_OPTION_BEGIN. So this should definitely be fixed before landing.

ENABLE_FTL_JIT should also no longer be set in OptionsWPE.cmake.

[Zan Dobersek]

Comment on attachment 309499 [details]
Patch

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

>>> Source/cmake/WebKitFeatures.cmake:69
>>> +    if (WTF_CPU_X86_64 OR WTF_CPU_ARM64)
>> 
>> Is it necessary to modify cmake/OptionsWPE.cmake?
> 
> Good call. Yes and no. ENABLE_FTL_DEFAULT should not be set at all in OptionsWPE.cmake, because it's just being immediately overridden here, since the next thing OptionsWPE.cmake does is call WEBKIT_OPTION_BEGIN. So this should definitely be fixed before landing.
> 
> ENABLE_FTL_JIT should also no longer be set in OptionsWPE.cmake.

ENABLE_FTL_DEFAULT should be removed from OptionsWPE, along with the ENABLE_FTL_JIT override.

But we also need to enable FTL_JIT and WEBASSEMBLY in build-jsc and FeatureList.pm. So I'll add that.

[Zan Dobersek]

Created attachment 309582 [details]
Patch

Updates OptionsWPE.cmake and enables FTL JIT and WebAssembly on ARM64 in build-jsc and FeatureList.pm.

[Yusuke Suzuki]

Comment on attachment 309582 [details]
Patch

r=me

[Zan Dobersek]

(In reply to Saam Barati from comment #16)
> (In reply to Zan Dobersek from comment #12)
> > (In reply to Yusuke Suzuki from comment #11)
> > > (In reply to comment #10)
> > > > Unfortunately JSC is completely broken on AArch64 Linux because of bug169510,
> > > > there is no reason to talk about testing/enabling FTL until bug169510 is
> > > > fixed.
> > > 
> > > Maybe, that ifdef thing reverted the above change.
> > > So, how is the current FTL AArch64 situation?
> > 
> > I fixed various problems over the past few weeks:
> > - bug #169510
> > - bug #170672
> > - bug #170891
> > - bug #171563
> > 
> > On the hardware I use, this leaves no consistent failures, but there are
> > some flakes.
> > 
> > Currently, with FTL still disabled,
> > ChakraCore.yaml/ChakraCore/test/GlobalFunctions/ParseInt1.js.default seems
> > to fail due to a compiler bug. 
> This could be related to some of my work on parseInt. Can you reproduce this
> easily locally? If so, can you see if it passes with DFG disabled. 
> 

It still fails if DFG is disabled.

I think it's a bug in GCC (tested with GCC 6.3), or even a completely valid optimization based on UB. The test passes if the parseInt template function has the O1 optimization level enforced through the `optimize` attribute, and fails as soon as that level is increased to O2.

Specifically, the difference in generated code that's causing the test failure can be narrowed to this pair of statements:

    number *= radix;
    number += digit;

Dunno, we should put together a test case and ask on the GCC bugzilla about the expected behavior here.

[Zan Dobersek]

(In reply to Yusuke Suzuki from comment #20)
> Comment on attachment 309582 [details]
> Patch
> 
> r=me

Thanks, I'll land the patch and see what the testers say.

[Zan Dobersek]

Comment on attachment 309582 [details]
Patch

Clearing flags on attachment: 309582

Committed r216575: <http://trac.webkit.org/changeset/216575>

[Csaba Osztrogonác]

The first test finished on the AArch64 bot after FTL is enabled.
There are only 4 extra failures introduced:
- stress/check-string-ident.js.ftl-no-cjit-b3o1
- stress/spread-calling.js.ftl-eager-no-cjit
- wasm.yaml/wasm/js-api/test_memory_constructor.js.default-wasm
- wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-eager-jettison

stress/check-string-ident.js.ftl-no-cjit-b3o1: Exception: Error: We should not have to compile this function more than once.
stress/check-string-ident.js.ftl-no-cjit-b3o1: global [e-mail]:15:20
stress/check-string-ident.js.ftl-no-cjit-b3o1: ERROR: Unexpected exit code: 3

stress/spread-calling.js.ftl-eager-no-cjit: Timed out after 207.000000 sec seconds!
stress/spread-calling.js.ftl-eager-no-cjit: Segmentation fault
stress/spread-calling.js.ftl-eager-no-cjit: ERROR: Unexpected exit code: 139

wasm.yaml/wasm/js-api/test_memory_constructor.js.default-wasm: Killed
wasm.yaml/wasm/js-api/test_memory_constructor.js.default-wasm: ERROR: Unexpected exit code: 137
FAIL: wasm.yaml/wasm/js-api/test_memory_constructor.js.default-wasm

wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-eager-jettison: Killed
wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-eager-jettison: ERROR: Unexpected exit code: 137
FAIL: wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-eager-jettison


I have no time to investigate them, feel free to pick it up if you are intereted in it.

[Zan Dobersek]

This is completed. Only one failure listed in comment #24 remains relevant, and that test is supposed to be skipped (which is not enforced properly).

[Radar WebKit Bug Importer]

<rdar://problem/35048129>