[llvm] [NewGVN] Patch replacement instruction even for removed instructions (PR #67426)

[Nikita Popov via llvm-commits]

https://github.com/nikic created https://github.com/llvm/llvm-project/pull/67426

When removing an instruction, we still need to merge its IR flags into the leader, because there may have been a transitive use.

Fixes https://github.com/llvm/llvm-project/issues/53218.

>From deb260e0857a5f5a8768ba4e1cd1824d715695e6 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Tue, 26 Sep 2023 15:02:09 +0200
Subject: [PATCH] [NewGVN] Patch replacement instruction even for removed
 instructions

When removing an instruction, we still need to merge its IR flags
into the leader, because there may have been a transitive use.

Fixes https://github.com/llvm/llvm-project/issues/53218.
---
 llvm/lib/Transforms/Scalar/NewGVN.cpp | 16 +++++++++++++---
 llvm/test/Transforms/NewGVN/flags.ll  |  2 +-
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp
index fdf81f56d75b93d..2230d2f9a985b70 100644
--- a/llvm/lib/Transforms/Scalar/NewGVN.cpp
+++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp
@@ -4030,9 +4030,19 @@ bool NewGVN::eliminateInstructions(Function &F) {
             // because stores are put in terms of the stored value, we skip
             // stored values here. If the stored value is really dead, it will
             // still be marked for deletion when we process it in its own class.
-            if (!EliminationStack.empty() && Def != EliminationStack.back() &&
-                isa<Instruction>(Def) && !FromStore)
-              markInstructionForDeletion(cast<Instruction>(Def));
+            auto *DefI = dyn_cast<Instruction>(Def);
+            if (!EliminationStack.empty() && DefI && !FromStore) {
+              Value *DominatingLeader = EliminationStack.back();
+              if (DominatingLeader != Def) {
+                // Even if the instruction is removed, we still need to update
+                // flags/metadata due to downstreams users of the leader.
+                auto *II = dyn_cast<IntrinsicInst>(DefI);
+                if (!II || II->getIntrinsicID() != Intrinsic::ssa_copy)
+                  patchReplacementInstruction(DefI, DominatingLeader);
+
+                markInstructionForDeletion(DefI);
+              }
+            }
             continue;
           }
           // At this point, we know it is a Use we are trying to possibly
diff --git a/llvm/test/Transforms/NewGVN/flags.ll b/llvm/test/Transforms/NewGVN/flags.ll
index f3e29ff3c094a02..d481fb3671696c8 100644
--- a/llvm/test/Transforms/NewGVN/flags.ll
+++ b/llvm/test/Transforms/NewGVN/flags.ll
@@ -23,7 +23,7 @@ entry:
 define void @test2(i8 %start, i8 %high) {
 ; CHECK-LABEL: define void @test2
 ; CHECK-SAME: (i8 [[START:%.*]], i8 [[HIGH:%.*]]) {
-; CHECK-NEXT:    [[START1:%.*]] = add nsw i8 [[START]], 4
+; CHECK-NEXT:    [[START1:%.*]] = add i8 [[START]], 4
 ; CHECK-NEXT:    [[T1:%.*]] = icmp ult i8 [[START1]], [[HIGH]]
 ; CHECK-NEXT:    call void @use(i1 [[T1]])
 ; CHECK-NEXT:    call void @use(i1 [[T1]])