From ed32d4ce3b082b33eaaeb56c82395dac98afca2d Mon Sep 17 00:00:00 2001 From: Bin Liu Date: Wed, 25 Mar 2015 08:30:30 -0700 Subject: [PATCH] Elements of a static array have type UncountedInit. Summary: Avoid `unbox()` and `incref()` on elements in static arrays. The way we determine the `Type` for instruction destinations is changed slightly. In addition to the opcode, we also check if the array is static. Potentially, similar things can be done for other instructions. Reviewed By: @jdelong Differential Revision: D1901198 --- hphp/doc/ir.specification | 2 +- hphp/runtime/vm/jit/code-gen-arm.cpp | 26 ++++++++++++---------- hphp/runtime/vm/jit/code-gen-minstr-x64.cpp | 8 ++++--- hphp/runtime/vm/jit/code-gen-x64.cpp | 29 ++++++++++++++++-------- hphp/runtime/vm/jit/guard-relaxation.cpp | 3 ++- hphp/runtime/vm/jit/ir-instruction.cpp | 34 ++++++++++++++++++----------- hphp/runtime/vm/jit/minstr-helpers.h | 29 ++++++++++++++---------- hphp/runtime/vm/jit/vasm-llvm.cpp | 8 ++++--- hphp/runtime/vm/jit/vasm-x64.cpp | 12 ++++++++-- 9 files changed, 97 insertions(+), 54 deletions(-) diff --git a/hphp/doc/ir.specification b/hphp/doc/ir.specification index 7ab6a004b48..b3b7aee6dd8 100644 --- a/hphp/doc/ir.specification +++ b/hphp/doc/ir.specification @@ -2167,7 +2167,7 @@ fields of that struct for holding intermediate values. Like ElemX, but used for intermediate element lookups that may modify the base as part of an unset operation. -| ArrayGet, D(Cell), S(Arr) S(Int,Str), PRc|Er +| ArrayGet, DArrElem, S(Arr) S(Int,Str), PRc|Er Get element with key S1 from base S0. diff --git a/hphp/runtime/vm/jit/code-gen-arm.cpp b/hphp/runtime/vm/jit/code-gen-arm.cpp index 6679f40d8bd..878b4b6b34d 100644 --- a/hphp/runtime/vm/jit/code-gen-arm.cpp +++ b/hphp/runtime/vm/jit/code-gen-arm.cpp @@ -1094,11 +1094,13 @@ void CodeGenerator::cgCallBuiltin(IRInstruction* inst) { // this should use some kind of cmov assert(isBuiltinByRef(funcReturnType) && isSmartPtrRef(funcReturnType)); v << load{mis[returnOffset + TVOFF(m_data)], dst}; - condZero(v, dst, dstType, [&](Vout& v) { - return v.cns(KindOfNull); - }, [&](Vout& v) { - return v.cns(returnType.toDataType()); - }); + if (dstType.isValid()) { + condZero(v, dst, dstType, [&](Vout& v) { + return v.cns(KindOfNull); + }, [&](Vout& v) { + return v.cns(returnType.toDataType()); + }); + } return; } @@ -1108,12 +1110,14 @@ void CodeGenerator::cgCallBuiltin(IRInstruction* inst) { assert(isBuiltinByRef(funcReturnType) && !isSmartPtrRef(funcReturnType)); auto tmp_dst_type = v.makeReg(); v << load{mis[returnOffset + TVOFF(m_data)], dst}; - v << loadzbl{mis[returnOffset + TVOFF(m_type)], tmp_dst_type}; - condZero(v, tmp_dst_type, dstType, [&](Vout& v) { - return v.cns(KindOfNull); - }, [&](Vout& v) { - return tmp_dst_type; - }); + if (dstType.isValid()) { + v << loadzbl{mis[returnOffset + TVOFF(m_type)], tmp_dst_type}; + condZero(v, tmp_dst_type, dstType, [&](Vout& v) { + return v.cns(KindOfNull); + }, [&](Vout& v) { + return tmp_dst_type; + }); + } return; } diff --git a/hphp/runtime/vm/jit/code-gen-minstr-x64.cpp b/hphp/runtime/vm/jit/code-gen-minstr-x64.cpp index 122a562279f..e78acb7246b 100644 --- a/hphp/runtime/vm/jit/code-gen-minstr-x64.cpp +++ b/hphp/runtime/vm/jit/code-gen-minstr-x64.cpp @@ -311,11 +311,13 @@ void CodeGenerator::cgElemArray(IRInstruction* i) { cgElemArrayImpl(i); } void CodeGenerator::cgElemArrayW(IRInstruction* i) { cgElemArrayImpl(i); } void CodeGenerator::cgArrayGet(IRInstruction* inst) { - auto const key = inst->src(1); - auto const keyInfo = checkStrictlyInteger(key); + auto const arrIsStatic = inst->src(0)->isA(Type::StaticArr); + auto const key = inst->src(1); + auto const keyInfo = checkStrictlyInteger(key); BUILD_OPTAB(ARRAYGET_HELPER_TABLE, keyInfo.type, - keyInfo.checkForInt); + keyInfo.checkForInt, + arrIsStatic); auto args = argGroup(inst).ssa(0); if (keyInfo.converted) { diff --git a/hphp/runtime/vm/jit/code-gen-x64.cpp b/hphp/runtime/vm/jit/code-gen-x64.cpp index b849e1d9fdd..5c4d9334777 100644 --- a/hphp/runtime/vm/jit/code-gen-x64.cpp +++ b/hphp/runtime/vm/jit/code-gen-x64.cpp @@ -666,8 +666,12 @@ CallDest CodeGenerator::callDestTV(const IRInstruction* inst) const { assert(loc.numAllocated() == 1); return { DestType::SIMD, loc.reg(0) }; } - assert(loc.numAllocated() == 2); - return { DestType::TV, loc.reg(0), loc.reg(1) }; + if (loc.numAllocated() == 2) { + return { DestType::TV, loc.reg(0), loc.reg(1) }; + } + assert(loc.numAllocated() == 1); + // Sometimes we statically know the type and only need the value. + return { DestType::TV, loc.reg(0), InvalidReg }; } CallDest CodeGenerator::callDestDbl(const IRInstruction* inst) const { @@ -3181,9 +3185,11 @@ void CodeGenerator::cgCallBuiltin(IRInstruction* inst) { auto rtype = v.cns(returnType.toDataType()); auto nulltype = v.cns(KindOfNull); v << load{rdsReg[returnOffset], dstReg}; - auto const sf = v.makeReg(); - v << testq{dstReg, dstReg, sf}; - v << cmovq{CC_Z, sf, rtype, nulltype, dstType}; + if (dstType.isValid()) { + auto const sf = v.makeReg(); + v << testq{dstReg, dstReg, sf}; + v << cmovq{CC_Z, sf, rtype, nulltype, dstType}; + } return; } if (returnType <= Type::Cell || returnType <= Type::BoxedCell) { @@ -3194,9 +3200,11 @@ void CodeGenerator::cgCallBuiltin(IRInstruction* inst) { emitLoadTVType(v, rdsReg[returnOffset + TVOFF(m_type)], tmp_type); v << load{rdsReg[returnOffset + TVOFF(m_data)], dstReg}; static_assert(KindOfUninit == 0, "KindOfUninit must be 0 for test"); - auto const sf = v.makeReg(); - v << testb{tmp_type, tmp_type, sf}; - v << cmovq{CC_Z, sf, tmp_type, nulltype, dstType}; + if (dstType.isValid()) { + auto const sf = v.makeReg(); + v << testb{tmp_type, tmp_type, sf}; + v << cmovq{CC_Z, sf, tmp_type, nulltype, dstType}; + } return; } not_reached(); @@ -3380,6 +3388,7 @@ void CodeGenerator::emitStoreTypedValue(Vptr dst, SSATmp* src, Vloc loc) { if (src->type().needsValueReg()) { v << store{srcReg0, refTVData(dst)}; } + assert(srcReg1.isValid()); // a precondition for this call emitStoreTVType(v, srcReg1, refTVType(dst)); } @@ -3433,7 +3442,9 @@ void CodeGenerator::emitLoadTypedValue(SSATmp* dst, Vloc dstLoc, Vptr ref) { return; } auto const typeDstReg = dstLoc.reg(1); - emitLoadTVType(v, refTVType(ref), typeDstReg); + if (typeDstReg.isValid()) { + emitLoadTVType(v, refTVType(ref), typeDstReg); + } v << load{refTVData(ref), valueDstReg}; } diff --git a/hphp/runtime/vm/jit/guard-relaxation.cpp b/hphp/runtime/vm/jit/guard-relaxation.cpp index 2befa0bba39..b165876db3c 100644 --- a/hphp/runtime/vm/jit/guard-relaxation.cpp +++ b/hphp/runtime/vm/jit/guard-relaxation.cpp @@ -51,7 +51,8 @@ bool shouldHHIRRelaxGuards() { #define DBoxPtr return false; #define DAllocObj return false; // fixed type from ExtraData #define DArrPacked return false; // fixed type -#define DArrElem assert(inst->is(LdPackedArrayElem)); \ +#define DArrElem assert(inst->is(LdPackedArrayElem, LdStructArrayElem, \ + ArrayGet)); \ return typeMightRelax(inst->src(0)); #define DThis return false; // fixed type from ctx class #define DCtx return false; diff --git a/hphp/runtime/vm/jit/ir-instruction.cpp b/hphp/runtime/vm/jit/ir-instruction.cpp index 2a8754df6cc..efbf07c3eda 100644 --- a/hphp/runtime/vm/jit/ir-instruction.cpp +++ b/hphp/runtime/vm/jit/ir-instruction.cpp @@ -247,30 +247,39 @@ Type allocObjReturn(const IRInstruction* inst) { } Type arrElemReturn(const IRInstruction* inst) { - assert(inst->op() == LdPackedArrayElem || inst->op() == LdStructArrayElem); + assert(inst->is(LdPackedArrayElem, LdStructArrayElem, ArrayGet)); assert(!inst->hasTypeParam() || inst->typeParam() <= Type::Gen); - auto const tyParam = inst->hasTypeParam() ? inst->typeParam() : Type::Gen; + + auto resultType = inst->hasTypeParam() ? inst->typeParam() : Type::Gen; + if (inst->is(ArrayGet)) { + resultType = resultType & Type::InitCell; + } + + // Elements of a static array are uncounted + if (inst->src(0)->isA(Type::StaticArr)) { + resultType = resultType & Type::UncountedInit; + } auto const arrTy = inst->src(0)->type().arrSpec().type(); - if (!arrTy) return tyParam; + if (!arrTy) return resultType; using T = RepoAuthType::Array::Tag; switch (arrTy->tag()) { case T::Packed: - { - auto const idx = inst->src(1); - if (!idx->hasConstVal()) return Type::Gen; - if (idx->intVal() >= 0 && idx->intVal() < arrTy->size()) { - return typeFromRAT(arrTy->packedElem(idx->intVal())) & tyParam; - } + { + auto const idx = inst->src(1); + if (idx->hasConstVal() && + idx->intVal() >= 0 && idx->intVal() < arrTy->size()) { + return typeFromRAT(arrTy->packedElem(idx->intVal())) & resultType; } - return Type::Gen; + return resultType; + } case T::PackedN: - return typeFromRAT(arrTy->elemType()) & tyParam; + return typeFromRAT(arrTy->elemType()) & resultType; } - return tyParam; + not_reached(); } Type thisReturn(const IRInstruction* inst) { @@ -373,7 +382,6 @@ Type outputType(const IRInstruction* inst, int dstId) { #undef DBuiltin #undef DSubtract #undef DCns - } }} diff --git a/hphp/runtime/vm/jit/minstr-helpers.h b/hphp/runtime/vm/jit/minstr-helpers.h index 1535b7e7e93..9f1a39cf717 100644 --- a/hphp/runtime/vm/jit/minstr-helpers.h +++ b/hphp/runtime/vm/jit/minstr-helpers.h @@ -457,26 +457,33 @@ ELEM_ARRAY_HELPER_TABLE(X) TypedValue arrayGetNotFound(int64_t k); TypedValue arrayGetNotFound(const StringData* k); -template +template TypedValue arrayGetImpl(ArrayData* a, key_type key) { auto ret = checkForInt ? checkedGet(a, key) : a->nvGet(key); if (ret) { - ret = tvToCell(ret); - tvRefcountedIncRef(ret); + if (arrIsStatic) { + tvAssertCell(ret); + } else { + ret = tvToCell(ret); + tvRefcountedIncRef(ret); + } return *ret; } return arrayGetNotFound(key); } -#define ARRAYGET_HELPER_TABLE(m) \ - /* name keyType checkForInt */ \ - m(arrayGetS, KeyType::Str, false) \ - m(arrayGetSi, KeyType::Str, true) \ - m(arrayGetI, KeyType::Int, false) +#define ARRAYGET_HELPER_TABLE(m) \ + /* name keyType checkForInt arrIsStatic */ \ + m(arrayGetSC, KeyType::Str, false, false) \ + m(arrayGetSU, KeyType::Str, false, true) \ + m(arrayGetSiC, KeyType::Str, true, false) \ + m(arrayGetSiU, KeyType::Str, true, true) \ + m(arrayGetIC, KeyType::Int, false, false) \ + m(arrayGetIU, KeyType::Int, false, true) -#define X(nm, keyType, checkForInt) \ -inline TypedValue nm(ArrayData* a, key_type key) {\ - return arrayGetImpl(a, key);\ +#define X(nm, keyType, checkForInt, isStatic) \ +inline TypedValue nm(ArrayData* a, key_type key) { \ + return arrayGetImpl(a, key); \ } ARRAYGET_HELPER_TABLE(X) #undef X diff --git a/hphp/runtime/vm/jit/vasm-llvm.cpp b/hphp/runtime/vm/jit/vasm-llvm.cpp index 7d09d039257..d31c39fff9c 100644 --- a/hphp/runtime/vm/jit/vasm-llvm.cpp +++ b/hphp/runtime/vm/jit/vasm-llvm.cpp @@ -1872,12 +1872,14 @@ void LLVMEmitter::emitCall(const Vinstr& inst) { defineValue(dests[0], callInst); break; case DestType::TV: { - assert(dests.size() == 2); static_assert(offsetof(TypedValue, m_data) == 0, ""); static_assert(offsetof(TypedValue, m_type) == 8, ""); + assert(dests.size() <= 2 && dests.size() >= 1); defineValue(dests[0], m_irb.CreateExtractValue(callInst, 0)); // m_data - auto type = m_irb.CreateExtractValue(callInst, 1); - defineValue(dests[1], zext(type, 64)); // m_type + if (dests.size() == 2) { + auto type = m_irb.CreateExtractValue(callInst, 1); + defineValue(dests[1], zext(type, 64)); // m_type + } break; } case DestType::SIMD: { diff --git a/hphp/runtime/vm/jit/vasm-x64.cpp b/hphp/runtime/vm/jit/vasm-x64.cpp index fa8419b2eb7..a1622e64fa0 100644 --- a/hphp/runtime/vm/jit/vasm-x64.cpp +++ b/hphp/runtime/vm/jit/vasm-x64.cpp @@ -973,8 +973,16 @@ void lowerVcall(Vunit& unit, Vlabel b, size_t iInst) { // the lower bits, so shift the type result register. static_assert(offsetof(TypedValue, m_data) == 0, ""); static_assert(offsetof(TypedValue, m_type) == 8, ""); - assert(dests.size() == 2); - v << copy2{reg::rax, reg::rdx, dests[0], dests[1]}; + if (dests.size() == 2) { + v << copy2{reg::rax, reg::rdx, dests[0], dests[1]}; + } else { + // We have cases where we statically know the type but need the value + // from native call. Even if the type does not really need a register + // (e.g., InitNull), a Vreg is still allocated in assignRegs(), so the + // following assertion holds. + assert(dests.size() == 1); + v << copy{reg::rax, dests[0]}; + } break; } case DestType::SIMD: { -- 2.11.4.GIT