From e4b58ea8c1ec267bd17a2d56dc08faf904bd9c70 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo@fhahn.com>
Date: Wed, 17 Jun 2020 09:24:56 +0100
Subject: [PATCH] [MemDep] Also remove load instructions from NonLocalDesCache.

Currently load instructions are added to the cache for invariant pointer
group dependencies, but only pointer values are removed currently. That
leads to dangling AssertingVHs in the test case below, where we delete a
load from an invariant pointer group. We should also remove the entries
from the cache.

Fixes PR46054.

Reviewers: efriedma, hfinkel, asbirlea

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D81726
---
 .../lib/Analysis/MemoryDependenceAnalysis.cpp | 20 +++--
 .../pr46054-md-nonlocaldefcache-cleanup.ll    | 81 +++++++++++++++++++
 2 files changed, 96 insertions(+), 5 deletions(-)
 create mode 100644 llvm/test/Transforms/GVN/pr46054-md-nonlocaldefcache-cleanup.ll

diff --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
index 142c7fe30523..566eba5c54af 100644
--- a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
+++ b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
@@ -1518,15 +1518,25 @@ void MemoryDependenceResults::removeInstruction(Instruction *RemInst) {
     LocalDeps.erase(LocalDepEntry);
   }
 
-  // If we have any cached pointer dependencies on this instruction, remove
-  // them.  If the instruction has non-pointer type, then it can't be a pointer
-  // base.
+  // If we have any cached dependencies on this instruction, remove
+  // them.
 
-  // Remove it from both the load info and the store info.  The instruction
-  // can't be in either of these maps if it is non-pointer.
+  // If the instruction is a pointer, remove it from both the load info and the
+  // store info.
   if (RemInst->getType()->isPointerTy()) {
     RemoveCachedNonLocalPointerDependencies(ValueIsLoadPair(RemInst, false));
     RemoveCachedNonLocalPointerDependencies(ValueIsLoadPair(RemInst, true));
+  } else {
+    // Otherwise, if the instructions is in the map directly, it must be a load.
+    // Remove it.
+    auto toRemoveIt = NonLocalDefsCache.find(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);
+      NonLocalDefsCache.erase(toRemoveIt);
+    }
   }
 
   // Loop over all of the things that depend on the instruction we're removing.
diff --git a/llvm/test/Transforms/GVN/pr46054-md-nonlocaldefcache-cleanup.ll b/llvm/test/Transforms/GVN/pr46054-md-nonlocaldefcache-cleanup.ll
new file mode 100644
index 000000000000..bf4efc78de9d
--- /dev/null
+++ b/llvm/test/Transforms/GVN/pr46054-md-nonlocaldefcache-cleanup.ll
@@ -0,0 +1,81 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -gvn -S %s | FileCheck %s
+
+; Test for PR46054. Make sure we correctly invalidate MemoryDependenceAnalysis,
+; after removing a nonlocaldef.
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+define void @test_double(double* %data) {
+; CHECK-LABEL: @test_double(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[LUC:%.*]] = tail call noalias nonnull i64* @data()
+; CHECK-NEXT:    store i64 1, i64* [[LUC]], align 8, !invariant.group !0
+; CHECK-NEXT:    call void @fn(i64 1)
+; CHECK-NEXT:    br i1 true, label [[A:%.*]], label [[ENTRY_B_CRIT_EDGE:%.*]]
+; CHECK:       entry.B_crit_edge:
+; CHECK-NEXT:    br label [[B:%.*]]
+; CHECK:       A:
+; CHECK-NEXT:    br label [[B]]
+; CHECK:       B:
+; CHECK-NEXT:    call void @fn(i64 1)
+; CHECK-NEXT:    ret void
+;
+entry:
+  %luc = tail call noalias nonnull i64* @data()
+  store i64 1, i64* %luc, !invariant.group !0
+  %QQ = load i64, i64* %luc, !invariant.group !0
+  call void @fn(i64 %QQ)
+  br i1 true, label %A, label %B
+
+A:                                                ; preds = %loop
+  br label %B
+
+B:                                                ; preds = %A, %loop
+  %QQ.1 = load i64, i64* %luc, !invariant.group !0
+  call void @fn(i64 %QQ.1)
+  ret void
+}
+
+declare void @fn(i64)
+
+declare noalias i64* @data()
+
+
+define void @test_double_ptr(double** %data) {
+; CHECK-LABEL: @test_double_ptr(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[LUC:%.*]] = tail call noalias nonnull i64** @data.ptr()
+; CHECK-NEXT:    store i64* null, i64** [[LUC]], align 8, !invariant.group !0
+; CHECK-NEXT:    call void @fn.ptr(i64* null)
+; CHECK-NEXT:    br i1 true, label [[A:%.*]], label [[ENTRY_B_CRIT_EDGE:%.*]]
+; CHECK:       entry.B_crit_edge:
+; CHECK-NEXT:    br label [[B:%.*]]
+; CHECK:       A:
+; CHECK-NEXT:    br label [[B]]
+; CHECK:       B:
+; CHECK-NEXT:    call void @fn.ptr(i64* null)
+; CHECK-NEXT:    ret void
+;
+entry:
+  %luc = tail call noalias nonnull i64** @data.ptr()
+  store i64* null, i64** %luc, !invariant.group !0
+  %QQ = load i64*, i64** %luc, !invariant.group !0
+  call void @fn.ptr(i64* %QQ)
+  br i1 true, label %A, label %B
+
+A:                                                ; preds = %loop
+  br label %B
+
+B:                                                ; preds = %A, %loop
+  %QQ.1 = load i64*, i64** %luc, !invariant.group !0
+  call void @fn.ptr(i64* %QQ.1)
+  ret void
+}
+
+
+declare void @fn.ptr(i64 *)
+
+declare noalias i64** @data.ptr()
+
+!0 = distinct !{}