Bug 138315 – [JSC] Erratum (835769) in Cortex-A53

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Akos Kiss]

There is an erratum (835769) in some Cortex-A53, which can only be worked around at code generation: It is possible for an A64 64-bit multiply-accumulate instruction to generate an incorrect result when immediately preceded by a memory instruction. In WebKit, that means 4 affected stages, potentially:

1. compilation with gcc or clang
2. processing LLInt asm sources with the offline assembler
3. MacroAssembler
4. LLVM in FTL

1. Fixes for both gcc and clang have already been submitted:
https://gcc.gnu.org/ml/gcc-cvs/2014-10/msg00331.html
https://gcc.gnu.org/ml/gcc-cvs/2014-10/msg00332.html
https://gcc.gnu.org/ml/gcc-cvs/2014-10/msg00333.html
https://gcc.gnu.org/ml/gcc-cvs/2014-10/msg00335.html
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20141006/239368.html
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20141006/116322.html
So, if building WebKit for Cortex-A53, the build system should detect whether the compiler tool chain has support for the workaround and apply it if available.

2 & 3. Offlineasm & MacroAssembler should be amended with the workaround.

4. LLVM trunk already has the workaround, so as soon as ARM64 (for Linux) has FTL working, WebKit can rely on LLVM generating correct code.

[Filip Pizlo]

Please make sure that whatever this fix is, it is possible to disable it.  It should be disabled by default on Darwin.

[Akos Kiss]

Created attachment 240860 [details]
Proposed patch.

Built with Tools/Scripts/build-jsc --gtk --cmakeargs="-DWTF_CPU_ARM64_CORTEXA53=ON", no regression.

Some extra comments:

1) The ARM64 assembler runs into the nop generating code path in 162 jsc tests. E.g., in sunspider-1.0/string-fasta.js.default it generates something like this below:

      57:< 1:-4>        GetLocal(@1, JS, Int32, arg1(B<Int32>/FlushedInt32), machine:arg1, R:Variables(7), bc#98) predicting Int32
            0x7fa947f30c:    ldur   w0, [fp, #56]
      58:< 2:-4>        ArithMul(Int32:@45, Int32:@57, Number, Int32, CheckOverflowAndNegativeZero, bc#98)
            0x7fa947f310:    nop   
            0x7fa947f314:    smaddl x1, w2, w0, x31


2) For now, offlineasm generates no extra nops, since the LLInt asm sources contain no patterns which would trigger the nop-inserting code. The offlineasm part of the patch has been tested with crafted code (of no real use), however, and the generated assembly was OK. E.g.:

    "\tldr x2, [x13, #16]\n"                                 // /home/akiss/devel/WebKit/Source/JavaScriptCore/llint/LowLevelInterpreter.asm:1375
#if CPU(ARM64_CORTEXA53)
    "\tnop\n"                                                // /home/akiss/devel/WebKit/Source/JavaScriptCore/llint/LowLevelInterpreter.asm:1376
#endif
    "\tmadd w1, w1, w1, wzr\n"                               // /home/akiss/devel/WebKit/Source/JavaScriptCore/llint/LowLevelInterpreter.asm:1376

[Filip Pizlo]

Comment on attachment 240860 [details]
Proposed patch.

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

LGTM with the one suggestion.

> Source/JavaScriptCore/assembler/ARM64Assembler.h:1571
> +#if CPU(ARM64_CORTEXA53)

You could remove this #if and instead have an #if inside the nopCortexA53Fix835769() method. That would beautify the code quite a bit without any change in perf or behavior.

[Akos Kiss]

Created attachment 240866 [details]
Updated patch

Thanks for the suggestion. Applied.

[WebKit Commit Bot]

Comment on attachment 240866 [details]
Updated patch

Clearing flags on attachment: 240866

Committed r175514: <http://trac.webkit.org/changeset/175514>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.