[webkit-reviews] review denied: [Bug 118203] DFG should support ret_object_or_this : [Attachment 208160] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Aug 5 20:37:03 PDT 2013


Filip Pizlo <fpizlo at apple.com> has denied Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 118203: DFG should support ret_object_or_this
https://bugs.webkit.org/show_bug.cgi?id=118203

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

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


> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:941
> +	       m_state.setFoundConstants(true);

This looks borked. Why aren't you setting the result of the node?

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:2823
>	   case op_ret:

It looks like this would work better with a helper function, rather than having
a bunch of opcode checks.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:3263
> +	   cellResult(thisGPR, node);

Yeah if a node returns a value, as this one clearly does, you need to have the
abstract interpreter indicate what it returns. Right now you're leaving the
result clear which is definitely wrong.


More information about the webkit-reviews mailing list