[webkit-reviews] review granted: [Bug 135593] Using +s to convert a string to a number badly confuses the DFG : [Attachment 236304] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Aug 21 15:20:00 PDT 2014
Filip Pizlo <fpizlo at apple.com> has granted Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 135593: Using +s to convert a string to a number badly confuses the DFG
https://bugs.webkit.org/show_bug.cgi?id=135593
Attachment 236304: Patch
https://bugs.webkit.org/attachment.cgi?id=236304&action=review
------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=236304&action=review
R=me. It would be slightly better if this also included the FTL implementation
of ToNumber. Also, a test would be cool. I'm fine with the FTL implementation
being a separate patch. As a test, maybe you could just land your huffman
thing, if you haven't already? Even better, land both huffman, and a
microbenchmark for +s.
> Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:1283
> + void fixupToNumber(Node* node)
> + {
> + if (node->child1()->shouldSpeculateInt32()) {
> + fixIntOrBooleanEdge(node->child1());
> + node->convertToIdentity();
> + return;
> + }
> +
> + if (node->child1()->shouldSpeculateMachineInt()) {
> + fixEdge<Int52RepUse>(node->child1());
> + node->convertToIdentity();
> + node->setResult(NodeResultInt52);
> + return;
> + }
> +
> + if (node->child1()->shouldSpeculateNumber()) {
> + fixDoubleOrBooleanEdge(node->child1());
> + node->convertToIdentity();
> + node->setResult(NodeResultDouble);
> + return;
> + }
> + }
> +
You only call this in one place so you might as well inline it. :-)
fixupToPrimitive() had a method because we would call it in more than one
place. Hopefully that's still the case, or else someone should probably inline
it.
More information about the webkit-reviews
mailing list