From 20b8aeea2ff81b36bf6fef9bd439caf052d68d2b Mon Sep 17 00:00:00 2001 From: jan Date: Fri, 15 Mar 2013 13:19:52 -0700 Subject: [PATCH] Consolidate ContDone into ContRetC Merge ContDone+ContExit into ContRetC with added support of passing results. This variable passing mechanism is not exposed to the PHP as the ReturnStatements in generators do not contain result expression. However, this is exposed by restored hphp_continuation_done() built-in to allow experimentation. The idea is that once we introduce ContYield opcode (merge of all opcodes used by YieldExpression), we could change ContRetC and ContYield to leave result and done-status on the stack and leave it up to the caller (ContNext/ContSend/ContRaise) to fill in Continuation fields. This will make these opcodes more generic and useful for other things, while allowing us to move some properties to the VM and kill opcodes like ContCurrent. --- hphp/compiler/analysis/emitter.cpp | 34 ++++++++++++------- hphp/doc/bytecode.specification | 8 +++-- hphp/runtime/vm/bytecode.cpp | 10 ++++-- hphp/runtime/vm/hhbc.h | 2 +- .../vm/translator/hopt/hhbctranslator.cpp | 16 ++++++--- .../vm/translator/hopt/hhbctranslator.h | 3 +- .../vm/translator/hopt/irtranslator.cpp | 4 +-- hphp/runtime/vm/translator/translator-x64.cpp | 27 ++++++++++----- hphp/runtime/vm/translator/translator-x64.h | 3 +- hphp/runtime/vm/translator/translator.cpp | 4 +-- hphp/runtime/vm/translator/translator.h | 1 + 11 files changed, 74 insertions(+), 38 deletions(-) diff --git a/hphp/compiler/analysis/emitter.cpp b/hphp/compiler/analysis/emitter.cpp index cd7c301be..40fd7aee9 100644 --- a/hphp/compiler/analysis/emitter.cpp +++ b/hphp/compiler/analysis/emitter.cpp @@ -2180,12 +2180,6 @@ bool EmitterVisitor::visitImpl(ConstructPtr node) { case Statement::KindOfReturnStatement: { ReturnStatementPtr r(static_pointer_cast(node)); bool retV = false; - if (m_curFunc->isGenerator()) { - assert(m_evalStack.size() == 0); - e.ContDone(); - e.ContExit(); - return false; - } if (visit(r->getRetExp())) { if (r->getRetExp()->getContext() & Expression::RefValue) { emitConvertToVar(e); @@ -2199,6 +2193,16 @@ bool EmitterVisitor::visitImpl(ConstructPtr node) { emitFreePendingIters(e); e.Null(); } + + if (m_curFunc->isGenerator()) { + assert(!retV); + m_metaInfo.addKnownDataType( + KindOfObject, false, m_ue.bcPos(), false, 1); + assert(m_evalStack.size() == 1); + e.ContRetC(); + return false; + } + if (r->isGuarded()) { m_metaInfo.add(m_ue.bcPos(), Unit::MetaInfo::GuardedThis, false, 0, 0); @@ -3163,9 +3167,12 @@ bool EmitterVisitor::visitImpl(ConstructPtr node) { e.CreateCont(callGetArgs, nameStr); return true; } else if (call->isCompilerCallToFunction("hphp_continuation_done")) { - inputIsAnObject(0); - e.ContDone(); - e.ContExit(); + assert(params && params->getCount() == 1); + visit((*params)[0]); + emitConvertToCell(e); + inputIsAnObject(1); + assert(m_evalStack.size() == 1); + e.ContRetC(); e.Null(); return true; } else if ((call->isCallToFunction("class_exists") || @@ -5379,12 +5386,13 @@ void EmitterVisitor::emitPostponedMeths() { // If the current position in the bytecode is reachable, emit code to // return null if (currentPositionIsReachable()) { + e.Null(); if (p.m_meth->getFunctionScope()->isGenerator()) { - assert(m_evalStack.size() == 0); - e.ContDone(); - e.ContExit(); + m_metaInfo.addKnownDataType( + KindOfObject, false, m_ue.bcPos(), false, 1); + assert(m_evalStack.size() == 1); + e.ContRetC(); } else { - e.Null(); if ((p.m_meth->getStmts() && p.m_meth->getStmts()->isGuarded())) { m_metaInfo.add(m_ue.bcPos(), Unit::MetaInfo::GuardedThis, false, 0, 0); diff --git a/hphp/doc/bytecode.specification b/hphp/doc/bytecode.specification index 5fc1a0d30..d1759d426 100644 --- a/hphp/doc/bytecode.specification +++ b/hphp/doc/bytecode.specification @@ -3650,10 +3650,12 @@ ContReceive [] -> [C] sent to the continuation will be pushed on the stack. The value will be no longer referenced by the Continuation object. -ContDone [] -> [] +ContRetC [C] -> [] - Finish continuation. Marks the continuation in local 0 as finished. Further - attempts to iterate it will fail. + Return from continuation. Marks the continuation in local 0 as finished, sets + the result and transfers control flow to the caller of the continuation body, + which must be a non-static method of the Continuation class. Further attempts + to iterate it will fail. ContNext [] -> [] diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp index de9025ad1..43bdc74cc 100644 --- a/hphp/runtime/vm/bytecode.cpp +++ b/hphp/runtime/vm/bytecode.cpp @@ -6893,11 +6893,17 @@ inline void OPTBLD_INLINE VMExecutionContext::iopContReceive(PC& pc) { tvWriteUninit(fr); } -inline void OPTBLD_INLINE VMExecutionContext::iopContDone(PC& pc) { +inline void OPTBLD_INLINE VMExecutionContext::iopContRetC(PC& pc) { NEXT(); c_Continuation* cont = frame_continuation(m_fp); cont->m_done = true; - cont->m_value.setNull(); + tvSetIgnoreRef(m_stack.topC(), cont->m_value.asTypedValue()); + m_stack.popC(); + + EventHook::FunctionExit(m_fp); + ActRec* prevFp = arGetSfp(m_fp); + pc = prevFp->m_func->getEntry() + m_fp->m_soff; + m_fp = prevFp; } inline void OPTBLD_INLINE VMExecutionContext::iopContNext(PC& pc) { diff --git a/hphp/runtime/vm/hhbc.h b/hphp/runtime/vm/hhbc.h index 330dc37a5..880b06807 100644 --- a/hphp/runtime/vm/hhbc.h +++ b/hphp/runtime/vm/hhbc.h @@ -557,7 +557,7 @@ enum SetOpOp { O(UnpackCont, NA, NOV, ONE(CV), NF) \ O(PackCont, ONE(IVA), ONE(CV), NOV, NF) \ O(ContReceive, NA, NOV, ONE(CV), NF) \ - O(ContDone, NA, NOV, NOV, NF) \ + O(ContRetC, NA, ONE(CV), NOV, CF_TF) \ O(ContNext, NA, NOV, NOV, NF) \ O(ContSend, NA, NOV, NOV, NF) \ O(ContRaise, NA, NOV, NOV, NF) \ diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp index 66e2ac94a..f23f90394 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp @@ -714,8 +714,7 @@ void HhbcTranslator::emitContEnter(int32_t returnBcOffset) { assert(m_stackDeficit == 0); } -void HhbcTranslator::emitContExit() { - m_tb->genExitWhenSurprised(getExitSlowTrace()); +void HhbcTranslator::emitContExitImpl() { SSATmp* retAddr = m_tb->genLdRetAddr(); // Despite the name, this doesn't actually free the AR; it updates the // hardware fp and returns the old one @@ -734,6 +733,11 @@ void HhbcTranslator::emitContExit() { m_hasExit = true; } +void HhbcTranslator::emitContExit() { + m_tb->genExitWhenSurprised(getExitSlowTrace()); + emitContExitImpl(); +} + void HhbcTranslator::emitUnpackCont() { m_tb->genLinkContVarEnv(); SSATmp* cont = m_tb->genLdAssertedLoc(0, Type::Obj); @@ -756,10 +760,14 @@ void HhbcTranslator::emitContReceive() { m_tb->genStProp(cont, valOffset, m_tb->genDefUninit(), true); } -void HhbcTranslator::emitContDone() { +void HhbcTranslator::emitContRetC() { SSATmp* cont = m_tb->genLdAssertedLoc(0, Type::Obj); + m_tb->genExitWhenSurprised(getExitSlowTrace()); m_tb->genStRaw(cont, RawMemSlot::ContDone, m_tb->genDefConst(true)); - m_tb->genSetPropCell(cont, CONTOFF(m_value), m_tb->genDefInitNull()); + m_tb->genSetPropCell(cont, CONTOFF(m_value), popC()); + + // transfer control + emitContExitImpl(); } void HhbcTranslator::emitContNext() { diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.h b/hphp/runtime/vm/translator/hopt/hhbctranslator.h index 6f11eb2c7..2c5ebddec 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.h +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.h @@ -327,11 +327,12 @@ struct HhbcTranslator { // continuations void emitCreateCont(bool getArgs, Id funNameStrId); void emitContEnter(int32_t returnBcOffset); + void emitContExitImpl(); void emitContExit(); void emitUnpackCont(); void emitPackCont(int64_t labelId); void emitContReceive(); - void emitContDone(); + void emitContRetC(); void emitContNext(); void emitContSendImpl(bool raise); void emitContSend(); diff --git a/hphp/runtime/vm/translator/hopt/irtranslator.cpp b/hphp/runtime/vm/translator/hopt/irtranslator.cpp index edf7b06e6..129a1c7e7 100644 --- a/hphp/runtime/vm/translator/hopt/irtranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/irtranslator.cpp @@ -633,9 +633,9 @@ void TranslatorX64::irTranslateContReceive(const Tracelet& t, HHIR_EMIT(ContReceive); } -void TranslatorX64::irTranslateContDone(const Tracelet& t, +void TranslatorX64::irTranslateContRetC(const Tracelet& t, const NormalizedInstruction& i) { - HHIR_EMIT(ContDone); + HHIR_EMIT(ContRetC); } void TranslatorX64::irTranslateContNext(const Tracelet& t, diff --git a/hphp/runtime/vm/translator/translator-x64.cpp b/hphp/runtime/vm/translator/translator-x64.cpp index 2d796021c..d5ef2fd24 100644 --- a/hphp/runtime/vm/translator/translator-x64.cpp +++ b/hphp/runtime/vm/translator/translator-x64.cpp @@ -7565,10 +7565,7 @@ void TranslatorX64::translateContEnter(const Tracelet& t, a. call (rax); } -void TranslatorX64::translateContExit(const Tracelet& t, - const NormalizedInstruction& i) { - syncOutputs(i); - +void TranslatorX64::emitContExit() { emitTestSurpriseFlags(a); { UnlikelyIfBlock ifTracer(CC_NZ, a, astubs); @@ -7581,13 +7578,25 @@ void TranslatorX64::translateContExit(const Tracelet& t, a. ret (); } -void TranslatorX64::translateContDone(const Tracelet& t, +void TranslatorX64::translateContExit(const Tracelet& t, const NormalizedInstruction& i) { - PhysReg contReg = getReg(i.inputs[0]->location); + syncOutputs(i); + emitContExit(); +} + +void TranslatorX64::translateContRetC(const Tracelet& t, + const NormalizedInstruction& i) { + PhysReg valueReg = getReg(i.inputs[0]->location); + PhysReg contReg = getReg(i.inputs[1]->location); a. store_imm8_disp_reg(0x1, CONTOFF(m_done), contReg); - // m_value.setNull() - emitTvSet(i, InvalidReg, KindOfNull, contReg, CONTOFF(m_value), false); + // m_value = $1 + emitTvSet(i, valueReg, i.inputs[0]->outerType(), + contReg, CONTOFF(m_value), false); + + // transfer control + syncOutputs(i.stackOff - 1); + emitContExit(); } static void contPreNextThrowHelper(c_Continuation* c) { @@ -12429,8 +12438,8 @@ bool TranslatorX64::dumpTCData() { SUPPORTED_OP(BareThis) \ SUPPORTED_OP(CheckThis) \ SUPPORTED_OP(PackCont) \ - SUPPORTED_OP(ContDone) \ SUPPORTED_OP(ContReceive) \ + SUPPORTED_OP(ContRetC) \ SUPPORTED_OP(ContNext) \ SUPPORTED_OP(ContSend) \ SUPPORTED_OP(ContRaise) \ diff --git a/hphp/runtime/vm/translator/translator-x64.h b/hphp/runtime/vm/translator/translator-x64.h index c1bbd1345..fb921af92 100644 --- a/hphp/runtime/vm/translator/translator-x64.h +++ b/hphp/runtime/vm/translator/translator-x64.h @@ -275,6 +275,7 @@ private: void emitCallFillCont(Asm& a, const Func* orig, const Func* gen); void emitCallPack(Asm& a, const NormalizedInstruction& i); void emitContRaiseCheck(Asm& a, const NormalizedInstruction& i); + void emitContExit(); void emitContPreNext(const NormalizedInstruction& i, ScratchReg& rCont); void emitContStartedCheck(const NormalizedInstruction& i, ScratchReg& rCont); template @@ -594,7 +595,7 @@ MINSTRS CASE(UnpackCont) \ CASE(PackCont) \ CASE(ContReceive) \ - CASE(ContDone) \ + CASE(ContRetC) \ CASE(ContNext) \ CASE(ContSend) \ CASE(ContRaise) \ diff --git a/hphp/runtime/vm/translator/translator.cpp b/hphp/runtime/vm/translator/translator.cpp index df14aeb31..57f6c49d9 100644 --- a/hphp/runtime/vm/translator/translator.cpp +++ b/hphp/runtime/vm/translator/translator.cpp @@ -1202,7 +1202,7 @@ static const struct { { OpUnpackCont, {Local, Stack1, OutInt64, 1 }}, { OpPackCont, {Local|Stack1, None, OutNone, -1 }}, { OpContReceive, {Local, Stack1, OutUnknown, 1 }}, - { OpContDone, {Local, None, OutNone, 0 }}, + { OpContRetC, {Local|Stack1, None, OutNone, -1 }}, { OpContNext, {None, None, OutNone, 0 }}, { OpContSend, {Local, None, OutNone, 0 }}, { OpContRaise, {Local, None, OutNone, 0 }}, @@ -1959,7 +1959,7 @@ void Translator::getInputs(Tracelet& t, case OpUnpackCont: case OpPackCont: case OpContReceive: - case OpContDone: + case OpContRetC: case OpContSend: case OpContRaise: loc = 0; diff --git a/hphp/runtime/vm/translator/translator.h b/hphp/runtime/vm/translator/translator.h index 55770397b..1a0a1073a 100644 --- a/hphp/runtime/vm/translator/translator.h +++ b/hphp/runtime/vm/translator/translator.h @@ -1042,6 +1042,7 @@ opcodeControlFlowInfo(const Opcode instr) { case OpSwitch: case OpSSwitch: case OpContExit: + case OpContRetC: case OpRetC: case OpRetV: case OpExit: