[llvm] 2f6fce8 - [Attributor][FIX] Ensure not to run new queries during manifest

Johannes Doerfert via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 23 20:35:33 PST 2023


Author: Johannes Doerfert
Date: 2023-01-23T20:32:06-08:00
New Revision: 2f6fce8bba66512ed52a15fac8f10828b0e4daba

URL: https://github.com/llvm/llvm-project/commit/2f6fce8bba66512ed52a15fac8f10828b0e4daba
DIFF: https://github.com/llvm/llvm-project/commit/2f6fce8bba66512ed52a15fac8f10828b0e4daba.diff

LOG: [Attributor][FIX] Ensure not to run new queries during manifest

If we modified the IR during manifest, e.g., SPMDzation, we might end up
with un-cached reachability queries. This is not good as the result is
going to be optimistic. We now cache the updateImpl result and use it
during manifest.

Bug was exposed in a follow up extension.

Added: 
    

Modified: 
    llvm/lib/Transforms/IPO/AttributorAttributes.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index 40beb7fd6980..fd6815246873 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -4121,14 +4121,19 @@ struct AAIsDeadFloating : public AAIsDeadValueImpl {
     if (SI.isVolatile())
       return false;
 
+    // If we are collecting assumes to be deleted we are in the manifest stage.
+    // It's problematic to collect the potential copies again now so we use the
+    // cached ones.
     bool UsedAssumedInformation = false;
-    SmallSetVector<Value *, 4> PotentialCopies;
-    if (!AA::getPotentialCopiesOfStoredValue(A, SI, PotentialCopies, *this,
-                                             UsedAssumedInformation)) {
-      LLVM_DEBUG(
-          dbgs()
-          << "[AAIsDead] Could not determine potential copies of store!\n");
-      return false;
+    if (!AssumeOnlyInst) {
+      PotentialCopies.clear();
+      if (!AA::getPotentialCopiesOfStoredValue(A, SI, PotentialCopies, *this,
+                                               UsedAssumedInformation)) {
+        LLVM_DEBUG(
+            dbgs()
+            << "[AAIsDead] Could not determine potential copies of store!\n");
+        return false;
+      }
     }
     LLVM_DEBUG(dbgs() << "[AAIsDead] Store has " << PotentialCopies.size()
                       << " potential copies.\n");
@@ -4217,6 +4222,10 @@ struct AAIsDeadFloating : public AAIsDeadValueImpl {
   void trackStatistics() const override {
     STATS_DECLTRACK_FLOATING_ATTR(IsDead)
   }
+
+private:
+  // The potential copies of a dead store, used for deletion during manifest.
+  SmallSetVector<Value *, 4> PotentialCopies;
 };
 
 struct AAIsDeadArgument : public AAIsDeadFloating {


        


More information about the llvm-commits mailing list