Bug 183471 - Disable 32-bit DFG JIT on Darwin and completely on Windows
Summary: Disable 32-bit DFG JIT on Darwin and completely on Windows
Status: RESOLVED LATER
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-08 12:25 PST by Filip Pizlo
Modified: 2018-03-09 11:48 PST (History)
9 users (show)

See Also:


Attachments
the patch (2.25 KB, patch)
2018-03-08 13:29 PST, Filip Pizlo
keith_miller: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2018-03-08 12:25:14 PST
I'm about to land Spectre work that will break 32-bit DFG.  So, let's disable it on Darwin.

I recommend finally disabling the 32-bit DFG on other platforms, since it's not being tested or maintained.
Comment 1 Filip Pizlo 2018-03-08 13:29:18 PST
Created attachment 335333 [details]
the patch
Comment 2 Keith Miller 2018-03-08 13:49:23 PST
Comment on attachment 335333 [details]
the patch

r=me.
Comment 3 Guillaume Emont 2018-03-08 14:11:24 PST
(In reply to Filip Pizlo from comment #0)
> I'm about to land Spectre work that will break 32-bit DFG.  So, let's
> disable it on Darwin.
> 
> I recommend finally disabling the 32-bit DFG on other platforms, since it's
> not being tested or maintained.

While optimizations are not actively added like they are for 64-bit, I think this last statement is a clear exaggeration. We maintain testbots for jsconly mips and armv7, and gtk+/x86. We fix issues that arise, and have downstream deployments of these, where we have use cases where DFG makes a strong difference. I explained all that in a mailing list thread[1] for any reader that wants more context.

We also plan on fixing the issues arising from the most recent/upcoming changes that break things on these platforms, while working on additional Spectre mitigations for them.

[1] https://lists.webkit.org/pipermail/webkit-dev/2018-February/029870.html
Comment 4 Guillaume Emont 2018-03-08 14:16:30 PST
Comment on attachment 335333 [details]
the patch

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

> Source/WTF/wtf/Platform.h:790
> +#if (CPU(ARM_THUMB2)) && (PLATFORM(GTK) || PLATFORM(WPE))

I believe it should be:
/* Enable the DFG JIT on ARMv7 and MIPS. Only tested on GTK+/WPE Linux. */
#if (CPU(ARM_THUMB2) || CPU(MIPS)) && (PLATFORM(GTK) || PLATFORM(WPE))

Though we might need something slightly different for it to work on jsconly. Maybe it's time to add a PLATFORM(JSCONLY)? Or should we just use OS(LINUX)?
Comment 5 Yusuke Suzuki 2018-03-09 10:16:14 PST
(In reply to Guillaume Emont from comment #4)
> Comment on attachment 335333 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=335333&action=review
> 
> > Source/WTF/wtf/Platform.h:790
> > +#if (CPU(ARM_THUMB2)) && (PLATFORM(GTK) || PLATFORM(WPE))
> 
> I believe it should be:
> /* Enable the DFG JIT on ARMv7 and MIPS. Only tested on GTK+/WPE Linux. */
> #if (CPU(ARM_THUMB2) || CPU(MIPS)) && (PLATFORM(GTK) || PLATFORM(WPE))
> 
> Though we might need something slightly different for it to work on jsconly.
> Maybe it's time to add a PLATFORM(JSCONLY)? Or should we just use OS(LINUX)?

I'm OK for adding PLATFORM(JSCONLY) as long as it is only used to control compile time flags in Platform.h. It should not be used in the other places.
Comment 6 Filip Pizlo 2018-03-09 10:50:53 PST
We won't do this for now.