From 2a1d59c286fb1522c9b4ac7bebb55673a47742eb Mon Sep 17 00:00:00 2001 From: Jan Oravec Date: Tue, 25 Jun 2013 16:52:41 -0700 Subject: [PATCH] Move Continuation state check to ContCheck opcode Add ContCheck opcode that moves away responsibility of checking Continuation status from Cont{Next,Send,Raise} opcodes. This check needs to run outside of try/catch in next()/send()/raise() methods and moving it to a separate opcode lets us merge Cont{Next,Send,Raise} with ContEnter. --- hphp/compiler/analysis/emitter.cpp | 6 ++++- hphp/doc/bytecode.specification | 22 +++++++++++------- hphp/runtime/base/execution_context.h | 2 -- hphp/runtime/vm/bytecode.cpp | 31 +++++++++++++------------ hphp/runtime/vm/hhbc.h | 1 + hphp/runtime/vm/jit/hhbctranslator.cpp | 12 +++++++--- hphp/runtime/vm/jit/hhbctranslator.h | 1 + hphp/runtime/vm/jit/irtranslator.cpp | 4 ++++ hphp/runtime/vm/jit/translator-instrs.h | 1 + hphp/runtime/vm/jit/translator.cpp | 1 + 10 files changed, 51 insertions(+), 30 deletions(-) diff --git a/hphp/compiler/analysis/emitter.cpp b/hphp/compiler/analysis/emitter.cpp index f78f09bfe..8ab3c04bc 100644 --- a/hphp/compiler/analysis/emitter.cpp +++ b/hphp/compiler/analysis/emitter.cpp @@ -7010,13 +7010,17 @@ static void emitContinuationMethod(UnitEmitter& ue, FuncEmitter* fe, // translations fe->setAttrs(Attr(fe->attrs() | AttrClone)); + // check continuation status; send()/raise() also checks started + ue.emitOp(OpContCheck); + ue.emitIVA(m == METH_SEND || m == METH_RAISE); + + const Offset ehStart = ue.bcPos(); static Op mOps[] = { OpContNext, OpContSend, OpContRaise, }; ue.emitOp(mOps[m]); - const Offset ehStart = ue.bcPos(); ue.emitOp(OpContEnter); ue.emitOp(OpContStopped); ue.emitOp(OpNull); diff --git a/hphp/doc/bytecode.specification b/hphp/doc/bytecode.specification index 608a7f062..941c6f854 100644 --- a/hphp/doc/bytecode.specification +++ b/hphp/doc/bytecode.specification @@ -3720,24 +3720,28 @@ ContRetC [C] -> [] which must be a non-static method of the Continuation class. Further attempts to iterate it will fail. +ContCheck [] -> [] + + Check whether continuation can be iterated. $this must be a Continuation + object. If the continuation is finished, already running, or not yet started + and is enabled, an exception will be thrown. + ContNext [] -> [] - Prepare continuation for iteration. $this must be a Continuation - object. If the continuation is finished or already running, an exception - will be thrown. + Prepare continuation for iteration. $this must be a Continuation object + that passed ContCheck check. ContSend [] -> [] - Prepare continuation to receive a value. $this must be a Continuation - object. If the continuation is finished or already running, an exception will - be thrown. Otherwise, the value in local 0 is stored in the continuation. + Prepare continuation to receive a value. $this must be a Continuation object + that passed ContCheck check. The value in local 0 is stored in the + continuation. ContRaise [] -> [] Prepare continuation to receive a thrown exception. $this must be a - Continuation object. If the continuation is finished or already running, an - exception will be thrown. Otherwise, the exception in local 0 is stored in - the continuation. + Continuation object that passed ContCheck check. The exception in + local 0 is stored in the continuation. ContValid [] -> [C:Bool] diff --git a/hphp/runtime/base/execution_context.h b/hphp/runtime/base/execution_context.h index 156458cb9..cca704d01 100644 --- a/hphp/runtime/base/execution_context.h +++ b/hphp/runtime/base/execution_context.h @@ -492,8 +492,6 @@ private: OPCODES #undef O - template - void contSendImpl(); void classExistsImpl(PC& pc, Attr typeAttr); void fPushObjMethodImpl( Class* cls, StringData* name, ObjectData* obj, int numArgs); diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp index b419a2f25..046bacabd 100644 --- a/hphp/runtime/vm/bytecode.cpp +++ b/hphp/runtime/vm/bytecode.cpp @@ -6853,33 +6853,34 @@ inline void OPTBLD_INLINE VMExecutionContext::iopContRetC(PC& pc) { m_fp = prevFp; } +inline void OPTBLD_INLINE VMExecutionContext::iopContCheck(PC& pc) { + NEXT(); + DECODE_IVA(check_started); + c_Continuation* cont = this_continuation(m_fp); + if (check_started) { + cont->startedCheck(); + } + cont->preNext(); +} + inline void OPTBLD_INLINE VMExecutionContext::iopContNext(PC& pc) { NEXT(); c_Continuation* cont = this_continuation(m_fp); - cont->preNext(); cont->m_received.setNull(); } -template -inline void VMExecutionContext::contSendImpl() { - c_Continuation* cont = this_continuation(m_fp); - cont->startedCheck(); - cont->preNext(); - cont->m_received.assignVal(tvAsVariant(frame_local(m_fp, 0))); - if (raise) { - assert(cont->m_label); - --cont->m_label; - } -} - inline void OPTBLD_INLINE VMExecutionContext::iopContSend(PC& pc) { NEXT(); - contSendImpl(); + c_Continuation* cont = this_continuation(m_fp); + cont->m_received.assignVal(tvAsVariant(frame_local(m_fp, 0))); } inline void OPTBLD_INLINE VMExecutionContext::iopContRaise(PC& pc) { NEXT(); - contSendImpl(); + c_Continuation* cont = this_continuation(m_fp); + cont->m_received.assignVal(tvAsVariant(frame_local(m_fp, 0))); + assert(cont->m_label); + --cont->m_label; } inline void OPTBLD_INLINE VMExecutionContext::iopContValid(PC& pc) { diff --git a/hphp/runtime/vm/hhbc.h b/hphp/runtime/vm/hhbc.h index 1f5885735..ce9fe18ed 100644 --- a/hphp/runtime/vm/hhbc.h +++ b/hphp/runtime/vm/hhbc.h @@ -552,6 +552,7 @@ enum SetOpOp { O(UnpackCont, NA, NOV, TWO(CV,CV), NF) \ O(PackCont, ONE(IVA), ONE(CV), NOV, NF) \ O(ContRetC, NA, ONE(CV), NOV, CF_TF) \ + O(ContCheck, ONE(IVA), NOV, NOV, NF) \ O(ContNext, NA, NOV, NOV, NF) \ O(ContSend, NA, NOV, NOV, NF) \ O(ContRaise, NA, NOV, NOV, NF) \ diff --git a/hphp/runtime/vm/jit/hhbctranslator.cpp b/hphp/runtime/vm/jit/hhbctranslator.cpp index 19246ad49..ababb3865 100644 --- a/hphp/runtime/vm/jit/hhbctranslator.cpp +++ b/hphp/runtime/vm/jit/hhbctranslator.cpp @@ -1155,10 +1155,18 @@ void HhbcTranslator::emitContRetC() { emitContExitImpl(); } +void HhbcTranslator::emitContCheck(bool checkStarted) { + assert(curClass()); + SSATmp* cont = gen(LdThis, m_tb->fp()); + if (checkStarted) { + gen(ContStartedCheck, getExitSlowTrace(), cont); + } + gen(ContPreNext, getExitSlowTrace(), cont); +} + void HhbcTranslator::emitContNext() { assert(curClass()); SSATmp* cont = gen(LdThis, m_tb->fp()); - gen(ContPreNext, getExitSlowTrace(), cont); if (RuntimeOption::EvalHHIRGenerateAsserts) { // We're guaranteed to have a Null in m_received at this point auto const oldVal = gen(LdProp, Type::Cell, cont, cns(CONTOFF(m_received))); @@ -1169,8 +1177,6 @@ void HhbcTranslator::emitContNext() { void HhbcTranslator::emitContSendImpl(bool raise) { assert(curClass()); SSATmp* cont = gen(LdThis, m_tb->fp()); - gen(ContStartedCheck, getExitSlowTrace(), cont); - gen(ContPreNext, getExitSlowTrace(), cont); gen(AssertLoc, Type::Cell, LocalId(0), m_tb->fp()); auto const newVal = gen(IncRef, ldLoc(0)); if (RuntimeOption::EvalHHIRGenerateAsserts) { diff --git a/hphp/runtime/vm/jit/hhbctranslator.h b/hphp/runtime/vm/jit/hhbctranslator.h index bf28ba39f..f6f2804dd 100644 --- a/hphp/runtime/vm/jit/hhbctranslator.h +++ b/hphp/runtime/vm/jit/hhbctranslator.h @@ -380,6 +380,7 @@ struct HhbcTranslator { void emitUnpackCont(); void emitPackCont(int64_t labelId); void emitContRetC(); + void emitContCheck(bool checkStarted); void emitContNext(); void emitContSendImpl(bool raise); void emitContSend(); diff --git a/hphp/runtime/vm/jit/irtranslator.cpp b/hphp/runtime/vm/jit/irtranslator.cpp index 299c53729..cf31f2682 100644 --- a/hphp/runtime/vm/jit/irtranslator.cpp +++ b/hphp/runtime/vm/jit/irtranslator.cpp @@ -564,6 +564,10 @@ void Translator::translateContRetC(const NormalizedInstruction& i) { HHIR_EMIT(ContRetC); } +void Translator::translateContCheck(const NormalizedInstruction& i) { + HHIR_EMIT(ContCheck, i.imm[0].u_IVA); +} + void Translator::translateContNext(const NormalizedInstruction& i) { HHIR_EMIT(ContNext); } diff --git a/hphp/runtime/vm/jit/translator-instrs.h b/hphp/runtime/vm/jit/translator-instrs.h index d4b972796..87edde3f6 100644 --- a/hphp/runtime/vm/jit/translator-instrs.h +++ b/hphp/runtime/vm/jit/translator-instrs.h @@ -133,6 +133,7 @@ CASE(UnpackCont) \ CASE(PackCont) \ CASE(ContRetC) \ + CASE(ContCheck) \ CASE(ContNext) \ CASE(ContSend) \ CASE(ContRaise) \ diff --git a/hphp/runtime/vm/jit/translator.cpp b/hphp/runtime/vm/jit/translator.cpp index f493d4350..f168cde85 100644 --- a/hphp/runtime/vm/jit/translator.cpp +++ b/hphp/runtime/vm/jit/translator.cpp @@ -1269,6 +1269,7 @@ static const struct { { OpUnpackCont, {Local, StackTop2, OutInt64, 2 }}, { OpPackCont, {Local|Stack1, None, OutNone, -1 }}, { OpContRetC, {Local|Stack1, None, OutNone, -1 }}, + { OpContCheck, {None, None, OutNone, 0 }}, { OpContNext, {None, None, OutNone, 0 }}, { OpContSend, {Local, None, OutNone, 0 }}, { OpContRaise, {Local, None, OutNone, 0 }},