[llvm] AtomicExpand: Stop trying to prune cmpxchg extractvalue users (PR #103211)

[Matt Arsenault via llvm-commits]

https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/103211

>From 43d4cb7e4bf7c20ddfd03caf2e63cf6a03bfa24c Mon Sep 17 00:00:00 2001
From: Matt Arsenault <Matthew.Arsenault at amd.com>
Date: Tue, 13 Aug 2024 13:40:13 +0400
Subject: [PATCH] AtomicExpand: Stop trying to prune cmpxchg extractvalue users

The expansion for cmpxchg was trying to tidy up extractvalue users
to directly use the lowered pieces, and then erasing the now dead
extractvalues. This was making an assumption about the iteration
order did not depend on those user instructions.

Continue doing the replacement, but just leave the dead extractvalues.
This is a minor regression, but it is of no importance since the
dead instructions will just get dropped during codegen anyway.
---
 llvm/lib/CodeGen/AtomicExpandPass.cpp                |  7 -------
 .../test/Transforms/AtomicExpand/ARM/cmpxchg-weak.ll | 12 ++++++++++++
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/llvm/lib/CodeGen/AtomicExpandPass.cpp b/llvm/lib/CodeGen/AtomicExpandPass.cpp
index d8f33c42a8a14..b6344e0c27c4e 100644
--- a/llvm/lib/CodeGen/AtomicExpandPass.cpp
+++ b/llvm/lib/CodeGen/AtomicExpandPass.cpp
@@ -1514,7 +1514,6 @@ bool AtomicExpandImpl::expandAtomicCmpXchg(AtomicCmpXchgInst *CI) {
 
   // Look for any users of the cmpxchg that are just comparing the loaded value
   // against the desired one, and replace them with the CFG-derived version.
-  SmallVector<ExtractValueInst *, 2> PrunedInsts;
   for (auto *User : CI->users()) {
     ExtractValueInst *EV = dyn_cast<ExtractValueInst>(User);
     if (!EV)
@@ -1527,14 +1526,8 @@ bool AtomicExpandImpl::expandAtomicCmpXchg(AtomicCmpXchgInst *CI) {
       EV->replaceAllUsesWith(Loaded);
     else
       EV->replaceAllUsesWith(Success);
-
-    PrunedInsts.push_back(EV);
   }
 
-  // We can remove the instructions now we're no longer iterating through them.
-  for (auto *EV : PrunedInsts)
-    EV->eraseFromParent();
-
   if (!CI->use_empty()) {
     // Some use of the full struct return that we don't understand has happened,
     // so we've got to reconstruct it properly.
diff --git a/llvm/test/Transforms/AtomicExpand/ARM/cmpxchg-weak.ll b/llvm/test/Transforms/AtomicExpand/ARM/cmpxchg-weak.ll
index 8195a5b6145e3..d1d6f89bccade 100644
--- a/llvm/test/Transforms/AtomicExpand/ARM/cmpxchg-weak.ll
+++ b/llvm/test/Transforms/AtomicExpand/ARM/cmpxchg-weak.ll
@@ -34,6 +34,9 @@ define i32 @test_cmpxchg_seq_cst(ptr %addr, i32 %desired, i32 %new) {
 ; CHECK:       [[CMPXCHG_END]]:
 ; CHECK-NEXT:    [[LOADED_EXIT:%.*]] = phi i32 [ [[LOADED_TRYSTORE]], %[[CMPXCHG_SUCCESS]] ], [ [[LOADED_FAILURE]], %[[CMPXCHG_FAILURE]] ]
 ; CHECK-NEXT:    [[SUCCESS1:%.*]] = phi i1 [ true, %[[CMPXCHG_SUCCESS]] ], [ false, %[[CMPXCHG_FAILURE]] ]
+; CHECK-NEXT:    [[TMP3:%.*]] = insertvalue { i32, i1 } poison, i32 [[LOADED_EXIT]], 0
+; CHECK-NEXT:    [[TMP4:%.*]] = insertvalue { i32, i1 } [[TMP3]], i1 [[SUCCESS1]], 1
+; CHECK-NEXT:    [[OLDVAL:%.*]] = extractvalue { i32, i1 } [[TMP4]], 0
 ; CHECK-NEXT:    ret i32 [[LOADED_EXIT]]
 ;
   %pair = cmpxchg weak ptr %addr, i32 %desired, i32 %new seq_cst seq_cst
@@ -72,6 +75,9 @@ define i1 @test_cmpxchg_weak_fail(ptr %addr, i32 %desired, i32 %new) {
 ; CHECK:       [[CMPXCHG_END]]:
 ; CHECK-NEXT:    [[LOADED_EXIT:%.*]] = phi i32 [ [[LOADED_TRYSTORE]], %[[CMPXCHG_SUCCESS]] ], [ [[LOADED_FAILURE]], %[[CMPXCHG_FAILURE]] ]
 ; CHECK-NEXT:    [[SUCCESS1:%.*]] = phi i1 [ true, %[[CMPXCHG_SUCCESS]] ], [ false, %[[CMPXCHG_FAILURE]] ]
+; CHECK-NEXT:    [[TMP3:%.*]] = insertvalue { i32, i1 } poison, i32 [[LOADED_EXIT]], 0
+; CHECK-NEXT:    [[TMP4:%.*]] = insertvalue { i32, i1 } [[TMP3]], i1 [[SUCCESS1]], 1
+; CHECK-NEXT:    [[OLDVAL:%.*]] = extractvalue { i32, i1 } [[TMP4]], 1
 ; CHECK-NEXT:    ret i1 [[SUCCESS1]]
 ;
   %pair = cmpxchg weak ptr %addr, i32 %desired, i32 %new seq_cst monotonic
@@ -108,6 +114,9 @@ define i32 @test_cmpxchg_monotonic(ptr %addr, i32 %desired, i32 %new) {
 ; CHECK:       [[CMPXCHG_END]]:
 ; CHECK-NEXT:    [[LOADED_EXIT:%.*]] = phi i32 [ [[LOADED_TRYSTORE]], %[[CMPXCHG_SUCCESS]] ], [ [[LOADED_FAILURE]], %[[CMPXCHG_FAILURE]] ]
 ; CHECK-NEXT:    [[SUCCESS1:%.*]] = phi i1 [ true, %[[CMPXCHG_SUCCESS]] ], [ false, %[[CMPXCHG_FAILURE]] ]
+; CHECK-NEXT:    [[TMP3:%.*]] = insertvalue { i32, i1 } poison, i32 [[LOADED_EXIT]], 0
+; CHECK-NEXT:    [[TMP4:%.*]] = insertvalue { i32, i1 } [[TMP3]], i1 [[SUCCESS1]], 1
+; CHECK-NEXT:    [[OLDVAL:%.*]] = extractvalue { i32, i1 } [[TMP4]], 0
 ; CHECK-NEXT:    ret i32 [[LOADED_EXIT]]
 ;
   %pair = cmpxchg weak ptr %addr, i32 %desired, i32 %new monotonic monotonic
@@ -147,6 +156,9 @@ define i32 @test_cmpxchg_seq_cst_minsize(ptr %addr, i32 %desired, i32 %new) mins
 ; CHECK:       [[CMPXCHG_END]]:
 ; CHECK-NEXT:    [[LOADED_EXIT:%.*]] = phi i32 [ [[LOADED_TRYSTORE]], %[[CMPXCHG_SUCCESS]] ], [ [[LOADED_FAILURE]], %[[CMPXCHG_FAILURE]] ]
 ; CHECK-NEXT:    [[SUCCESS1:%.*]] = phi i1 [ true, %[[CMPXCHG_SUCCESS]] ], [ false, %[[CMPXCHG_FAILURE]] ]
+; CHECK-NEXT:    [[TMP3:%.*]] = insertvalue { i32, i1 } poison, i32 [[LOADED_EXIT]], 0
+; CHECK-NEXT:    [[TMP4:%.*]] = insertvalue { i32, i1 } [[TMP3]], i1 [[SUCCESS1]], 1
+; CHECK-NEXT:    [[OLDVAL:%.*]] = extractvalue { i32, i1 } [[TMP4]], 0
 ; CHECK-NEXT:    ret i32 [[LOADED_EXIT]]
 ;
   %pair = cmpxchg weak ptr %addr, i32 %desired, i32 %new seq_cst seq_cst