[webkit-reviews] review denied: [Bug 129355] Compilation policy management belongs in operationOptimize(), not the DFG Driver. : [Attachment 225270] patch 2:=?UTF-8?Q?=20applied=20Filip=E2=80=99s=20feedback?=.

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has denied Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 129355: Compilation policy management belongs in operationOptimize(), not
the DFG Driver.
https://bugs.webkit.org/show_bug.cgi?id=129355

Attachment 225270: patch 2: applied Filip’s feedback.
https://bugs.webkit.org/attachment.cgi?id=225270&action=review

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


Almost there.

> Source/JavaScriptCore/dfg/DFGCapabilities.cpp:45
> +    if (!Options::useDFGJIT()
> +	   || !MacroAssembler::supportsFloatingPoint()
> +	   ||
!Options::bytecodeRangeToDFGCompile().isInRange(codeBlock->instructionCount()))

> +	   return false;
> +    return true;

Why not just turn this into a single return statement?

> Source/JavaScriptCore/jit/JITOperations.cpp:1065
> +	   codeBlock->optimizeAfterWarmUp();

You could call updateAllPredictions() here.

> Source/JavaScriptCore/jit/JITOperations.cpp:1071
> +	   codeBlock->optimizeAfterWarmUp();

And here.

You could even have a helper that calls both optimizeAfterWarmUp and
updateAllPredictions.