[llvm] a3b12f0 - [MemDep] Remove cached non-local defs referencing deleted instructions (#219929)

[via llvm-commits]

Author: Oscar Priego
Date: 2026-09-08T14:46:54+02:00
New Revision: a3b12f0cd5337230199fc9c6319cc2c4614f75b8

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

LOG: [MemDep] Remove cached non-local defs referencing deleted instructions (#219929)

MemoryDependenceResults::removeInstruction() removed entries from
NonLocalDefsCache when the removed non-pointer instruction was itself a
cache key, but did not remove entries where that instruction was the cached
dependency result.

This could leave a stale instruction reachable through NonLocalDefsCache
after GVN erased it. A later non-local load query could then consume the
dangling dependency and crash in GVN::analyzeLoadAvailability().

Use ReverseNonLocalDefsCache to remove forward cache entries whose
dependency result references the removed instruction, keeping both maps
consistent.

Related removal paths now use the existing reverse-map helper, with
consistency assertions to catch future map divergence.

Fixes #219885.

A downstream Mono integration has also hit #219885 in practice when
tagging non-pointer array-header loads with !invariant.group. They
currently gate those tags off as a temporary workaround pending the
upstream fix in this PR:

Phantomical/mono at 7e44c68

Added: 
    

Modified: 
    llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
    llvm/test/Transforms/GVN/pr46054-md-nonlocaldefcache-cleanup.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
index 0ccaadaa1dfb7..a58608b53bb1f 100644
--- a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -868,8 +868,9 @@ void MemoryDependenceResults::getNonLocalPointerDependency(
     auto NonLocalDefIt = NonLocalDefsCache.find(QueryInst);
     if (NonLocalDefIt != NonLocalDefsCache.end()) {
       Result.push_back(NonLocalDefIt->second);
-      ReverseNonLocalDefsCache[NonLocalDefIt->second.getResult().getInst()]
-          .erase(QueryInst);
+      RemoveFromReverseMap<const Value *>(
+          ReverseNonLocalDefsCache, NonLocalDefIt->second.getResult().getInst(),
+          QueryInst);
       NonLocalDefsCache.erase(NonLocalDefIt);
       return;
     }
@@ -1504,8 +1505,10 @@ void MemoryDependenceResults::removeCachedNonLocalPointerDependencies(
     if (auto *I = dyn_cast<Instruction>(P.getPointer())) {
       auto toRemoveIt = ReverseNonLocalDefsCache.find(I);
       if (toRemoveIt != ReverseNonLocalDefsCache.end()) {
-        for (const auto *entry : toRemoveIt->second)
-          NonLocalDefsCache.erase(entry);
+        for (const auto *Entry : toRemoveIt->second) {
+          [[maybe_unused]] bool Removed = NonLocalDefsCache.erase(Entry);
+          assert(Removed && "Reverse non-local def map out of sync?");
+        }
         ReverseNonLocalDefsCache.erase(toRemoveIt);
       }
     }
@@ -1587,12 +1590,22 @@ void MemoryDependenceResults::removeInstruction(Instruction *RemInst) {
     if (toRemoveIt != NonLocalDefsCache.end()) {
       assert(isa<LoadInst>(RemInst) &&
              "only load instructions should be added directly");
-      const Instruction *DepV = toRemoveIt->second.getResult().getInst();
-      ReverseNonLocalDefsCache.find(DepV)->second.erase(RemInst);
+      Instruction *DepV = toRemoveIt->second.getResult().getInst();
+      RemoveFromReverseMap<const Value *>(ReverseNonLocalDefsCache, DepV,
+                                          RemInst);
       NonLocalDefsCache.erase(toRemoveIt);
     }
   }
 
+  auto ReverseNonLocalDefIt = ReverseNonLocalDefsCache.find(RemInst);
+  if (ReverseNonLocalDefIt != ReverseNonLocalDefsCache.end()) {
+    for (const Value *QueryInst : ReverseNonLocalDefIt->second) {
+      [[maybe_unused]] bool Removed = NonLocalDefsCache.erase(QueryInst);
+      assert(Removed && "Reverse non-local def map out of sync?");
+    }
+    ReverseNonLocalDefsCache.erase(ReverseNonLocalDefIt);
+  }
+
   // Loop over all of the things that depend on the instruction we're removing.
   SmallVector<std::pair<Instruction *, Instruction *>, 8> ReverseDepsToAdd;
 

diff  --git a/llvm/test/Transforms/GVN/pr46054-md-nonlocaldefcache-cleanup.ll b/llvm/test/Transforms/GVN/pr46054-md-nonlocaldefcache-cleanup.ll
index 6ed447a5f2f7b..66d59ed7e5982 100644
--- a/llvm/test/Transforms/GVN/pr46054-md-nonlocaldefcache-cleanup.ll
+++ b/llvm/test/Transforms/GVN/pr46054-md-nonlocaldefcache-cleanup.ll
@@ -78,4 +78,27 @@ declare void @fn.ptr(ptr)
 
 declare noalias ptr @data.ptr()
 
+define i32 @test_non_pointer_load_deleted_as_cached_def(ptr %p, i1 %c) {
+; CHECK-LABEL: @test_non_pointer_load_deleted_as_cached_def(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[A:%.*]], label [[MID:%.*]]
+; CHECK:       a:
+; CHECK-NEXT:    br label [[MID]]
+; CHECK:       mid:
+; CHECK-NEXT:    [[Q:%.*]] = load atomic i32, ptr [[P:%.*]] unordered, align 4, !invariant.group !0
+; CHECK-NEXT:    ret i32 [[Q]]
+;
+entry:
+  %x = load i32, ptr %p, align 4, !invariant.group !0
+  br i1 %c, label %a, label %mid
+
+a:
+  br label %mid
+
+mid:
+  %q = load atomic i32, ptr %p unordered, align 4, !invariant.group !0
+  %d = add i32 %x, 0
+  ret i32 %q
+}
+
 !0 = distinct !{}