Bug 161895 – [JSC] Make the rounding-related nodes support any type

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Benjamin Poulain]

[JSC] Make the rounding-related nodes support any type

[Benjamin Poulain]

Created attachment 288680 [details]
Patch

[Geoffrey Garen]

Comment on attachment 288680 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 288680 [details]
Patch

Clearing flags on attachment: 288680

Committed r205931: <http://trac.webkit.org/changeset/205931>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

(In reply to comment #3)
> Comment on attachment 288680 [details]
> Patch
> 
> Clearing flags on attachment: 288680
> 
> Committed r205931: <http://trac.webkit.org/changeset/205931>

The new tests fail on 32 bit platforms, see build.webkit.org for details:
- https://build.webkit.org/builders/Apple%20El%20Capitan%2032-bit%20JSC%20%28BuildAndTest%29/builds/3699
- https://build.webkit.org/builders/Apple%20Yosemite%2032-bit%20JSC%20%28BuildAndTest%29/builds/10652
- https://build.webkit.org/builders/GTK%20Linux%2032-bit%20Release/builds/64031
...

[Ryan Haddad]

(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

[WebKit Commit Bot]

Re-opened since this is blocked by bug 162021

[Benjamin Poulain]

Created attachment 289275 [details]
Patch

[Geoffrey Garen]

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?

[Benjamin Poulain]

(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).

[WebKit Commit Bot]

Comment on attachment 289275 [details]
Patch

Clearing flags on attachment: 289275

Committed r206134: <http://trac.webkit.org/changeset/206134>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.