diff --git a/include/llvm/Transforms/Scalar/GVNExpression.h b/include/llvm/Transforms/Scalar/GVNExpression.h index a971df975b6..324ebca46de 100644 --- a/include/llvm/Transforms/Scalar/GVNExpression.h +++ b/include/llvm/Transforms/Scalar/GVNExpression.h @@ -58,10 +58,11 @@ class Expression { private: ExpressionType EType; unsigned Opcode; + mutable hash_code HashVal; public: Expression(ExpressionType ET = ET_Base, unsigned O = ~2U) - : EType(ET), Opcode(O) {} + : EType(ET), Opcode(O), HashVal(0) {} Expression(const Expression &) = delete; Expression &operator=(const Expression &) = delete; virtual ~Expression(); @@ -82,6 +83,14 @@ public: return equals(Other); } + hash_code getComputedHash() const { + // It's theoretically possible for a thing to hash to zero. In that case, + // we will just compute the hash a few extra times, which is no worse that + // we did before, which was to compute it always. + if (static_cast(HashVal) == 0) + HashVal = getHashValue(); + return HashVal; + } virtual bool equals(const Expression &Other) const { return true; } diff --git a/lib/Transforms/Scalar/NewGVN.cpp b/lib/Transforms/Scalar/NewGVN.cpp index 39ed209bda2..5e9f40019ce 100644 --- a/lib/Transforms/Scalar/NewGVN.cpp +++ b/lib/Transforms/Scalar/NewGVN.cpp @@ -377,7 +377,6 @@ private: int StoreCount = 0; }; -struct HashedExpression; namespace llvm { template <> struct DenseMapInfo { static const Expression *getEmptyKey() { @@ -391,41 +390,25 @@ template <> struct DenseMapInfo { return reinterpret_cast(Val); } static unsigned getHashValue(const Expression *E) { - return static_cast(E->getHashValue()); + return static_cast(E->getComputedHash()); } - static unsigned getHashValue(const HashedExpression &HE); - static bool isEqual(const HashedExpression &LHS, const Expression *RHS); static bool isEqual(const Expression *LHS, const Expression *RHS) { if (LHS == RHS) return true; if (LHS == getTombstoneKey() || RHS == getTombstoneKey() || LHS == getEmptyKey() || RHS == getEmptyKey()) return false; + // Compare hashes before equality. This is *not* what the hashtable does, + // since it is computing it modulo the number of buckets, whereas we are + // using the full hash keyspace. Since the hashes are precomputed, this + // check is *much* faster than equality. + if (LHS->getComputedHash() != RHS->getComputedHash()) + return false; return *LHS == *RHS; } }; } // end namespace llvm -// This is just a wrapper around Expression that computes the hash value once at -// creation time. Hash values for an Expression can't change once they are -// inserted into the DenseMap (it breaks DenseMap), so they must be immutable at -// that point anyway. -struct HashedExpression { - const Expression *E; - unsigned HashVal; - HashedExpression(const Expression *E) - : E(E), HashVal(DenseMapInfo::getHashValue(E)) {} -}; - -unsigned -DenseMapInfo::getHashValue(const HashedExpression &HE) { - return HE.HashVal; -} -bool DenseMapInfo::isEqual(const HashedExpression &LHS, - const Expression *RHS) { - return isEqual(LHS.E, RHS); -} - namespace { class NewGVN { Function &F; @@ -707,7 +690,7 @@ private: void markPredicateUsersTouched(Instruction *); void markValueLeaderChangeTouched(CongruenceClass *CC); void markMemoryLeaderChangeTouched(CongruenceClass *CC); - void markPhiOfOpsChanged(const HashedExpression &HE); + void markPhiOfOpsChanged(const Expression *E); void addPredicateUsers(const PredicateBase *, Instruction *) const; void addMemoryUsers(const MemoryAccess *To, MemoryAccess *U) const; void addAdditionalUsers(Value *To, Value *User) const; @@ -2199,8 +2182,8 @@ void NewGVN::moveValueToNewCongruenceClass(Instruction *I, const Expression *E, // For a given expression, mark the phi of ops instructions that could have // changed as a result. -void NewGVN::markPhiOfOpsChanged(const HashedExpression &HE) { - touchAndErase(ExpressionToPhiOfOps, HE); +void NewGVN::markPhiOfOpsChanged(const Expression *E) { + touchAndErase(ExpressionToPhiOfOps, E); } // Perform congruence finding on a given value numbering expression. @@ -2214,14 +2197,13 @@ void NewGVN::performCongruenceFinding(Instruction *I, const Expression *E) { assert(!IClass->isDead() && "Found a dead class"); CongruenceClass *EClass = nullptr; - HashedExpression HE(E); if (const auto *VE = dyn_cast(E)) { EClass = ValueToClass.lookup(VE->getVariableValue()); } else if (isa(E)) { EClass = TOPClass; } if (!EClass) { - auto lookupResult = ExpressionToClass.insert_as({E, nullptr}, HE); + auto lookupResult = ExpressionToClass.insert({E, nullptr}); // If it's not in the value table, create a new congruence class. if (lookupResult.second) { @@ -2272,7 +2254,7 @@ void NewGVN::performCongruenceFinding(Instruction *I, const Expression *E) { << "\n"); if (ClassChanged) { moveValueToNewCongruenceClass(I, E, IClass, EClass); - markPhiOfOpsChanged(HE); + markPhiOfOpsChanged(E); } markUsersTouched(I);