[webkit-reviews] review granted: [Bug 213193] [JSC] add machinery to disable JIT tiers when experimental features are enabled : [Attachment 401894] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Caitlin Potter (:caitp)
<caitp at igalia.com>'s request for review:
Bug 213193: [JSC] add machinery to disable JIT tiers when experimental features
are enabled
https://bugs.webkit.org/show_bug.cgi?id=213193

Attachment 401894: Patch

https://bugs.webkit.org/attachment.cgi?id=401894&action=review




--- Comment #2 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 401894
  --> https://bugs.webkit.org/attachment.cgi?id=401894
Patch

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

r=me

> Source/JavaScriptCore/runtime/OptionsList.h:562
> +    BaselineOnly = 0,
> +    SupportsDFG = 1,
> +    SupportsFTL = 2,

Can you express the values as 1 << 0, 1 << 1, 1 << 2 instead?  That will make
it clearer that these will be used as bit masks.  Let's also rename
BaselineOnly to SupportsBaseline because it is conceivable that an experimental
feature will be landed with LLInt only support.

Or do we want to maintain a minimum requirement that LLInt and Baseline be
supported before an experimental feature can be turned on at all?  Since we do
test with LLInt only and Baseline w/o LLInt, it might be easier to go with
LLIntAndBaselineOnly as the minimum so that we'll have less Options to check. 
If you prefer this route, please rename BaselineOnly to LLIntAndBaselineOnly,
and keep its values 0, and express the other two as 1 << 0, and 1 << 1.  I
think this might be the better way to go.