[PATCH] D65852: AssumptionCache: remove old affected values after RAUW

[Tim Northover via Phabricator via llvm-commits]

t.p.northover created this revision.
Herald added subscribers: hiraditya, mcrosier.
Herald added a project: LLVM.

If they're left in the cache then they can't be removed efficiently when the cache is notified to unlink a @llvm.assume call, and that can lead to values from different functions entirely remaining there. ValueTracking.cpp does not like this.

There aren't that many users of the assumption cache so we could modify them to cope with this situation, but I decided the cache was still messed up and most RAUW calls are fairly swiftly followed by removing the original so not much is lost by transferring any assumptions.

Unfortunately I couldn't write a test for this, the example I had was huge (from Clang's ExprConstant) and didn't reproduce under opt even with the whole file.


https://reviews.llvm.org/D65852

Files:
  llvm/include/llvm/Analysis/AssumptionCache.h
  llvm/lib/Analysis/AssumptionCache.cpp


Index: llvm/lib/Analysis/AssumptionCache.cpp
===================================================================
--- llvm/lib/Analysis/AssumptionCache.cpp
+++ llvm/lib/Analysis/AssumptionCache.cpp
@@ -140,7 +140,7 @@
   // 'this' now dangles!
 }
 
-void AssumptionCache::copyAffectedValuesInCache(Value *OV, Value *NV) {
+void AssumptionCache::transferAffectedValuesInCache(Value *OV, Value *NV) {
   auto &NAVV = getOrInsertAffectedValues(NV);
   auto AVI = AffectedValues.find(OV);
   if (AVI == AffectedValues.end())
@@ -149,6 +149,7 @@
   for (auto &A : AVI->second)
     if (std::find(NAVV.begin(), NAVV.end(), A) == NAVV.end())
       NAVV.push_back(A);
+  AffectedValues.erase(OV);
 }
 
 void AssumptionCache::AffectedValueCallbackVH::allUsesReplacedWith(Value *NV) {
@@ -157,7 +158,7 @@
 
   // Any assumptions that affected this value now affect the new value.
 
-  AC->copyAffectedValuesInCache(getValPtr(), NV);
+  AC->transferAffectedValuesInCache(getValPtr(), NV);
   // 'this' now might dangle! If the AffectedValues map was resized to add an
   // entry for NV then this object might have been destroyed in favor of some
   // copy in the grown map.
Index: llvm/include/llvm/Analysis/AssumptionCache.h
===================================================================
--- llvm/include/llvm/Analysis/AssumptionCache.h
+++ llvm/include/llvm/Analysis/AssumptionCache.h
@@ -73,8 +73,8 @@
   /// Get the vector of assumptions which affect a value from the cache.
   SmallVector<WeakTrackingVH, 1> &getOrInsertAffectedValues(Value *V);
 
-  /// Copy affected values in the cache for OV to be affected values for NV.
-  void copyAffectedValuesInCache(Value *OV, Value *NV);
+  /// Move affected values in the cache for OV to be affected values for NV.
+  void transferAffectedValuesInCache(Value *OV, Value *NV);
 
   /// Flag tracking whether we have scanned the function yet.
   ///


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65852.213814.patch
Type: text/x-patch
Size: 1895 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190807/6e73a3b3/attachment.bin>