RESOLVED FIXED122263
DFG: ConstProp the pattern ValueToInt32(Bool(x)) -> Int32(x) 
https://bugs.webkit.org/show_bug.cgi?id=122263
Summary DFG: ConstProp the pattern ValueToInt32(Bool(x)) -> Int32(x)
Nadav Rotem
Reported 2013-10-03 00:26:00 PDT
DFG: ConstProp the pattern ValueToInt32(Bool(x)) -> Int32(x)
Attachments
Patch (1.98 KB, patch)
2013-10-03 00:26 PDT, Nadav Rotem 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Nadav Rotem
Comment 1 2013-10-03 00:26:43 PDT
Created attachment 213234 [details] Patch
Geoffrey Garen
Comment 2 2013-10-03 00:43:09 PDT
Comment on attachment 213234 [details] Patch r=me
WebKit Commit Bot
Comment 3 2013-10-03 01:41:48 PDT
Comment on attachment 213234 [details] Patch Clearing flags on attachment: 213234 Committed r156830: <http://trac.webkit.org/changeset/156830>
WebKit Commit Bot
Comment 4 2013-10-03 01:41:50 PDT
All reviewed patches have been landed. Closing bug.
Filip Pizlo
Comment 5 2013-10-03 07:48:58 PDT
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 6 2013-10-03 07:53:24 PDT
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.
Note You need to log in before you can comment on or make changes to this bug.