[webkit-reviews] review granted: [Bug 79554] DFG should support activations and nested functions : [Attachment 128844] the patch

[bugzilla-daemon at webkit.org]

Oliver Hunt <oliver at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 79554: DFG should support activations and nested functions
https://bugs.webkit.org/show_bug.cgi?id=79554

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

------- Additional Comments from Oliver Hunt <oliver at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=128844&action=review


Basically r=me, but it would be nice if you could try to remove the duplicated
code :-(

Also we probably want a bug to track to do function instantiation inline like
we do in the baseline JIT.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:3540
> +    case CreateActivation: {
> +	   JSValueOperand value(this, node.child1());
> +	   GPRTemporary result(this, value);
> +	   
> +	   GPRReg valueTagGPR = value.tagGPR();
> +	   GPRReg valuePayloadGPR = value.payloadGPR();
> +	   GPRReg resultGPR = result.gpr();
> +	   
> +	   m_jit.move(valuePayloadGPR, resultGPR);
> +	   
> +	   JITCompiler::Jump alreadyCreated =
m_jit.branch32(JITCompiler::NotEqual, valueTagGPR,
TrustedImm32(JSValue::EmptyValueTag));
> +	   
> +	   silentSpillAllRegisters(resultGPR);
> +	   callOperation(operationCreateActivation, resultGPR);
> +	   silentFillAllRegisters(resultGPR);
> +	   
> +	   alreadyCreated.link(&m_jit);
> +	   
> +	   cellResult(resultGPR, m_compileIndex);
> +	   break;
> +    }
> +	   
> +    case TearOffActivation: {
> +	   JSValueOperand value(this, node.child1());
> +	   GPRTemporary result(this, value);
> +	   
> +	   GPRReg valueTagGPR = value.tagGPR();
> +	   GPRReg valuePayloadGPR = value.payloadGPR();
> +	   
> +	   JITCompiler::Jump notCreated = m_jit.branch32(JITCompiler::Equal,
valueTagGPR, TrustedImm32(JSValue::EmptyValueTag));
> +	   
> +	   silentSpillAllRegisters(InvalidGPRReg);
> +	   callOperation(operationTearOffActivation, valuePayloadGPR);
> +	   silentFillAllRegisters(InvalidGPRReg);
> +	   
> +	   notCreated.link(&m_jit);
> +	   
> +	   noResult(m_compileIndex);
> +	   break;
> +    }
> +	   
> +    case NewFunctionNoCheck: {
> +	   GPRResult result(this);
> +	   GPRReg resultGPR = result.gpr();
> +	   flushRegisters();
> +	   callOperation(
> +	       operationNewFunction, resultGPR,
m_jit.codeBlock()->functionDecl(node.functionDeclIndex()));
> +	   cellResult(resultGPR, m_compileIndex);
> +	   break;
> +    }

It feels like these should be entirely sharable between  32bit and 64bit
implementations