Bug 122263 – DFG: ConstProp the pattern ValueToInt32(Bool(x)) -> Int32(x)

• Component: New Bugs
• Status: RESOLVED FIXED

[Nadav Rotem]

DFG: ConstProp the pattern ValueToInt32(Bool(x)) -> Int32(x)

[Nadav Rotem]

Created attachment 213234 [details]
Patch

[Geoffrey Garen]

Comment on attachment 213234 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 213234 [details]
Patch

Clearing flags on attachment: 213234

Committed r156830: <http://trac.webkit.org/changeset/156830>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Filip Pizlo]

Comment on attachment 213234 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:297
> +                setConstant(node, JSValue(child.asBoolean()));

Is the goal here to convert the Boolean to a number?  If so, does JSValue(bool) accomplish this?  My guesses that if it does, it's sort of by luck and we'd want to call jsNumber() directly.

[Filip Pizlo]

Comment on attachment 213234 [details]
Patch

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

>> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:297
>> +                setConstant(node, JSValue(child.asBoolean()));
> 
> Is the goal here to convert the Boolean to a number?  If so, does JSValue(bool) accomplish this?  My guesses that if it does, it's sort of by luck and we'd want to call jsNumber() directly.

Ugh, yeah we don't have a JSValue(bool) overload. So this works - but I think it would be better to use jsNumber() here and in the case above. We've been considering getting rid of the JSValue(number) overloads because you can get quite confused in corner cases.