[webkit-reviews] review denied: [Bug 117103] DFG should populate frame bytecodeOffsets on OSR exit : [Attachment 203492] updated patch.

[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 117103: DFG should populate frame bytecodeOffsets on OSR exit
https://bugs.webkit.org/show_bug.cgi?id=117103

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

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


> Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp:44
> +static size_t callOpcodeLength(VM* vm, CodeBlock* codeBlock, unsigned
bytecodeIndex)
> +{
> +    Opcode opcode = codeBlock->instructions()[bytecodeIndex].u.opcode;
> +    OpcodeID opcodeID = vm->interpreter->getOpcodeID(opcode);
> +    RELEASE_ASSERT(opcodeID == op_call || opcodeID == op_call_eval ||
opcodeID == op_call_varargs);
> +    ASSERT(OPCODE_LENGTH(op_call) == OPCODE_LENGTH(op_call_eval));
> +    return UNLIKELY(opcodeID == op_call_varargs) ?
OPCODE_LENGTH(op_call_varargs) : OPCODE_LENGTH(op_call);
> +}

This is wrong.	You forgot about op_construct.

Also the name "callOpcodeLength" is really weird.  Think of a better name.

> Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp:44
> +static size_t callOpcodeLength(VM* vm, CodeBlock* codeBlock, unsigned
bytecodeIndex)
> +{
> +    Opcode opcode = codeBlock->instructions()[bytecodeIndex].u.opcode;
> +    OpcodeID opcodeID = vm->interpreter->getOpcodeID(opcode);
> +    RELEASE_ASSERT(opcodeID == op_call || opcodeID == op_call_eval ||
opcodeID == op_call_varargs);
> +    ASSERT(OPCODE_LENGTH(op_call) == OPCODE_LENGTH(op_call_eval));
> +    return UNLIKELY(opcodeID == op_call_varargs) ?
OPCODE_LENGTH(op_call_varargs) : OPCODE_LENGTH(op_call);
> +}

Ditto.