From 50d412de361f0ea815b2245c682a414503587821 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 29 Jul 2020 13:26:34 -0700 Subject: [PATCH] Tolerate pathological growth of optCSEHash (#40056) The optCSEhash can grow an unbounded amount if the function has numerous trees which are put into the optCSEhash as possible CSE candidates, but fewer than MAX_CSE_CNT are found, then the compiler will spend excessive amounts of time looking up entries in the optCSEhash. This fix addresses the issue by making the optCSEhash able to grow its count of buckets. --- src/coreclr/src/jit/compiler.h | 7 +++- src/coreclr/src/jit/optcse.cpp | 69 +++++++++++++++++++++++++++------- 2 files changed, 62 insertions(+), 14 deletions(-) diff --git a/src/coreclr/src/jit/compiler.h b/src/coreclr/src/jit/compiler.h index e1698611978e1..011a2afa78729 100644 --- a/src/coreclr/src/jit/compiler.h +++ b/src/coreclr/src/jit/compiler.h @@ -6363,7 +6363,12 @@ class Compiler // not used for shared const CSE's }; - static const size_t s_optCSEhashSize; + static const size_t s_optCSEhashSizeInitial; + static const size_t s_optCSEhashGrowthFactor; + static const size_t s_optCSEhashBucketSize; + size_t optCSEhashSize; // The current size of hashtable + size_t optCSEhashCount; // Number of entries in hashtable + size_t optCSEhashMaxCountBeforeResize; // Number of entries before resize CSEdsc** optCSEhash; CSEdsc** optCSEtab; diff --git a/src/coreclr/src/jit/optcse.cpp b/src/coreclr/src/jit/optcse.cpp index 1b70f7a027b61..5f89a2e96632f 100644 --- a/src/coreclr/src/jit/optcse.cpp +++ b/src/coreclr/src/jit/optcse.cpp @@ -20,7 +20,9 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX /*****************************************************************************/ /* static */ -const size_t Compiler::s_optCSEhashSize = EXPSET_SZ * 2; +const size_t Compiler::s_optCSEhashSizeInitial = EXPSET_SZ * 2; +const size_t Compiler::s_optCSEhashGrowthFactor = 2; +const size_t Compiler::s_optCSEhashBucketSize = 4; /***************************************************************************** * @@ -36,11 +38,11 @@ void Compiler::optCSEstop() CSEdsc* dsc; CSEdsc** ptr; - unsigned cnt; + size_t cnt; optCSEtab = new (this, CMK_CSE) CSEdsc*[optCSECandidateCount](); - for (cnt = s_optCSEhashSize, ptr = optCSEhash; cnt; cnt--, ptr++) + for (cnt = optCSEhashSize, ptr = optCSEhash; cnt; cnt--, ptr++) { for (dsc = *ptr; dsc; dsc = dsc->csdNextInBucket) { @@ -373,7 +375,11 @@ void Compiler::optValnumCSE_Init() cseMaskTraits = nullptr; // Allocate and clear the hash bucket table - optCSEhash = new (this, CMK_CSE) CSEdsc*[s_optCSEhashSize](); + optCSEhash = new (this, CMK_CSE) CSEdsc*[s_optCSEhashSizeInitial](); + + optCSEhashSize = s_optCSEhashSizeInitial; + optCSEhashMaxCountBeforeResize = optCSEhashSize * s_optCSEhashBucketSize; + optCSEhashCount = 0; optCSECandidateCount = 0; optDoCSE = false; // Stays false until we find duplicate CSE tree @@ -382,6 +388,20 @@ void Compiler::optValnumCSE_Init() optCseCheckedBoundMap = nullptr; } +unsigned optCSEKeyToHashIndex(size_t key, size_t optCSEhashSize) +{ + unsigned hash; + + hash = (unsigned)key; +#ifdef TARGET_64BIT + hash ^= (unsigned)(key >> 32); +#endif + hash *= (unsigned)(optCSEhashSize + 1); + hash >>= 7; + + return hash % optCSEhashSize; +} + //--------------------------------------------------------------------------- // optValnumCSE_Index: // - Returns the CSE index to use for this tree, @@ -402,7 +422,6 @@ void Compiler::optValnumCSE_Init() unsigned Compiler::optValnumCSE_Index(GenTree* tree, Statement* stmt) { size_t key; - unsigned hash; unsigned hval; CSEdsc* hashDsc; bool isIntConstHash = false; @@ -502,14 +521,7 @@ unsigned Compiler::optValnumCSE_Index(GenTree* tree, Statement* stmt) // Compute the hash value for the expression - hash = (unsigned)key; -#ifdef TARGET_64BIT - hash ^= (unsigned)(key >> 32); -#endif - hash *= (unsigned)(s_optCSEhashSize + 1); - hash >>= 7; - - hval = hash % s_optCSEhashSize; + hval = optCSEKeyToHashIndex(key, optCSEhashSize); /* Look for a matching index in the hash table */ @@ -627,6 +639,37 @@ unsigned Compiler::optValnumCSE_Index(GenTree* tree, Statement* stmt) if (optCSECandidateCount < MAX_CSE_CNT) { + if (optCSEhashCount == optCSEhashMaxCountBeforeResize) + { + size_t newOptCSEhashSize = optCSEhashSize * s_optCSEhashGrowthFactor; + CSEdsc** newOptCSEhash = new (this, CMK_CSE) CSEdsc*[newOptCSEhashSize](); + + // Iterate through each existing entry, moving to the new table + CSEdsc** ptr; + CSEdsc* dsc; + size_t cnt; + for (cnt = optCSEhashSize, ptr = optCSEhash; cnt; cnt--, ptr++) + { + for (dsc = *ptr; dsc;) + { + CSEdsc* nextDsc = dsc->csdNextInBucket; + + size_t newHval = optCSEKeyToHashIndex(dsc->csdHashKey, newOptCSEhashSize); + + // Move CSEdsc to bucket in enlarged table + dsc->csdNextInBucket = newOptCSEhash[newHval]; + newOptCSEhash[newHval] = dsc; + + dsc = nextDsc; + } + } + + optCSEhash = newOptCSEhash; + optCSEhashSize = newOptCSEhashSize; + optCSEhashMaxCountBeforeResize = optCSEhashMaxCountBeforeResize * s_optCSEhashGrowthFactor; + } + + ++optCSEhashCount; hashDsc = new (this, CMK_CSE) CSEdsc; hashDsc->csdHashKey = key;