From f56d1fed4edc80909544f75708ce0ff5c83a839b Mon Sep 17 00:00:00 2001 From: Jordan DeLong Date: Sat, 22 Jun 2013 00:14:09 -0700 Subject: [PATCH] Try not to look up scalar class constants in targetcache Instead, just check the class is defined and is the class we expected at translation time (side exiting if not), and just use the scalar value directly. If the class is Persistent or a parent of the current context we don't need to check the Class* either. --- hphp/doc/ir.specification | 6 ++ hphp/runtime/vm/class.cpp | 6 +- hphp/runtime/vm/class.h | 23 +++++- hphp/runtime/vm/jit/codegen.cpp | 8 ++ hphp/runtime/vm/jit/extradata.h | 18 +++++ hphp/runtime/vm/jit/hhbctranslator.cpp | 108 +++++++++++++++++++------ hphp/runtime/vm/jit/hhbctranslator.h | 8 ++ hphp/runtime/vm/jit/ir.h | 1 + hphp/runtime/vm/jit/translator-x64.cpp | 22 ----- hphp/runtime/vm/jit/translator-x64.h | 4 - 10 files changed, 150 insertions(+), 54 deletions(-) diff --git a/hphp/doc/ir.specification b/hphp/doc/ir.specification index 5ff8da6f4..c10f357ce 100644 --- a/hphp/doc/ir.specification +++ b/hphp/doc/ir.specification @@ -405,6 +405,12 @@ CheckInitMem S0:PtrToGen S1:ConstInt -> L If the value at S0 + S1 (in bytes) has type Uninit, branch to L. +CheckDefinedClsEq -> L + + Compare the currently defined class of name `className' with + `classPtr'; if they aren't equal or if `className' is not defined, + branch to L. + GuardRefs S0:FuncPtr S1:Int S2:Int S3:Int S4:Int S5:Int Perform reffiness guard checks. Operands: diff --git a/hphp/runtime/vm/class.cpp b/hphp/runtime/vm/class.cpp index 2505fe5af..d070145f1 100644 --- a/hphp/runtime/vm/class.cpp +++ b/hphp/runtime/vm/class.cpp @@ -1190,13 +1190,11 @@ TypedValue* Class::clsCnsGet(const StringData* clsCnsName) const { return clsCns; } -/* - * Class::clsCnsType: provide the current runtime type of this class - * constant. This has predictive value for the translator. - */ DataType Class::clsCnsType(const StringData* cnsName) const { Slot slot; TypedValue* cns = cnsNameToTV(cnsName, slot); + // TODO: lookup the constant in target cache in case it's dynamic + // and already initialized. if (!cns) return KindOfUninit; return cns->m_type; } diff --git a/hphp/runtime/vm/class.h b/hphp/runtime/vm/class.h index 8b01e204a..208fa034e 100644 --- a/hphp/runtime/vm/class.h +++ b/hphp/runtime/vm/class.h @@ -776,8 +776,30 @@ public: return m_constants.contains(clsCnsName); } + /* + * Look up the actual value of a class constant. + * + * Performs dynamic initialization if necessary. + */ TypedValue* clsCnsGet(const StringData* clsCnsName) const; + + /* + * Look up a class constant's TypedValue if it doesn't require + * dynamic initialization. + * + * The TypedValue represents the constant's value iff it is a + * scalar, otherwise it has m_type set to KindOfUninit. (Non-scalar + * class constants need to run 86cinit code to determine their value + * at runtime.) + */ + TypedValue* cnsNameToTV(const StringData* name, Slot& slot) const; + + /* + * Provide the current runtime type of this class constant. This + * has predictive value for the translator. + */ DataType clsCnsType(const StringData* clsCnsName) const; + void initialize() const; void initPropHandle() const; unsigned propHandle() const { return m_propDataCache; } @@ -886,7 +908,6 @@ private: void setPropData(PropInitVec* propData) const; void setSPropData(TypedValue* sPropData) const; TypedValue* getSPropData() const; - TypedValue* cnsNameToTV(const StringData* name, Slot& slot) const; void importTraitMethod(const TraitMethod& traitMethod, const StringData* methName, diff --git a/hphp/runtime/vm/jit/codegen.cpp b/hphp/runtime/vm/jit/codegen.cpp index 3ea58e40d..40cbf7f65 100644 --- a/hphp/runtime/vm/jit/codegen.cpp +++ b/hphp/runtime/vm/jit/codegen.cpp @@ -4206,6 +4206,14 @@ void CodeGenerator::cgCheckTypeMem(IRInstruction* inst) { reg[TVOFF(m_data)], inst->taken()); } +void CodeGenerator::cgCheckDefinedClsEq(IRInstruction* inst) { + auto const clsName = inst->extra()->clsName; + auto const cls = inst->extra()->cls; + auto const ch = TargetCache::allocKnownClass(clsName); + m_as.cmpq(cls, rVmTl[ch]); + emitFwdJcc(CC_NZ, inst->taken()); +} + void CodeGenerator::cgGuardRefs(IRInstruction* inst) { assert(inst->numSrcs() == 5); diff --git a/hphp/runtime/vm/jit/extradata.h b/hphp/runtime/vm/jit/extradata.h index 751c5a978..caf93bf06 100644 --- a/hphp/runtime/vm/jit/extradata.h +++ b/hphp/runtime/vm/jit/extradata.h @@ -300,6 +300,23 @@ struct ClsCnsName : IRExtraData { const StringData* cnsName; }; +/* + * The name of a class, and the expected Class* at runtime. + */ +struct CheckDefinedClsData : IRExtraData { + CheckDefinedClsData(const StringData* clsName, const Class* cls) + : clsName(clsName) + , cls(cls) + {} + + std::string show() const { + return folly::to(clsName->data()); + } + + const StringData* clsName; + const Class* cls; +}; + ////////////////////////////////////////////////////////////////////// #define X(op, data) \ @@ -361,6 +378,7 @@ X(ReqBindJmpZero, ReqBindJccData); X(ReqBindJmpNZero, ReqBindJccData); X(SideExitGuardLoc, SideExitGuardData); X(SideExitGuardStk, SideExitGuardData); +X(CheckDefinedClsEq, CheckDefinedClsData); #undef X diff --git a/hphp/runtime/vm/jit/hhbctranslator.cpp b/hphp/runtime/vm/jit/hhbctranslator.cpp index d81f08ab1..14e876b30 100644 --- a/hphp/runtime/vm/jit/hhbctranslator.cpp +++ b/hphp/runtime/vm/jit/hhbctranslator.cpp @@ -36,6 +36,31 @@ TRACE_SET_MOD(hhir); using namespace HPHP::Transl; +////////////////////////////////////////////////////////////////////// + +namespace { + +bool classIsUnique(const Class* cls) { + return RuntimeOption::RepoAuthoritative && + cls && + (cls->attrs() & AttrUnique); +} + +bool classIsPersistent(const Class* cls) { + return RuntimeOption::RepoAuthoritative && + cls && + (cls->attrs() & AttrPersistent); +} + +bool classIsUniqueNormalClass(const Class* cls) { + return classIsUnique(cls) && + !(cls->attrs() & (AttrInterface | AttrTrait)); +} + +} + +////////////////////////////////////////////////////////////////////// + HhbcTranslator::HhbcTranslator(IRFactory& irFactory, Offset startOffset, uint32_t initialSpOffsetFromFp, @@ -56,6 +81,20 @@ HhbcTranslator::HhbcTranslator(IRFactory& irFactory, gen(DefSP, StackOffset(initialSpOffsetFromFp), fp); } +bool HhbcTranslator::classIsUniqueOrCtxParent(const Class* cls) const { + if (!cls) return false; + if (classIsUnique(cls)) return true; + if (!curClass()) return false; + return curClass()->classof(cls); +} + +bool HhbcTranslator::classIsPersistentOrCtxParent(const Class* cls) const { + if (!cls) return false; + if (classIsPersistent(cls)) return true; + if (!curClass()) return false; + return curClass()->classof(cls); +} + ArrayData* HhbcTranslator::lookupArrayId(int arrId) { return curUnit()->lookupArrayId(arrId); } @@ -492,27 +531,12 @@ void HhbcTranslator::emitCns(uint32_t id) { SSATmp* result = nullptr; Type cnsType = Type::Cell; if (tv) { - switch (tv->m_type) { - case KindOfUninit: - // a dynamic system constant. always a slow lookup - result = gen(LookupCns, cnsType, cnsNameTmp); - break; - case KindOfBoolean: - result = cns((bool)tv->m_data.num); - break; - case KindOfInt64: - result = cns(tv->m_data.num); - break; - case KindOfDouble: - result = cns(tv->m_data.dbl); - break; - case KindOfString: - case KindOfStaticString: - result = cns(tv->m_data.pstr); - break; - default: - not_reached(); - } + result = + // KindOfUninit is a dynamic system constant. always a slow + // lookup. + tv->m_type == KindOfUninit + ? gen(LookupCns, cnsType, cnsNameTmp) + : staticTVCns(tv); } else { SSATmp* c1 = gen(LdCns, cnsType, cnsNameTmp); result = m_tb->cond( @@ -1505,9 +1529,26 @@ void HhbcTranslator::emitCmp(Opcode opc) { gen(DecRef, src1); } +// Return a constant SSATmp representing a static value held in a +// TypedValue. The TypedValue may be a non-scalar, but it must have a +// static value. +SSATmp* HhbcTranslator::staticTVCns(const TypedValue* tv) { + switch (tv->m_type) { + case KindOfNull: return m_tb->genDefInitNull(); + case KindOfBoolean: return cns(!!tv->m_data.num); + case KindOfInt64: return cns(tv->m_data.num); + case KindOfString: + case KindOfStaticString: return cns(tv->m_data.pstr); + case KindOfDouble: return cns(tv->m_data.dbl); + case KindOfArray: return cns(tv->m_data.parr); + default: always_assert(0); + } +} + void HhbcTranslator::emitClsCnsD(int32_t cnsNameId, int32_t clsNameId) { - auto const clsCnsName = ClsCnsName { lookupStringId(clsNameId), - lookupStringId(cnsNameId) }; + auto const clsNameStr = lookupStringId(clsNameId); + auto const cnsNameStr = lookupStringId(cnsNameId); + auto const clsCnsName = ClsCnsName { clsNameStr, cnsNameStr }; // If we have to side exit, do the target cache lookup before // chaining to another Tracelet so forward progress still happens. @@ -1518,6 +1559,27 @@ void HhbcTranslator::emitClsCnsD(int32_t cnsNameId, int32_t clsNameId) { } ); + /* + * If the class is already defined in this request, and this + * constant is a scalar constant, we can just compile it to a + * literal. + * + * We need to guard at runtime that the class is defined in this + * request and has the Class* we expect. If the class is persistent + * or a parent of the current context, we don't need the guard. + */ + if (auto const cls = Unit::lookupClass(clsNameStr)) { + Slot ignore; + auto const tv = cls->cnsNameToTV(cnsNameStr, ignore); + if (tv && tv->m_type != KindOfUninit) { + if (!classIsPersistentOrCtxParent(cls)) { + gen(CheckDefinedClsEq, CheckDefinedClsData{clsNameStr, cls}, sideExit); + } + push(staticTVCns(tv)); + return; + } + } + auto const cns = gen(LdClsCns, clsCnsName, Type::Uncounted); gen(CheckInit, sideExit, cns); push(cns); diff --git a/hphp/runtime/vm/jit/hhbctranslator.h b/hphp/runtime/vm/jit/hhbctranslator.h index 6268abd8c..cf04e1090 100644 --- a/hphp/runtime/vm/jit/hhbctranslator.h +++ b/hphp/runtime/vm/jit/hhbctranslator.h @@ -662,6 +662,7 @@ private: SSATmp* emitIterInitCommon(int offset, Lambda genFunc); IRInstruction* makeMarker(Offset bcOff); void emitMarker(); + SSATmp* staticTVCns(const TypedValue*); private: // Exit trace creation routines. IRTrace* getExitTrace(Offset targetBcOff = -1); @@ -730,6 +731,13 @@ private: Offset bcOff() const { return m_bcStateStack.back().bcOff; } SrcKey curSrcKey() const { return SrcKey(curFunc(), bcOff()); } + /* + * Predictates for testing information about the relationship of a + * class to the current context class. + */ + bool classIsUniqueOrCtxParent(const Class*) const; + bool classIsPersistentOrCtxParent(const Class*) const; + /* * Return the SrcKey for the next HHBC (whether it is in this * tracelet or not). diff --git a/hphp/runtime/vm/jit/ir.h b/hphp/runtime/vm/jit/ir.h index c0a6bc273..6d5fdce45 100644 --- a/hphp/runtime/vm/jit/ir.h +++ b/hphp/runtime/vm/jit/ir.h @@ -171,6 +171,7 @@ O(CheckStk, D(StkPtr), S(StkPtr), E) \ O(CastStk, D(StkPtr), S(StkPtr), Mem|N|Er) \ O(AssertStk, D(StkPtr), S(StkPtr), E) \ O(AssertStkVal, D(StkPtr), S(StkPtr) S(Gen), E) \ +O(CheckDefinedClsEq, ND, NA, E) \ O(GuardRefs, ND, S(Func) \ S(Int) \ S(Int) \ diff --git a/hphp/runtime/vm/jit/translator-x64.cpp b/hphp/runtime/vm/jit/translator-x64.cpp index fc8d95c78..9bfffa369 100644 --- a/hphp/runtime/vm/jit/translator-x64.cpp +++ b/hphp/runtime/vm/jit/translator-x64.cpp @@ -191,28 +191,6 @@ struct IfCountNotStatic { } }; -bool -classIsUnique(const Class* cls) { - return RuntimeOption::RepoAuthoritative && - cls && - (cls->attrs() & AttrUnique); -} - -bool -classIsUniqueOrCtxParent(const Class* cls) { - if (!cls) return false; - if (classIsUnique(cls)) return true; - Class* ctx = arGetContextClass(curFrame()); - if (!ctx) return false; - return ctx->classof(cls); -} - -bool -classIsUniqueNormalClass(const Class* cls) { - return classIsUnique(cls) && - !(cls->attrs() & (AttrInterface | AttrTrait)); -} - // Segfault handler: figure out if it's an intentional segfault // (timeout exception) and if so, act appropriately. Otherwise, pass // the signal on. diff --git a/hphp/runtime/vm/jit/translator-x64.h b/hphp/runtime/vm/jit/translator-x64.h index 6f4c80092..4143a8e1e 100644 --- a/hphp/runtime/vm/jit/translator-x64.h +++ b/hphp/runtime/vm/jit/translator-x64.h @@ -624,10 +624,6 @@ TXFlags planInstrAdd_Int(const NormalizedInstruction& i); TXFlags planInstrAdd_Array(const NormalizedInstruction& i); void dumpTranslationInfo(const Tracelet& t, TCA postGuards); -bool classIsUnique(const Class* cls); -bool classIsUniqueOrCtxParent(const Class* cls); -bool classIsUniqueNormalClass(const Class* cls); - // SpaceRecorder is used in translator-x64.cpp and in hopt/irtranslator.cpp // RAII logger for TC space consumption. struct SpaceRecorder {