[webkit-reviews] review denied: [Bug 129051] GetMyArgumentsByLength in FTL doesn't work with inline code : [Attachment 224890] Fixed GetMyArgumentsLength in FTL to work with inline code

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has denied Matthew Mirman <mmirman at apple.com>'s
request for review:
Bug 129051: GetMyArgumentsByLength in FTL doesn't work with inline code
https://bugs.webkit.org/show_bug.cgi?id=129051

Attachment 224890: Fixed GetMyArgumentsLength in FTL to work with inline code
https://bugs.webkit.org/attachment.cgi?id=224890&action=review

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


> Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:300
> +	   case GetMyArgumentsLengthSafe:

I don't think you want to combine these two.  The purpose of
GetMyArgumenstLengthSafe is to have a slow path, rather than a speculation, for
the arguments-created case.  See what the DFG does, notice the slow path call:

    case GetMyArgumentsLengthSafe: {
	GPRTemporary result(this);
	GPRReg resultGPR = result.gpr();
	
	JITCompiler::Jump created = m_jit.branchTest64(
	    JITCompiler::NonZero,
	    JITCompiler::addressFor(
	       
m_jit.graph().machineArgumentsRegisterFor(node->origin.semantic)));
	
	if (node->origin.semantic.inlineCallFrame) {
	    m_jit.move(
	       
Imm64(JSValue::encode(jsNumber(node->origin.semantic.inlineCallFrame->arguments
.size() - 1))),
		resultGPR);
	} else {
	    m_jit.load32(JITCompiler::payloadFor(JSStack::ArgumentCount),
resultGPR);
	    m_jit.sub32(TrustedImm32(1), resultGPR);
	    m_jit.or64(GPRInfo::tagTypeNumberRegister, resultGPR);
	}
	
	// FIXME: the slow path generator should perform a forward speculation
that the
	// result is an integer. For now we postpone the speculation by having
this return
	// a JSValue.
	
	addSlowPathGenerator(
	    slowPathCall(
		created, this, operationGetArgumentsLength, resultGPR,
	       
m_jit.graph().machineArgumentsRegisterFor(node->origin.semantic).offset()));
	
	jsValueResult(resultGPR, node);
	break;
    }