1439333 - AutoFlushICache methods are ifdef'd out for ARM64

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P3)

Description

[Lars T Hansen [:lth]]

flush() and setInhibit() are not enabled for ARM64.  As it happens, they don't need to be - setInhibit is used by nobody, and flush() is not yet used by ARM64 (probably won't need it).

However it's an antipattern to have #ifdef ARCH ... #endif without covering all ARCH.  Really there ought always to be a default #else MOZ_CRASH() case so that porting work can be less hit-and-miss.  I'll add the appropriate default cases here.

Comment 1

[Lars T Hansen [:lth]]

Ah, but ARM64 asm code *should* call AutoFlushICache::flush() when patching, and when they do that the AutoFlushICache objects that *are* created but appear to be inert in bug 1430743 will be effectful.

Comment 2

[Lars T Hansen [:lth]]

Attachment: bug1439333-autoflushicache-arm64.patch

Make ARM64 opt into existing flushing machinery.

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 8952165 [details] [diff] [review]
bug1439333-autoflushicache-arm64.patch

Review of attachment 8952165 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for fixing this footgun.

Comment 4

[Lars T Hansen [:lth]]

Attachment: bug1439333-autoflushicache-arm64-v2.patch

Rebased, etc, ready to land.  Carrying r+.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2ffffb30b31
Make AutoFlushICache::flush() do something on ARM64. r=jandem

Comment 6

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/d2ffffb30b31