[webkit-reviews] review granted: [Bug 118546] CodeBlocks should be able to determine bytecode liveness : [Attachment 208147] Patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has granted Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 118546: CodeBlocks should be able to determine bytecode liveness
https://bugs.webkit.org/show_bug.cgi?id=118546

Attachment 208147: Patch
https://bugs.webkit.org/attachment.cgi?id=208147&action=review

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=208147&action=review


> Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:75
> +	   && (!numberOfCapturedVariables(codeBlock) // If we have no captured
variables, we're good to go.
> +	   || (operand < captureStart(codeBlock) // If we have captured
variables but the operand is outside of the captured range, we're good to go.
> +	   || operand >= captureEnd(codeBlock)));

Indent the || four spaces, since it's within the parantheses on line 73.  Also,
doesn't CodeBlock have an isCaptured() method?	If it doesn't, it might be cool
to give it that.

> Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:501
> +    for (unsigned i = 0; i < basicBlocks.size(); i++) {
> +	   if (leaderOffset == basicBlocks[i]->leaderBytecodeOffset())
> +	       return basicBlocks[i].get();
> +    }

WTF binary search?

> Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.cpp:512
> +    for (unsigned i = 0; i < basicBlocks.size(); i++) {
> +	   BytecodeBasicBlock* block = basicBlocks[i].get();
> +	   if (bytecodeOffset >= block->leaderBytecodeOffset() &&
bytecodeOffset < block->leaderBytecodeOffset() + block->totalBytecodeLength())
> +	       return block;
> +    }
> +    return 0;

ditto