Bug 161895 - [JSC] Make the rounding-related nodes support any type
Summary: [JSC] Make the rounding-related nodes support any type
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
Depends on: 162021
Blocks:
  Show dependency treegraph
 
Reported: 2016-09-13 01:07 PDT by Benjamin Poulain
Modified: 2016-09-19 17:50 PDT (History)
8 users (show)

See Also:


Attachments
Patch (73.62 KB, patch)
2016-09-13 01:50 PDT, Benjamin Poulain
no flags Details | Formatted Diff | Diff
Patch (78.13 KB, patch)
2016-09-19 16:23 PDT, Benjamin Poulain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2016-09-13 01:07:59 PDT
[JSC] Make the rounding-related nodes support any type
Comment 1 Benjamin Poulain 2016-09-13 01:50:06 PDT
Created attachment 288680 [details]
Patch
Comment 2 Geoffrey Garen 2016-09-13 11:03:36 PDT
Comment on attachment 288680 [details]
Patch

r=me
Comment 3 WebKit Commit Bot 2016-09-14 14:26:36 PDT
Comment on attachment 288680 [details]
Patch

Clearing flags on attachment: 288680

Committed r205931: <http://trac.webkit.org/changeset/205931>
Comment 4 WebKit Commit Bot 2016-09-14 14:26:40 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Ryan Haddad 2016-09-15 08:09:27 PDT
(In reply to comment #5)
> The new tests fail on 32 bit platforms, see build.webkit.org for details:

Filed https://bugs.webkit.org/show_bug.cgi?id=162016
Comment 7 WebKit Commit Bot 2016-09-15 09:50:35 PDT
Re-opened since this is blocked by bug 162021
Comment 8 Benjamin Poulain 2016-09-19 16:23:50 PDT
Created attachment 289275 [details]
Patch
Comment 9 Geoffrey Garen 2016-09-19 16:28:50 PDT
Comment on attachment 289275 [details]
Patch

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

r=me

> Source/JavaScriptCore/jsc.cpp:1739
> +template<typename ValueType>
> +typename std::enable_if<!std::is_fundamental<ValueType>::value>::type addOption(VM&, JSObject*, Identifier, ValueType) { }
> +
> +template<typename ValueType>
> +typename std::enable_if<std::is_fundamental<ValueType>::value>::type addOption(VM& vm, JSObject* optionsObject, Identifier identifier, ValueType value)
> +{
> +    optionsObject->putDirect(vm, identifier, JSValue(value));
> +}
> +
> +EncodedJSValue JSC_HOST_CALL functionJSCOptions(ExecState* exec)
> +{
> +    JSObject* optionsObject = constructEmptyObject(exec);
> +#define FOR_EACH_OPTION(type_, name_, defaultValue_, availability_, description_) \
> +    addOption(exec->vm(), optionsObject, Identifier::fromString(exec, #name_), Options::name_());
> +    JSC_OPTIONS(FOR_EACH_OPTION)
> +#undef FOR_EACH_OPTION
> +    return JSValue::encode(optionsObject);
> +}

Did you mean to include this?
Comment 10 Benjamin Poulain 2016-09-19 16:30:17 PDT
(In reply to comment #9)
> Comment on attachment 289275 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=289275&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/jsc.cpp:1739
> > +template<typename ValueType>
> > +typename std::enable_if<!std::is_fundamental<ValueType>::value>::type addOption(VM&, JSObject*, Identifier, ValueType) { }
> > +
> > +template<typename ValueType>
> > +typename std::enable_if<std::is_fundamental<ValueType>::value>::type addOption(VM& vm, JSObject* optionsObject, Identifier identifier, ValueType value)
> > +{
> > +    optionsObject->putDirect(vm, identifier, JSValue(value));
> > +}
> > +
> > +EncodedJSValue JSC_HOST_CALL functionJSCOptions(ExecState* exec)
> > +{
> > +    JSObject* optionsObject = constructEmptyObject(exec);
> > +#define FOR_EACH_OPTION(type_, name_, defaultValue_, availability_, description_) \
> > +    addOption(exec->vm(), optionsObject, Identifier::fromString(exec, #name_), Options::name_());
> > +    JSC_OPTIONS(FOR_EACH_OPTION)
> > +#undef FOR_EACH_OPTION
> > +    return JSValue::encode(optionsObject);
> > +}
> 
> Did you mean to include this?

Yep. I exposed the JSC options to the tests to change the compilation count limits based on them.

In particular, when we use useMaximalFlushInsertionPhase, we should compile exactly 2 times in the worst case (once to discover the negative zero, then once with the right types).

Filip asked me to update the existing tests to account for FTL-eager too (will do in a follow up).
Comment 11 WebKit Commit Bot 2016-09-19 17:50:52 PDT
Comment on attachment 289275 [details]
Patch

Clearing flags on attachment: 289275

Committed r206134: <http://trac.webkit.org/changeset/206134>
Comment 12 WebKit Commit Bot 2016-09-19 17:50:57 PDT
All reviewed patches have been landed.  Closing bug.