[webkit-reviews] review granted: [Bug 204317] [JSC] DFG terminal's liveness should respect caller's opcodeID : [Attachment 384015] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 21 15:56:41 PST 2019

Saam Barati <sbarati at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 204317: [JSC] DFG terminal's liveness should respect caller's opcodeID

Attachment 384015: Patch


--- Comment #8 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 384015
  --> https://bugs.webkit.org/attachment.cgi?id=384015

View in context: https://bugs.webkit.org/attachment.cgi?id=384015&action=review


> Source/JavaScriptCore/ChangeLog:29
> +	   in DFG, but this is wasting.

"wasting" => "wasteful". Maybe also say "because varargs forwarding phase could
eliminate the allocation."

> Source/JavaScriptCore/ChangeLog:33
> +	   is not profitable, and (2) we need to be careful to make stack
arguments live to allow materialization of arguments objects.

why would (2) not Just Work? It seems more principled to apply this to all
calls and see if there is any bug fallout. I don't see why we should special
case varargs calls.

> Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:43
> +// <- AfterUse
> +//	  Use by exception handlers


> Source/JavaScriptCore/bytecode/BytecodeLivenessAnalysis.h:46
> +    BeforeUse = 0,

is = 0 needed for anything?

> Source/JavaScriptCore/dfg/DFGGraph.h:916
> +	   InlineCallFrame* inlineCallFrame = origin.inlineCallFrame();
> +	   CodeBlock* codeBlock = baselineCodeBlockFor(inlineCallFrame);
> +	   auto instruction =

nit: if you keep this code, you should put it inside the "if".

However, I think the more robust thing is for all inlined things, we used

> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:94
> +# After calling, calling bytecode is claiming input registers are not used.

are these comments needed?

More information about the webkit-reviews mailing list