[webkit-reviews] review granted: [Bug 118750] fourthTier: NaturalLoops should be able to quickly answer questions like "what loops own this basic block" : [Attachment 206819] the patch

[bugzilla-daemon at webkit.org]

Mark Hahnenberg <mhahnenberg at apple.com> has granted Filip Pizlo
<fpizlo at apple.com>'s request for review:
Bug 118750: fourthTier: NaturalLoops should be able to quickly answer questions
like "what loops own this basic block"
https://bugs.webkit.org/show_bug.cgi?id=118750

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

------- Additional Comments from Mark Hahnenberg <mhahnenberg at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=206819&action=review


r=me with comments.

> Source/JavaScriptCore/dfg/DFGNaturalLoops.cpp:190
> +	       ASSERT(simpleLoopsOf == fancyLoopsOf);

RELEASE_ASSERT

> Source/JavaScriptCore/dfg/DFGNaturalLoops.h:116
> +	   for (
> +	       const NaturalLoop* loop = innerMostLoopOf(block);
> +	       loop;
> +	       loop = innerMostOuterLoop(*loop)) {
> +	       if (loop->header() == block)
> +		   return loop;

Don't need to for-loop. Modified version looks good.

> Source/JavaScriptCore/dfg/DFGPlan.cpp:192
> +    // to self-validate. Now is as good a time as any, to do this.

No comma :-)