RESOLVED FIXED 161639
MapHash should do constant folding when it has a constant argument and its legal to hash that value
https://bugs.webkit.org/show_bug.cgi?id=161639
Summary MapHash should do constant folding when it has a constant argument and its le...
Saam Barati
Reported 2016-09-06 12:14:54 PDT
....
Attachments
WIP (4.05 KB, patch)
2016-09-11 18:34 PDT, Saam Barati
no flags
patch (8.27 KB, patch)
2016-09-11 23:10 PDT, Saam Barati
no flags
patch (8.34 KB, patch)
2016-09-11 23:11 PDT, Saam Barati
fpizlo: review+
patch for landing (8.51 KB, patch)
2016-09-12 11:38 PDT, Saam Barati
no flags
Saam Barati
Comment 1 2016-09-11 18:34:05 PDT
Created attachment 288544 [details] WIP I think this is the patch. Lets see what EWS thinks.
Saam Barati
Comment 2 2016-09-11 23:10:04 PDT
Saam Barati
Comment 3 2016-09-11 23:11:29 PDT
Filip Pizlo
Comment 4 2016-09-12 10:57:18 PDT
Comment on attachment 288554 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=288554&action=review > Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:987 > + // We don't want this JSValue to devolve into a double so > + // we cast it to an int32_t. When we actually use this > + // value, we will use it as if it were unsigned. > + setConstant(node, JSValue(static_cast<int32_t>(*hash))); Does MapHash ordinarily return an int32? If so, then this comment would be less confusing as: "Although C++ code uses uint32_t for the hash, the closest type in DFG IR is Int32 and that's what MapHash returns. So, we have to cast to int32_t to avoid large unsigned values becoming doubles." Also, I would use jsNumber(int32_t) instead of JSValue().
Saam Barati
Comment 5 2016-09-12 11:38:59 PDT
Created attachment 288594 [details] patch for landing
WebKit Commit Bot
Comment 6 2016-09-12 14:06:30 PDT
Comment on attachment 288594 [details] patch for landing Clearing flags on attachment: 288594 Committed r205819: <http://trac.webkit.org/changeset/205819>
WebKit Commit Bot
Comment 7 2016-09-12 14:06:36 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.