diff --git a/hphp/runtime/vm/translator/hopt/cse.h b/hphp/runtime/vm/translator/hopt/cse.h index 74e8bb0bb..7c06933b4 100644 --- a/hphp/runtime/vm/translator/hopt/cse.h +++ b/hphp/runtime/vm/translator/hopt/cse.h @@ -71,16 +71,16 @@ struct CSEHash { private: struct EqualsOp { bool operator()(IRInstruction* i1, IRInstruction* i2) const { - return i1->equals(i2); + return i1->cseEquals(i2); } }; struct HashOp { size_t operator()(IRInstruction* inst) const { - return inst->hash(); + return inst->cseHash(); } size_t hash(IRInstruction* inst) const { - return inst->hash(); + return inst->cseHash(); } }; diff --git a/hphp/runtime/vm/translator/hopt/ir.cpp b/hphp/runtime/vm/translator/hopt/ir.cpp index c335c55b2..41c4a8b48 100644 --- a/hphp/runtime/vm/translator/hopt/ir.cpp +++ b/hphp/runtime/vm/translator/hopt/ir.cpp @@ -203,17 +203,17 @@ RetType dispatchExtra(Opcode opc, IRExtraData* data, Args&&... args) { not_reached(); } -FOLLY_CREATE_HAS_MEMBER_FN_TRAITS(has_hash, hash); -FOLLY_CREATE_HAS_MEMBER_FN_TRAITS(has_equals, equals); -FOLLY_CREATE_HAS_MEMBER_FN_TRAITS(has_clone, clone); -FOLLY_CREATE_HAS_MEMBER_FN_TRAITS(has_show, show); +FOLLY_CREATE_HAS_MEMBER_FN_TRAITS(has_cseHash, cseHash); +FOLLY_CREATE_HAS_MEMBER_FN_TRAITS(has_cseEquals, cseEquals); +FOLLY_CREATE_HAS_MEMBER_FN_TRAITS(has_clone, clone); +FOLLY_CREATE_HAS_MEMBER_FN_TRAITS(has_show, show); template typename std::enable_if< - has_hash::value, + has_cseHash::value, size_t ->::type hashExtraImpl(T* t) { return t->hash(); } -size_t hashExtraImpl(IRExtraData*) { +>::type cseHashExtraImpl(T* t) { return t->cseHash(); } +size_t cseHashExtraImpl(IRExtraData*) { // This probably means an instruction was marked CanCSE but its // extra data had no hash function. always_assert(!"attempted to hash extra data that didn't " @@ -222,13 +222,13 @@ size_t hashExtraImpl(IRExtraData*) { template typename std::enable_if< - has_equals::value || - has_equals::value, + has_cseEquals::value || + has_cseEquals::value, bool ->::type equalsExtraImpl(T* t, IRExtraData* o) { - return t->equals(*static_cast(o)); +>::type cseEqualsExtraImpl(T* t, IRExtraData* o) { + return t->cseEquals(*static_cast(o)); } -bool equalsExtraImpl(IRExtraData*, IRExtraData*) { +bool cseEqualsExtraImpl(IRExtraData*, IRExtraData*) { // This probably means an instruction was marked CanCSE but its // extra data had no equals function. always_assert(!"attempted to compare extra data that didn't " @@ -260,13 +260,13 @@ typename std::enable_if< >::type showExtraImpl(T* t) { return t->show(); } std::string showExtraImpl(IRExtraData*) { return "..."; } -MAKE_DISPATCHER(HashDispatcher, size_t, hashExtraImpl); -size_t hashExtra(Opcode opc, IRExtraData* data) { +MAKE_DISPATCHER(HashDispatcher, size_t, cseHashExtraImpl); +size_t cseHashExtra(Opcode opc, IRExtraData* data) { return dispatchExtra(opc, data); } -MAKE_DISPATCHER(EqualsDispatcher, bool, equalsExtraImpl); -bool equalsExtra(Opcode opc, IRExtraData* data, IRExtraData* other) { +MAKE_DISPATCHER(EqualsDispatcher, bool, cseEqualsExtraImpl); +bool cseEqualsExtra(Opcode opc, IRExtraData* data, IRExtraData* other) { return dispatchExtra(opc, data, other); } @@ -679,7 +679,9 @@ void Block::removeEdge(IRInstruction* jmp) { assert((node->next = nullptr, true)); } -bool IRInstruction::equals(IRInstruction* inst) const { +bool IRInstruction::cseEquals(IRInstruction* inst) const { + assert(canCSE()); + if (m_op != inst->m_op || m_typeParam != inst->m_typeParam || m_numSrcs != inst->m_numSrcs) { @@ -690,20 +692,29 @@ bool IRInstruction::equals(IRInstruction* inst) const { return false; } } - if (hasExtra() && !equalsExtra(getOpcode(), m_extra, inst->m_extra)) { + if (hasExtra() && !cseEqualsExtra(getOpcode(), m_extra, inst->m_extra)) { return false; } - // TODO: check label for ControlFlowInstructions? + /* + * Don't CSE on m_taken---it's ok to use the destination of some + * earlier guarded load even though the instruction we may have + * generated here would've exited to a different trace. + * + * For example, we use this to cse LdThis regardless of its label. + */ return true; } -size_t IRInstruction::hash() const { +size_t IRInstruction::cseHash() const { + assert(canCSE()); + size_t srcHash = 0; for (unsigned i = 0; i < getNumSrcs(); ++i) { srcHash = CSEHash::hashCombine(srcHash, getSrc(i)); } if (hasExtra()) { - srcHash = CSEHash::hashCombine(srcHash, hashExtra(getOpcode(), m_extra)); + srcHash = CSEHash::hashCombine(srcHash, + cseHashExtra(getOpcode(), m_extra)); } return CSEHash::hashCombine(srcHash, m_op, m_typeParam); } diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index 7251331ca..43e4a8bff 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -610,9 +610,9 @@ enum Opcode : uint16_t { * * In addition, for extra data used with a cse-able instruction: * - * - Implement an equals() member that indicates equality for CSE + * - Implement an cseEquals() member that indicates equality for CSE * purposes. - * - Implement a hash() method. + * - Implement a cseHash() method. * * Finally, optionally they may implement a show() method for use in * debug printouts. @@ -688,8 +688,8 @@ struct LocalId : IRExtraData { : locId(id) {} - bool equals(LocalId o) const { return locId == o.locId; } - size_t hash() const { return std::hash()(locId); } + bool cseEquals(LocalId o) const { return locId == o.locId; } + size_t cseHash() const { return std::hash()(locId); } std::string show() const { return folly::to(locId); } uint32_t locId; @@ -714,8 +714,8 @@ struct ConstData : IRExtraData { return ret; } - bool equals(ConstData o) const { return m_dataBits == o.m_dataBits; } - size_t hash() const { return std::hash()(m_dataBits); } + bool cseEquals(ConstData o) const { return m_dataBits == o.m_dataBits; } + size_t cseHash() const { return std::hash()(m_dataBits); } private: uintptr_t m_dataBits; @@ -1765,8 +1765,14 @@ struct IRInstruction { bool isControlFlowInstruction() const { return m_taken != nullptr; } bool isBlockEnd() const { return m_taken || isTerminal(); } - bool equals(IRInstruction* inst) const; - size_t hash() const; + /* + * Comparison and hashing for the purposes of CSE-equality. + * + * Pre: canCSE() + */ + bool cseEquals(IRInstruction* inst) const; + size_t cseHash() const; + void print(std::ostream& ostream) const; void print() const; std::string toString() const;