From 0d5d5bca72eacc4a8d6ccdc64fb7bdac1b0c4cb4 Mon Sep 17 00:00:00 2001 From: Paul Bissonnette Date: Tue, 11 Jun 2013 11:09:56 -0700 Subject: [PATCH] Added IterBreakV, MIter{Init,InitK,Next,NextK,Free} and fixed memory tracking bug. Added IR opcodes to perform MIter* instructions in JIT. Added IterBreakV bytecode operation to break out of multiple loops containing iterators. Emitter and assembler were modified to support such use. --- hphp/compiler/analysis/emitter.cpp | 72 ++++++---- hphp/compiler/analysis/emitter.h | 14 +- hphp/compiler/analysis/peephole.cpp | 5 + hphp/doc/bytecode.specification | 41 +++++- hphp/doc/ir.specification | 10 ++ hphp/runtime/base/array/array_iterator.cpp | 142 +++++++++++-------- hphp/runtime/base/array/array_iterator.h | 10 +- hphp/runtime/base/execution_context.h | 2 +- hphp/runtime/base/memory/memory_manager.cpp | 9 +- hphp/runtime/vm/as.cpp | 48 +++++++ hphp/runtime/vm/bytecode.cpp | 73 ++++++++-- hphp/runtime/vm/hhbc.cpp | 37 ++++- hphp/runtime/vm/hhbc.h | 34 +++-- hphp/runtime/vm/jit/codegen.cpp | 80 +++++++++++ hphp/runtime/vm/jit/codegen.h | 2 + hphp/runtime/vm/jit/extradata.h | 1 + hphp/runtime/vm/jit/hhbctranslator.cpp | 111 ++++++++++++++- hphp/runtime/vm/jit/hhbctranslator.h | 15 ++ hphp/runtime/vm/jit/ir.h | 14 ++ hphp/runtime/vm/jit/irtranslator.cpp | 50 +++++++ hphp/runtime/vm/jit/translator-instrs.h | 6 + hphp/runtime/vm/jit/translator.cpp | 1 + hphp/runtime/vm/jit/translator.h | 1 + hphp/runtime/vm/verifier/check_func.cpp | 9 +- hphp/test/quick/asm_iterbreak.hhas | 68 +++++++++ hphp/test/quick/asm_iterbreak.hhas.expect | 4 + hphp/test/quick/bad_iter_warning.php | 24 ++++ hphp/test/quick/bad_iter_warning.php.expectf | 8 ++ 28 files changed, 759 insertions(+), 132 deletions(-) create mode 100644 hphp/test/quick/asm_iterbreak.hhas create mode 100644 hphp/test/quick/asm_iterbreak.hhas.expect create mode 100644 hphp/test/quick/bad_iter_warning.php create mode 100644 hphp/test/quick/bad_iter_warning.php.expectf diff --git a/hphp/compiler/analysis/emitter.cpp b/hphp/compiler/analysis/emitter.cpp index 462914357..6f734f6ca 100644 --- a/hphp/compiler/analysis/emitter.cpp +++ b/hphp/compiler/analysis/emitter.cpp @@ -306,6 +306,7 @@ static int32_t countStackValues(const std::vector& immVec) { #define DEC_MA std::vector #define DEC_BLA std::vector& #define DEC_SLA std::vector& +#define DEC_ILA std::vector& #define DEC_IVA int32_t #define DEC_HA int32_t #define DEC_IA int32_t @@ -375,6 +376,7 @@ static int32_t countStackValues(const std::vector& immVec) { #define POP_HA_MA(i) #define POP_HA_BLA(i) #define POP_HA_SLA(i) +#define POP_HA_ILA(i) #define POP_HA_IVA(i) #define POP_HA_IA(i) #define POP_HA_I64A(i) @@ -458,6 +460,19 @@ static int32_t countStackValues(const std::vector& immVec) { #define IMPL3_BLA IMPL_BLA(a3) #define IMPL4_BLA IMPL_BLA(a4) +#define IMPL_ILA(var) do { \ + auto& ue = getUnitEmitter(); \ + ue.emitInt32(var.size()); \ + for (auto& i : var) { \ + ue.emitInt32(i.kind); \ + ue.emitInt32(i.id); \ + } \ +} while(0) +#define IMPL1_ILA IMPL_ILA(a1) +#define IMPL2_ILA IMPL_ILA(a2) +#define IMPL3_ILA IMPL_ILA(a3) +#define IMPL4_ILA IMPL_ILA(a4) + #define IMPL_SLA(var) do { \ auto& ue = getUnitEmitter(); \ ue.emitInt32(var.size()); \ @@ -618,6 +633,11 @@ static int32_t countStackValues(const std::vector& immVec) { #undef IMPL2_SLA #undef IMPL3_SLA #undef IMPL4_SLA +#undef IMPL_ILA +#undef IMPL1_ILA +#undef IMPL2_ILA +#undef IMPL3_ILA +#undef IMPL4_ILA #undef IMPL_IVA #undef IMPL1_IVA #undef IMPL2_IVA @@ -2024,31 +2044,16 @@ bool EmitterVisitor::visitImpl(ConstructPtr node) { return false; } - // "continue N" breaks out of N-1 loops and jumps to the top of the Nth. - // So whether this is break or continue, free N-1 Iters. - for (uint64_t i = 0; i < destLevel; ++i) { - if (m_controlTargets[i].m_itId != -1) { - if (m_controlTargets[i].m_itRef) { - e.MIterFree(m_controlTargets[i].m_itId); - } else { - e.IterFree(m_controlTargets[i].m_itId); - } - } + if (bs->is(Statement::KindOfBreakStatement)) { + // break N levels for a break + emitIterBreak(e, destLevel+1, + m_controlTargets[destLevel].m_brkTarg); + } else { + // break N-1 levels for a continue + emitIterBreak(e, destLevel, + m_controlTargets[destLevel].m_cntTarg); } - // Only free the Nth-level Iter for break statements. - if (bs->is(Statement::KindOfBreakStatement)) { - if (m_controlTargets[destLevel].m_itId != -1) { - if (m_controlTargets[destLevel].m_itRef) { - e.MIterFree(m_controlTargets[destLevel].m_itId); - } else { - e.IterFree(m_controlTargets[destLevel].m_itId); - } - } - e.Jmp(m_controlTargets[destLevel].m_brkTarg); - } else { - e.Jmp(m_controlTargets[destLevel].m_cntTarg); - } return false; } @@ -4280,6 +4285,25 @@ void EmitterVisitor::emitCGet(Emitter& e) { } } +void EmitterVisitor::emitIterBreak(Emitter& e, uint64_t n, Label& targ) { + std::vector immItrList; + + for (uint64_t level = 0; level < n; ++level) { + if (m_controlTargets[level].m_itId != -1) { + immItrList.push_back(Emitter::IterPair(m_controlTargets[level].m_itRef + ? KindOfMIter : KindOfIter, + m_controlTargets[level] + .m_itId)); + } + } + + if (immItrList.size()) { + e.IterBreak(immItrList, targ); + } else { + e.Jmp(targ); + } +} + void EmitterVisitor::emitVGet(Emitter& e) { if (checkIfStackEmpty("VGet*")) return; LocationGuard loc(e, m_tempLoc); @@ -6460,7 +6484,7 @@ class ForeachIterGuard { public: ForeachIterGuard(EmitterVisitor& ev, Id iterId, - EmitterVisitor::IterKind kind) + IterKind kind) : m_ev(ev) { m_ev.pushIterScope(iterId, kind); diff --git a/hphp/compiler/analysis/emitter.h b/hphp/compiler/analysis/emitter.h index f60870740..f4fbe5ac8 100644 --- a/hphp/compiler/analysis/emitter.h +++ b/hphp/compiler/analysis/emitter.h @@ -89,6 +89,12 @@ public: Id str; Label* dest; }; + + struct IterPair { + IterPair(IterKind k, Id i) : kind(k), id(i) {} + IterKind kind; + Id id; + }; #define O(name, imm, pop, push, flags) \ void name(imm); #define NA @@ -103,6 +109,7 @@ public: #define MA std::vector #define BLA std::vector& #define SLA std::vector& +#define ILA std::vector& #define IVA int32_t #define HA int32_t #define IA int32_t @@ -122,6 +129,7 @@ public: #undef MA #undef BLA #undef SLA +#undef ILA #undef IVA #undef HA #undef IA @@ -374,11 +382,6 @@ public: EXCEPTION_COMMON_IMPL(IncludeTimeFatalException); }; - enum IterKind { - KindOfIter = 0, - KindOfMIter = 1 - }; - void pushIterScope(Id id, IterKind kind) { m_pendingIters.emplace_back(id, kind); } @@ -605,6 +608,7 @@ public: void emitStringSwitch(Emitter& e, SwitchStatementPtr s, std::vector