Summary: | [JSC] Erratum (835769) in Cortex-A53 | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Akos Kiss <akiss> | ||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, dbates, fpizlo, gyuyoung.kim, loki, msaboff, rakuco, ryuan.choi, sergio | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=197192 | ||||||||
Attachments: |
|
Description
Akos Kiss
2014-11-03 10:22:22 PST
Please make sure that whatever this fix is, it is possible to disable it. It should be disabled by default on Darwin. 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
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. Created attachment 240866 [details]
Updated patch
Thanks for the suggestion. Applied.
Comment on attachment 240866 [details] Updated patch Clearing flags on attachment: 240866 Committed r175514: <http://trac.webkit.org/changeset/175514> All reviewed patches have been landed. Closing bug. |