[webkit-reviews] review granted: [Bug 131906] Take block execution count estimates into account when voting double : [Attachment 229754] the patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 131906: Take block execution count estimates into account when voting
double
https://bugs.webkit.org/show_bug.cgi?id=131906

Attachment 229754: the patch
https://bugs.webkit.org/attachment.cgi?id=229754&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=229754&action=review


r=me

> Source/JavaScriptCore/dfg/DFGNodeFlags.h:63
> +#define NodeBytecodeReallyWantsInt	    0x2000 // The result of this
computation is known to be used in a context that strongly prefers integer
values, to the point that we should avoid using doubles if at all possible.

I'm not a big fan of this name. "Really" doesn't do much to explain the
difference from UsesAsInt.

How about "NodeBytecodeUsesAsArrayIndex"?

> Source/JavaScriptCore/dfg/DFGOSREntrypointCreationPhase.cpp:86
> -	   BasicBlock* newRoot = insertionSet.insert(0, PNaN);
> +	   BasicBlock* newRoot = insertionSet.insert(0, 1);

Why 1 instead of NaN? I thought one goal of this patch was to keep the entry
point as NaN, and require clients to be NaN-friendly.